Auto merge of #87296 - Aaron1011:inert-warn, r=petrochenkov

Warn on inert attributes used on bang macro invocation These attributes are currently discarded. This may change in the future (see #63221), but for now, placing inert attributes on a macro invocation does nothing, so we should warn users about it. Technically, it's possible for there to be attribute macro on the same macro invocation (or at a higher scope), which inspects the inert attribute. For example: ```rust #[look_for_inline_attr] #[inline] my_macro!() #[look_for_nested_inline] mod foo { #[inline] my_macro!() } ``` However, this would be a very strange thing to do. Anyone running into this can manually suppress the warning.
rust-lang · Jul 24, 2021 · 18840b0 · 18840b0
2 parents f9b95f9 + b41672e
commit 18840b0
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 24 deletions.
diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs
@@ -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;
@@ -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;
@@ -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;
@@ -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 {
+                    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,
+                        },
+                    );
+                }
             }
         }
     }
@@ -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, || {
@@ -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())
@@ -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();
 
@@ -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()
@@ -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)
@@ -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)
@@ -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)

diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs
@@ -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));

diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs
@@ -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)]

diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs
@@ -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),

diff --git a/src/test/ui/lint/inert-attr-macro.rs b/src/test/ui/lint/inert-attr-macro.rs
@@ -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!(); }
+}
diff --git a/src/test/ui/lint/inert-attr-macro.stderr b/src/test/ui/lint/inert-attr-macro.stderr
@@ -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
+
diff --git a/src/test/ui/repr/repr-no-niche.rs b/src/test/ui/repr/repr-no-niche.rs
@@ -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 {
@@ -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() {
@@ -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),
@@ -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),