Auto merge of #83488 - Aaron1011:ban-expr-inner-attrs, r=petrochenkov

Ban custom inner attributes in expressions and statements

Split out from #82608

Custom inner attributes are unstable, so this won't break any stable users.
This allows us to speed up token collection, and avoid a redundant call to `collect_tokens_no_attrs` when parsing an `Expr` that has outer attributes.

r? `@petrochenkov`

Author: bors

compiler/rustc_expand/src/expand.rs

@@ -206,30 +206,36 @@ ast_fragments! {
     }
 }
 
+pub enum SupportsMacroExpansion {
+    No,
+    Yes { supports_inner_attrs: bool },
+}
+
 impl AstFragmentKind {
     crate fn dummy(self, span: Span) -> AstFragment {
         self.make_from(DummyResult::any(span)).expect("couldn't create a dummy AST fragment")
     }
 
-    /// Fragment supports macro expansion and not just inert attributes, `cfg` and `cfg_attr`.
-    pub fn supports_macro_expansion(self) -> bool {
+    pub fn supports_macro_expansion(self) -> SupportsMacroExpansion {
         match self {
             AstFragmentKind::OptExpr
             | AstFragmentKind::Expr
-            | AstFragmentKind::Pat
-            | AstFragmentKind::Ty
             | AstFragmentKind::Stmts
-            | AstFragmentKind::Items
+            | AstFragmentKind::Ty
+            | AstFragmentKind::Pat => SupportsMacroExpansion::Yes { supports_inner_attrs: false },
+            AstFragmentKind::Items
             | AstFragmentKind::TraitItems
             | AstFragmentKind::ImplItems
-            | AstFragmentKind::ForeignItems => true,
+            | AstFragmentKind::ForeignItems => {
+                SupportsMacroExpansion::Yes { supports_inner_attrs: true }
+            }
             AstFragmentKind::Arms
             | AstFragmentKind::Fields
             | AstFragmentKind::FieldPats
             | AstFragmentKind::GenericParams
             | AstFragmentKind::Params
             | AstFragmentKind::StructFields
-            | AstFragmentKind::Variants => false,
+            | AstFragmentKind::Variants => SupportsMacroExpansion::No,
         }
     }
 

compiler/rustc_parse/src/parser/expr.rs

@@ -92,6 +92,21 @@ impl<'a> Parser<'a> {
         self.parse_expr_res(Restrictions::empty(), None)
     }
 
+    /// Parses an expression, forcing tokens to be collected
+    pub fn parse_expr_force_collect(&mut self) -> PResult<'a, P<Expr>> {
+        // If we have outer attributes, then the call to `collect_tokens_trailing_token`
+        // will be made for us.
+        if matches!(self.token.kind, TokenKind::Pound | TokenKind::DocComment(..)) {
+            self.parse_expr()
+        } else {
+            // If we don't have outer attributes, then we need to ensure
+            // that collection happens by using `collect_tokens_no_attrs`.
+            // Expression don't support custom inner attributes, so `parse_expr`
+            // will never try to collect tokens if we don't have outer attributes.
+            self.collect_tokens_no_attrs(|this| this.parse_expr())
+        }
+    }
+
     pub(super) fn parse_anon_const_expr(&mut self) -> PResult<'a, AnonConst> {
         self.parse_expr().map(|value| AnonConst { id: DUMMY_NODE_ID, value })
     }

compiler/rustc_parse/src/parser/mod.rs

@@ -987,7 +987,7 @@ impl<'a> Parser<'a> {
                     }
 
                     // Collect tokens because they are used during lowering to HIR.
-                    let expr = self.collect_tokens_no_attrs(|this| this.parse_expr())?;
+                    let expr = self.parse_expr_force_collect()?;
                     let span = expr.span;
 
                     match &expr.kind {

compiler/rustc_parse/src/parser/nonterminal.rs

@@ -128,22 +128,7 @@ impl<'a> Parser<'a> {
                 })?)
             }
 
-            // If there are attributes present, then `parse_expr` will end up collecting tokens,
-            // turning the outer `collect_tokens_no_attrs` into a no-op due to the already present
-            // tokens. If there are *not* attributes present, then the outer
-            // `collect_tokens_no_attrs` will ensure that we will end up collecting tokens for the
-            // expressions.
-            //
-            // This is less efficient than it could be, since the outer `collect_tokens_no_attrs`
-            // still needs to snapshot the `TokenCursor` before calling `parse_expr`, even when
-            // `parse_expr` will end up collecting tokens. Ideally, this would work more like
-            // `parse_item`, and take in a `ForceCollect` parameter. However, this would require
-            // adding a `ForceCollect` parameter in a bunch of places in expression parsing
-            // for little gain. If the perf impact from this turns out to be noticeable, we should
-            // revisit this apporach.
-            NonterminalKind::Expr => {
-                token::NtExpr(self.collect_tokens_no_attrs(|this| this.parse_expr())?)
-            }
+            NonterminalKind::Expr => token::NtExpr(self.parse_expr_force_collect()?),
             NonterminalKind::Literal => {
                 // The `:literal` matcher does not support attributes
                 token::NtLiteral(

compiler/rustc_resolve/src/macros.rs

@@ -17,7 +17,7 @@ use rustc_errors::struct_span_err;
 use rustc_expand::base::Annotatable;
 use rustc_expand::base::{Indeterminate, ResolverExpand, SyntaxExtension, SyntaxExtensionKind};
 use rustc_expand::compile_declarative_macro;
-use rustc_expand::expand::{AstFragment, Invocation, InvocationKind};
+use rustc_expand::expand::{AstFragment, Invocation, InvocationKind, SupportsMacroExpansion};
 use rustc_feature::is_builtin_attr_name;
 use rustc_hir::def::{self, DefKind, NonMacroAttrKind};
 use rustc_hir::def_id;
@@ -278,12 +278,12 @@ impl<'a> ResolverExpand for Resolver<'a> {
 
         // Derives are not included when `invocations` are collected, so we have to add them here.
         let parent_scope = &ParentScope { derives, ..parent_scope };
-        let require_inert = !invoc.fragment_kind.supports_macro_expansion();
+        let supports_macro_expansion = invoc.fragment_kind.supports_macro_expansion();
         let node_id = self.lint_node_id(eager_expansion_root);
         let (ext, res) = self.smart_resolve_macro_path(
             path,
             kind,
-            require_inert,
+            supports_macro_expansion,
             inner_attr,
             parent_scope,
             node_id,
@@ -457,7 +457,7 @@ impl<'a> Resolver<'a> {
         &mut self,
         path: &ast::Path,
         kind: MacroKind,
-        require_inert: bool,
+        supports_macro_expansion: SupportsMacroExpansion,
         inner_attr: bool,
         parent_scope: &ParentScope<'a>,
         node_id: NodeId,
@@ -505,8 +505,17 @@ impl<'a> Resolver<'a> {
 
         let unexpected_res = if ext.macro_kind() != kind {
             Some((kind.article(), kind.descr_expected()))
-        } else if require_inert && matches!(res, Res::Def(..)) {
-            Some(("a", "non-macro attribute"))
+        } else if matches!(res, Res::Def(..)) {
+            match supports_macro_expansion {
+                SupportsMacroExpansion::No => Some(("a", "non-macro attribute")),
+                SupportsMacroExpansion::Yes { supports_inner_attrs } => {
+                    if inner_attr && !supports_inner_attrs {
+                        Some(("a", "non-macro inner attribute"))
+                    } else {
+                        None
+                    }
+                }
+            }
         } else {
             None
         };

src/test/ui/proc-macro/inner-attrs.rs

@@ -1,10 +1,10 @@
-// check-pass
 // compile-flags: -Z span-debug --error-format human
 // aux-build:test-macros.rs
 
 #![feature(custom_inner_attributes)]
 #![feature(proc_macro_hygiene)]
 #![feature(stmt_expr_attributes)]
+#![feature(rustc_attrs)]
 
 #![no_std] // Don't load unnecessary hygiene information from std
 extern crate std;
@@ -25,17 +25,34 @@ struct MyStruct {
 
 fn bar() {
     (#![print_target_and_args(fifth)] 1, 2);
+    //~^ ERROR expected non-macro inner attribute, found attribute macro
+
+    #[print_target_and_args(tuple_attrs)] (
+        #![cfg_attr(FALSE, rustc_dummy)]
+        3, 4, {
+            #![cfg_attr(not(FALSE), rustc_dummy(innermost))]
+            5
+        }
+    );
+
+    #[print_target_and_args(array_attrs)] [
+        #![rustc_dummy(inner)]
+        true; 0
+    ];
 
     [#![print_target_and_args(sixth)] 1 , 2];
+    //~^ ERROR expected non-macro inner attribute, found attribute macro
     [#![print_target_and_args(seventh)] true ; 5];
-
+    //~^ ERROR expected non-macro inner attribute, found attribute macro
 
     match 0 {
         #![print_target_and_args(eighth)]
+        //~^ ERROR expected non-macro inner attribute, found attribute macro
         _ => {}
     }
 
     MyStruct { #![print_target_and_args(ninth)] field: true };
+    //~^ ERROR expected non-macro inner attribute, found attribute macro
 }
 
 extern {

src/test/ui/proc-macro/inner-attrs.stderr

@@ -0,0 +1,32 @@
+error: expected non-macro inner attribute, found attribute macro `print_target_and_args`
+  --> $DIR/inner-attrs.rs:27:9
+   |
+LL |     (#![print_target_and_args(fifth)] 1, 2);
+   |         ^^^^^^^^^^^^^^^^^^^^^ not a non-macro inner attribute
+
+error: expected non-macro inner attribute, found attribute macro `print_target_and_args`
+  --> $DIR/inner-attrs.rs:43:9
+   |
+LL |     [#![print_target_and_args(sixth)] 1 , 2];
+   |         ^^^^^^^^^^^^^^^^^^^^^ not a non-macro inner attribute
+
+error: expected non-macro inner attribute, found attribute macro `print_target_and_args`
+  --> $DIR/inner-attrs.rs:45:9
+   |
+LL |     [#![print_target_and_args(seventh)] true ; 5];
+   |         ^^^^^^^^^^^^^^^^^^^^^ not a non-macro inner attribute
+
+error: expected non-macro inner attribute, found attribute macro `print_target_and_args`
+  --> $DIR/inner-attrs.rs:49:12
+   |
+LL |         #![print_target_and_args(eighth)]
+   |            ^^^^^^^^^^^^^^^^^^^^^ not a non-macro inner attribute
+
+error: expected non-macro inner attribute, found attribute macro `print_target_and_args`
+  --> $DIR/inner-attrs.rs:54:19
+   |
+LL |     MyStruct { #![print_target_and_args(ninth)] field: true };
+   |                   ^^^^^^^^^^^^^^^^^^^^^ not a non-macro inner attribute
+
+error: aborting due to 5 previous errors
+