[EXPERIMENT] Forbid derive attributes on invalid targets

compiler/rustc_ast/src/attr/mod.rs

@@ -623,7 +623,8 @@ impl HasAttrs for StmtKind {
         match *self {
             StmtKind::Local(ref local) => local.attrs(),
             StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => expr.attrs(),
-            StmtKind::Empty | StmtKind::Item(..) => &[],
+            StmtKind::Item(ref item) => item.attrs(),
+            StmtKind::Empty => &[],
             StmtKind::MacCall(ref mac) => mac.attrs.attrs(),
         }
     }
@@ -632,7 +633,8 @@ impl HasAttrs for StmtKind {
         match self {
             StmtKind::Local(local) => local.visit_attrs(f),
             StmtKind::Expr(expr) | StmtKind::Semi(expr) => expr.visit_attrs(f),
-            StmtKind::Empty | StmtKind::Item(..) => {}
+            StmtKind::Item(item) => item.visit_attrs(f),
+            StmtKind::Empty => {}
             StmtKind::MacCall(mac) => {
                 mac.attrs.visit_attrs(f);
             }

compiler/rustc_expand/src/expand.rs

@@ -47,6 +47,7 @@ macro_rules! ast_fragments {
     ) => {
         /// A fragment of AST that can be produced by a single macro expansion.
         /// Can also serve as an input and intermediate result for macro expansion operations.
+        #[derive(Debug)]
         pub enum AstFragment {
             OptExpr(Option<P<ast::Expr>>),
             $($Kind($AstTy),)*
@@ -88,7 +89,7 @@ macro_rules! ast_fragments {
                         macro _repeating($flat_map_ast_elt) {}
                         placeholder(AstFragmentKind::$Kind, *id, None).$make_ast()
                     })),)?)*
-                    _ => panic!("unexpected AST fragment kind")
+                    _ => panic!("unexpected AST fragment kind: {:?}", self)
                 }
             }
 
@@ -483,12 +484,12 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 InvocationRes::DeriveContainer(_exts) => {
                     // FIXME: Consider using the derive resolutions (`_exts`) immediately,
                     // instead of enqueuing the derives to be resolved again later.
-                    let (derives, item) = match invoc.kind {
+                    let (mut derives, item) = match invoc.kind {
                         InvocationKind::DeriveContainer { derives, item } => (derives, item),
                         _ => unreachable!(),
                     };
                     if !item.derive_allowed() {
-                        self.error_derive_forbidden_on_non_adt(&derives, &item);
+                        self.error_derive_forbidden_on_non_adt(&mut derives, &item);
                     }
 
                     let mut item = self.fully_configure(item);
@@ -539,7 +540,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
         fragment_with_placeholders
     }
 
-    fn error_derive_forbidden_on_non_adt(&self, derives: &[Path], item: &Annotatable) {
+    fn error_derive_forbidden_on_non_adt(&self, derives: &mut Vec<Path>, item: &Annotatable) {
         let attr = self.cx.sess.find_by_name(item.attrs(), sym::derive);
         let span = attr.map_or(item.span(), |attr| attr.span);
         let mut err = struct_span_err!(
@@ -560,6 +561,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
             );
         }
         err.emit();
+        *derives = Vec::new();
     }
 
     fn resolve_imports(&mut self) {
@@ -1097,22 +1099,6 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
         (attr, traits, after_derive)
     }
 
-    /// Alternative to `classify_item()` that ignores `#[derive]` so invocations fallthrough
-    /// to the unused-attributes lint (making it an error on statements and expressions
-    /// is a breaking change)
-    fn classify_nonitem(
-        &mut self,
-        nonitem: &mut impl HasAttrs,
-    ) -> (Option<ast::Attribute>, /* after_derive */ bool) {
-        let (mut attr, mut after_derive) = (None, false);
-
-        nonitem.visit_attrs(|mut attrs| {
-            attr = self.find_attr_invoc(&mut attrs, &mut after_derive);
-        });
-
-        (attr, after_derive)
-    }
-
     fn configure<T: HasAttrs>(&mut self, node: T) -> Option<T> {
         self.cfg.configure(node)
     }
@@ -1154,19 +1140,20 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
         visit_clobber(expr.deref_mut(), |mut expr| {
             self.cfg.configure_expr_kind(&mut expr.kind);
 
-            // ignore derives so they remain unused
-            let (attr, after_derive) = self.classify_nonitem(&mut expr);
+            let (attr, derives, after_derive) = self.classify_item(&mut expr);
 
-            if let Some(ref attr_value) = attr {
-                // Collect the invoc regardless of whether or not attributes are permitted here
-                // expansion will eat the attribute so it won't error later.
-                self.cfg.maybe_emit_expr_attr_err(attr_value);
+            if attr.is_some() || !derives.is_empty() {
+                if let Some(attr) = &attr {
+                    // Collect the invoc regardless of whether or not attributes are permitted here
+                    // expansion will eat the attribute so it won't error later.
+                    self.cfg.maybe_emit_expr_attr_err(&attr);
+                }
 
                 // AstFragmentKind::Expr requires the macro to emit an expression.
                 return self
                     .collect_attr(
                         attr,
-                        vec![],
+                        derives,
                         Annotatable::Expr(P(expr)),
                         AstFragmentKind::Expr,
                         after_derive,
@@ -1304,16 +1291,17 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
         expr.filter_map(|mut expr| {
             self.cfg.configure_expr_kind(&mut expr.kind);
 
-            // Ignore derives so they remain unused.
-            let (attr, after_derive) = self.classify_nonitem(&mut expr);
+            let (attr, derives, after_derive) = self.classify_item(&mut expr);
 
-            if let Some(ref attr_value) = attr {
-                self.cfg.maybe_emit_expr_attr_err(attr_value);
+            if attr.is_some() || !derives.is_empty() {
+                if let Some(attr) = &attr {
+                    self.cfg.maybe_emit_expr_attr_err(attr);
+                }
 
                 return self
                     .collect_attr(
                         attr,
-                        vec![],
+                        derives,
                         Annotatable::Expr(P(expr)),
                         AstFragmentKind::OptExpr,
                         after_derive,
@@ -1357,12 +1345,11 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
         // we'll expand attributes on expressions separately
         if !stmt.is_expr() {
             let (attr, derives, after_derive) = if stmt.is_item() {
-                self.classify_item(&mut stmt)
+                // FIXME: Handle custom attributes on statements (#15701)
+                (None, vec![], false)
             } else {
                 // ignore derives on non-item statements so it falls through
-                // to the unused-attributes lint
-                let (attr, after_derive) = self.classify_nonitem(&mut stmt);
-                (attr, vec![], after_derive)
+                self.classify_item(&mut stmt)
             };
 
             if attr.is_some() || !derives.is_empty() {

compiler/rustc_hir/src/hir.rs

@@ -1099,11 +1099,11 @@ pub enum StmtKind<'hir> {
     Semi(&'hir Expr<'hir>),
 }
 
-impl StmtKind<'hir> {
-    pub fn attrs(&self) -> &'hir [Attribute] {
+impl<'hir> StmtKind<'hir> {
+    pub fn attrs(&self, get_item: impl FnOnce(ItemId) -> &'hir Item<'hir>) -> &'hir [Attribute] {
         match *self {
             StmtKind::Local(ref l) => &l.attrs,
-            StmtKind::Item(_) => &[],
+            StmtKind::Item(ref item_id) => &get_item(*item_id).attrs,
             StmtKind::Expr(ref e) | StmtKind::Semi(ref e) => &e.attrs,
         }
     }

compiler/rustc_lint/src/builtin.rs

@@ -994,7 +994,8 @@ impl EarlyLintPass for UnusedDocComment {
     fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) {
         let kind = match stmt.kind {
             ast::StmtKind::Local(..) => "statements",
-            ast::StmtKind::Item(..) => "inner items",
+            // Disabled pending discussion in #78306
+            ast::StmtKind::Item(..) => return,
             // expressions will be reported by `check_expr`.
             ast::StmtKind::Empty
             | ast::StmtKind::Semi(_)

compiler/rustc_lint/src/early.rs

@@ -18,6 +18,7 @@ use crate::context::{EarlyContext, LintContext, LintStore};
 use crate::passes::{EarlyLintPass, EarlyLintPassObject};
 use rustc_ast as ast;
 use rustc_ast::visit as ast_visit;
+use rustc_attr::HasAttrs;
 use rustc_session::lint::{BufferedEarlyLint, LintBuffer, LintPass};
 use rustc_session::Session;
 use rustc_span::symbol::Ident;
@@ -119,8 +120,22 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
     }
 
     fn visit_stmt(&mut self, s: &'a ast::Stmt) {
-        run_early_pass!(self, check_stmt, s);
-        self.check_id(s.id);
+        // Add the statement's lint attributes to our
+        // current state when checking the statement itself.
+        // This allows us to handle attributes like
+        // `#[allow(unused_doc_comments)]`, which apply to
+        // sibling attributes on the same target
+        //
+        // Note that statements get their attributes from
+        // the AST struct that they wrap (e.g. an item)
+        self.with_lint_attrs(s.id, s.attrs(), |cx| {
+            run_early_pass!(cx, check_stmt, s);
+            cx.check_id(s.id);
+        });
+        // The visitor for the AST struct wrapped
+        // by the statement (e.g. `Item`) will call
+        // `with_lint_attrs`, so do this walk
+        // outside of the above `with_lint_attrs` call
         ast_visit::walk_stmt(self, s);
     }
 

compiler/rustc_lint/src/late.rs

@@ -174,12 +174,13 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
     }
 
     fn visit_stmt(&mut self, s: &'tcx hir::Stmt<'tcx>) {
-        // statement attributes are actually just attributes on one of
-        // - item
-        // - local
-        // - expression
-        // so we keep track of lint levels there
-        lint_callback!(self, check_stmt, s);
+        let get_item = |id: hir::ItemId| self.context.tcx.hir().item(id.id);
+        let attrs = &s.kind.attrs(get_item);
+        // See `EarlyContextAndPass::visit_stmt` for an explanation
+        // of why we call `walk_stmt` outside of `with_lint_attrs`
+        self.with_lint_attrs(s.hir_id, attrs, |cx| {
+            lint_callback!(cx, check_stmt, s);
+        });
         hir_visit::walk_stmt(self, s);
     }
 

compiler/rustc_lint/src/levels.rs

@@ -562,6 +562,13 @@ impl<'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'_, 'tcx> {
         })
     }
 
+    fn visit_stmt(&mut self, e: &'tcx hir::Stmt<'tcx>) {
+        // We will call `with_lint_attrs` when we walk
+        // the `StmtKind`. The outer statement itself doesn't
+        // define the lint levels.
+        intravisit::walk_stmt(self, e);
+    }
+
     fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) {
         self.with_lint_attrs(e.hir_id, &e.attrs, |builder| {
             intravisit::walk_expr(builder, e);

compiler/rustc_middle/src/hir/map/mod.rs

@@ -816,7 +816,7 @@ impl<'hir> Map<'hir> {
             Some(Node::Variant(ref v)) => Some(&v.attrs[..]),
             Some(Node::Field(ref f)) => Some(&f.attrs[..]),
             Some(Node::Expr(ref e)) => Some(&*e.attrs),
-            Some(Node::Stmt(ref s)) => Some(s.kind.attrs()),
+            Some(Node::Stmt(ref s)) => Some(s.kind.attrs(|id| self.item(id.id))),
             Some(Node::Arm(ref a)) => Some(&*a.attrs),
             Some(Node::GenericParam(param)) => Some(&param.attrs[..]),
             // Unit/tuple structs/variants take the attributes straight from

src/test/ui/issues/issue-36617.rs

@@ -1,5 +1,4 @@
 #![derive(Copy)] //~ ERROR `derive` may only be applied to structs, enums and unions
                  //~| ERROR cannot determine resolution for the derive macro `Copy`
-                 //~| ERROR cannot determine resolution for the derive macro `Copy`
 
 fn main() {}

src/test/ui/issues/issue-36617.stderr

@@ -12,14 +12,6 @@ LL | #![derive(Copy)]
    |
    = note: import resolution is stuck, try simplifying macro imports
 
-error: cannot determine resolution for the derive macro `Copy`
-  --> $DIR/issue-36617.rs:1:11
-   |
-LL | #![derive(Copy)]
-   |           ^^^^
-   |
-   = note: import resolution is stuck, try simplifying macro imports
-
-error: aborting due to 3 previous errors
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0774`.

src/test/ui/issues/issue-49934-errors.rs

@@ -1,10 +1,8 @@
 fn foo<#[derive(Debug)] T>() {
 //~^ ERROR `derive` may only be applied to structs, enums and unions
-//~| ERROR expected an inert attribute, found a derive macro
     match 0 {
         #[derive(Debug)]
         //~^ ERROR `derive` may only be applied to structs, enums and unions
-        //~| ERROR expected an inert attribute, found a derive macro
         _ => (),
     }
 }

src/test/ui/issues/issue-49934-errors.stderr

@@ -4,24 +4,12 @@ error[E0774]: `derive` may only be applied to structs, enums and unions
 LL | fn foo<#[derive(Debug)] T>() {
    |        ^^^^^^^^^^^^^^^^
 
-error: expected an inert attribute, found a derive macro
-  --> $DIR/issue-49934-errors.rs:1:17
-   |
-LL | fn foo<#[derive(Debug)] T>() {
-   |                 ^^^^^
-
 error[E0774]: `derive` may only be applied to structs, enums and unions
-  --> $DIR/issue-49934-errors.rs:5:9
+  --> $DIR/issue-49934-errors.rs:4:9
    |
 LL |         #[derive(Debug)]
    |         ^^^^^^^^^^^^^^^^
 
-error: expected an inert attribute, found a derive macro
-  --> $DIR/issue-49934-errors.rs:5:18
-   |
-LL |         #[derive(Debug)]
-   |                  ^^^^^
-
-error: aborting due to 4 previous errors
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0774`.

src/test/ui/issues/issue-49934.rs

@@ -1,7 +1,5 @@
-// check-pass
-
 #![feature(stmt_expr_attributes)]
-#![warn(unused_attributes)] //~ NOTE the lint level is defined here
+#![warn(unused_attributes)]
 
 fn main() {
     // fold_stmt (Item)
@@ -11,25 +9,24 @@ fn main() {
 
     // fold_stmt (Mac)
     #[derive(Debug)]
-    //~^ WARN `#[derive]` does nothing on macro invocations
-    //~| NOTE this may become a hard error in a future release
+    //~^ ERROR `derive` may only
     println!("Hello, world!");
 
     // fold_stmt (Semi)
-    #[derive(Debug)] //~ WARN unused attribute
+    #[derive(Debug)] //~ ERROR `derive` may only
     "Hello, world!";
 
     // fold_stmt (Local)
-    #[derive(Debug)] //~ WARN unused attribute
+    #[derive(Debug)] //~ ERROR `derive` may only
     let _ = "Hello, world!";
 
     // visit_expr
     let _ = #[derive(Debug)] "Hello, world!";
-    //~^ WARN unused attribute
+    //~^ ERROR `derive` may only
 
     let _ = [
         // filter_map_expr
-        #[derive(Debug)] //~ WARN unused attribute
+        #[derive(Debug)] //~ ERROR `derive` may only
         "Hello, world!"
     ];
 }