Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on inert attributes used on bang macro invocation #87296

Merged
merged 2 commits into from
Jul 24, 2021
Merged

Warn on inert attributes used on bang macro invocation #87296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
070df9e
Warn on inert attributes used on bang macro invocation
Aaron1011 Jul 15, 2021
b41672e
Fix incorrect use of `#[repr(no_niche)]`
Aaron1011 Jul 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 33 additions & 16 deletions compiler/rustc_expand/src/expand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_ast::ptr::P;
use rustc_ast::token;
use rustc_ast::tokenstream::TokenStream;
use rustc_ast::visit::{self, AssocCtxt, Visitor};
use rustc_ast::{AstLike, Block, Inline, ItemKind, Local, MacArgs};
use rustc_ast::{AstLike, Block, Inline, ItemKind, Local, MacArgs, MacCall};
use rustc_ast::{MacCallStmt, MacStmtStyle, MetaItemKind, ModKind, NestedMetaItem};
use rustc_ast::{NodeId, PatKind, Path, StmtKind, Unsafe};
use rustc_ast_pretty::pprust;
Expand All @@ -26,7 +26,7 @@ use rustc_parse::parser::{
AttemptLocalParseRecovery, ForceCollect, Parser, RecoverColon, RecoverComma,
};
use rustc_parse::validate_attr;
use rustc_session::lint::builtin::UNUSED_DOC_COMMENTS;
use rustc_session::lint::builtin::{UNUSED_ATTRIBUTES, UNUSED_DOC_COMMENTS};
use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_session::parse::{feature_err, ParseSess};
use rustc_session::Limit;
Expand Down Expand Up @@ -1070,7 +1070,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {

// Detect use of feature-gated or invalid attributes on macro invocations
// since they will not be detected after macro expansion.
fn check_attributes(&mut self, attrs: &[ast::Attribute]) {
fn check_attributes(&mut self, attrs: &[ast::Attribute], call: &MacCall) {
let features = self.cx.ecfg.features.unwrap();
let mut attrs = attrs.iter().peekable();
let mut span: Option<Span> = None;
Expand All @@ -1085,14 +1085,31 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
continue;
}

if attr.doc_str().is_some() {
if attr.is_doc_comment() {
self.cx.sess.parse_sess.buffer_lint_with_diagnostic(
&UNUSED_DOC_COMMENTS,
current_span,
ast::CRATE_NODE_ID,
self.cx.current_expansion.lint_node_id,
"unused doc comment",
BuiltinLintDiagnostics::UnusedDocComment(attr.span),
);
} else if rustc_attr::is_builtin_attr(attr) {
let attr_name = attr.ident().unwrap().name;
// `#[cfg]` and `#[cfg_attr]` are special - they are
// eagerly evaluated.
if attr_name != sym::cfg && attr_name != sym::cfg_attr {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cfg and cfg_attr should be unreachable here in theory, but that's not entirely true right now because cfg(TRUE) doesn't remove itself.

self.cx.sess.parse_sess.buffer_lint_with_diagnostic(
&UNUSED_ATTRIBUTES,
attr.span,
self.cx.current_expansion.lint_node_id,
&format!("unused attribute `{}`", attr_name),
BuiltinLintDiagnostics::UnusedBuiltinAttribute {
attr_name,
macro_name: pprust::path_to_string(&call.path),
invoc_span: call.path.span,
},
);
}
}
}
}
Expand Down Expand Up @@ -1152,7 +1169,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

if let ast::ExprKind::MacCall(mac) = expr.kind {
self.check_attributes(&expr.attrs);
self.check_attributes(&expr.attrs, &mac);
self.collect_bang(mac, expr.span, AstFragmentKind::Expr).make_expr().into_inner()
} else {
assign_id!(self, &mut expr.id, || {
Expand Down Expand Up @@ -1253,7 +1270,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

if let ast::ExprKind::MacCall(mac) = expr.kind {
self.check_attributes(&expr.attrs);
self.check_attributes(&expr.attrs, &mac);
self.collect_bang(mac, expr.span, AstFragmentKind::OptExpr)
.make_opt_expr()
.map(|expr| expr.into_inner())
Expand Down Expand Up @@ -1296,7 +1313,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {

if let StmtKind::MacCall(mac) = stmt.kind {
let MacCallStmt { mac, style, attrs, tokens: _ } = mac.into_inner();
self.check_attributes(&attrs);
self.check_attributes(&attrs, &mac);
let mut placeholder =
self.collect_bang(mac, stmt.span, AstFragmentKind::Stmts).make_stmts();

Expand Down Expand Up @@ -1344,9 +1361,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
let span = item.span;

match item.kind {
ast::ItemKind::MacCall(..) => {
ast::ItemKind::MacCall(ref mac) => {
self.check_attributes(&attrs, &mac);
item.attrs = attrs;
self.check_attributes(&item.attrs);
item.and_then(|item| match item.kind {
ItemKind::MacCall(mac) => {
self.collect_bang(mac, span, AstFragmentKind::Items).make_items()
Expand Down Expand Up @@ -1455,8 +1472,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

match item.kind {
ast::AssocItemKind::MacCall(..) => {
self.check_attributes(&item.attrs);
ast::AssocItemKind::MacCall(ref mac) => {
self.check_attributes(&item.attrs, &mac);
item.and_then(|item| match item.kind {
ast::AssocItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::TraitItems)
Expand All @@ -1480,8 +1497,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

match item.kind {
ast::AssocItemKind::MacCall(..) => {
self.check_attributes(&item.attrs);
ast::AssocItemKind::MacCall(ref mac) => {
self.check_attributes(&item.attrs, &mac);
item.and_then(|item| match item.kind {
ast::AssocItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::ImplItems)
Expand Down Expand Up @@ -1526,8 +1543,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

match foreign_item.kind {
ast::ForeignItemKind::MacCall(..) => {
self.check_attributes(&foreign_item.attrs);
ast::ForeignItemKind::MacCall(ref mac) => {
self.check_attributes(&foreign_item.attrs, &mac);
foreign_item.and_then(|item| match item.kind {
ast::ForeignItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::ForeignItems)
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_lint/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,16 @@ pub trait LintContext: Sized {
Applicability::MachineApplicable,
);
}
BuiltinLintDiagnostics::UnusedBuiltinAttribute {
attr_name,
macro_name,
invoc_span
} => {
db.span_note(
invoc_span,
&format!("the built-in attribute `{attr_name}` will be ignored, since it's applied to the macro invocation `{macro_name}`")
);
}
}
// Rewrap `db`, and pass control to the user.
decorate(LintDiagnosticBuilder::new(db));
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#![feature(box_syntax)]
#![feature(box_patterns)]
#![feature(crate_visibility_modifier)]
#![feature(format_args_capture)]
#![feature(iter_order_by)]
#![feature(iter_zip)]
#![feature(never_type)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint_defs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ pub enum BuiltinLintDiagnostics {
DeprecatedMacro(Option<Symbol>, Span),
MissingAbi(Span, Abi),
UnusedDocComment(Span),
UnusedBuiltinAttribute { attr_name: Symbol, macro_name: String, invoc_span: Span },
PatternsInFnsWithoutBody(Span, Ident),
LegacyDeriveHelpers(Span),
ExternDepSpec(String, ExternDepSpec),
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/lint/inert-attr-macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// check-pass

#![warn(unused)]

macro_rules! foo {
() => {}
}

fn main() {
#[inline] foo!(); //~ WARN unused attribute `inline`

// This does nothing, since `#[allow(warnings)]` is itself
// an inert attribute on a macro call
#[allow(warnings)] #[inline] foo!(); //~ WARN unused attribute `allow`
//~^ WARN unused attribute `inline`

// This does work, since the attribute is on a parent
// of the macro invocation.
#[allow(warnings)] { #[inline] foo!(); }
}
44 changes: 44 additions & 0 deletions src/test/ui/lint/inert-attr-macro.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
warning: unused attribute `inline`
--> $DIR/inert-attr-macro.rs:10:5
|
LL | #[inline] foo!();
| ^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/inert-attr-macro.rs:3:9
|
LL | #![warn(unused)]
| ^^^^^^
= note: `#[warn(unused_attributes)]` implied by `#[warn(unused)]`
note: the built-in attribute `inline` will be ignored, since it's applied to the macro invocation `foo`
--> $DIR/inert-attr-macro.rs:10:15
|
LL | #[inline] foo!();
| ^^^

warning: unused attribute `allow`
--> $DIR/inert-attr-macro.rs:14:5
|
LL | #[allow(warnings)] #[inline] foo!();
| ^^^^^^^^^^^^^^^^^^
|
note: the built-in attribute `allow` will be ignored, since it's applied to the macro invocation `foo`
--> $DIR/inert-attr-macro.rs:14:34
|
LL | #[allow(warnings)] #[inline] foo!();
| ^^^

warning: unused attribute `inline`
--> $DIR/inert-attr-macro.rs:14:24
|
LL | #[allow(warnings)] #[inline] foo!();
| ^^^^^^^^^
|
note: the built-in attribute `inline` will be ignored, since it's applied to the macro invocation `foo`
--> $DIR/inert-attr-macro.rs:14:34
|
LL | #[allow(warnings)] #[inline] foo!();
| ^^^

warning: 3 warnings emitted

14 changes: 6 additions & 8 deletions src/test/ui/repr/repr-no-niche.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ mod enum_inline {
// general; this test is relying on that.)
two_fifty_six_variant_enum!(Visible2, N8);

#[repr(no_niche)]
two_fifty_six_variant_enum!(Cloaked2, N8);
two_fifty_six_variant_enum!(#[repr(no_niche)] Cloaked2, N8);
}

mod enum_param {
Expand All @@ -96,8 +95,7 @@ mod enum_param {
// here as above (assuming `T` is instantiated with `NonZeroU8`).
two_fifty_six_variant_enum!(Visible2<T>);

#[repr(no_niche)]
two_fifty_six_variant_enum!(Cloaked2<T>);
two_fifty_six_variant_enum!(#[repr(no_niche)] Cloaked2<T>);
}

fn main() {
Expand Down Expand Up @@ -157,8 +155,8 @@ fn main() {
}

macro two_fifty_six_variant_enum {
($name:ident<$param:ident>) => {
#[derive(Debug)]
($(#[$attr:meta])* $name:ident<$param:ident>) => {
#[derive(Debug)] $(#[$attr])*
pub enum $name<$param> {
_V00($param, u16), _V01(u16, $param), _V02($param, u16), _V03(u16, $param),
_V04($param, u16), _V05(u16, $param), _V06($param, u16), _V07(u16, $param),
Expand Down Expand Up @@ -242,8 +240,8 @@ macro two_fifty_six_variant_enum {
}
},

($name:ident, $param:ty) => {
#[derive(Debug)]
($(#[$attr:meta])* $name:ident, $param:ty) => {
#[derive(Debug)] $(#[$attr])*
pub enum $name {
_V00($param, u16), _V01(u16, $param), _V02($param, u16), _V03(u16, $param),
_V04($param, u16), _V05(u16, $param), _V06($param, u16), _V07(u16, $param),
Expand Down