Rollup merge of #150971 - disallow-eii-in-statement-position, r=wafflelapkin

Disallow eii in statement position

With how v2 macros resolve, and the name resolution of `super` works, I realized with @WaffleLapkin that there's actually no way to consistently expand EIIs in statement position.

r? @WaffleLapkin

Author: JonathanBrouwer

compiler/rustc_builtin_macros/messages.ftl

@@ -161,6 +161,8 @@ builtin_macros_eii_only_once = `#[{$name}]` can only be specified once
 
 builtin_macros_eii_shared_macro_expected_function = `#[{$name}]` is only valid on functions
 builtin_macros_eii_shared_macro_expected_max_one_argument = `#[{$name}]` expected no arguments or a single argument: `#[{$name}(default)]`
+builtin_macros_eii_shared_macro_in_statement_position = `#[{$name}]` can only be used on functions inside a module
+    .label = `#[{$name}]` is used on this item, which is part of another item's local scope
 
 builtin_macros_env_not_defined = environment variable `{$var}` not defined at compile time
     .cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead

compiler/rustc_builtin_macros/src/eii.rs

@@ -1,8 +1,7 @@
 use rustc_ast::token::{Delimiter, TokenKind};
 use rustc_ast::tokenstream::{DelimSpacing, DelimSpan, Spacing, TokenStream, TokenTree};
 use rustc_ast::{
-    Attribute, DUMMY_NODE_ID, EiiDecl, EiiImpl, ItemKind, MetaItem, Path, Stmt, StmtKind,
-    Visibility, ast,
+    Attribute, DUMMY_NODE_ID, EiiDecl, EiiImpl, ItemKind, MetaItem, Path, StmtKind, Visibility, ast,
 };
 use rustc_ast_pretty::pprust::path_to_string;
 use rustc_expand::base::{Annotatable, ExtCtxt};
@@ -12,6 +11,7 @@ use thin_vec::{ThinVec, thin_vec};
 use crate::errors::{
     EiiExternTargetExpectedList, EiiExternTargetExpectedMacro, EiiExternTargetExpectedUnsafe,
     EiiMacroExpectedMaxOneArgument, EiiOnlyOnce, EiiSharedMacroExpectedFunction,
+    EiiSharedMacroInStatementPosition,
 };
 
 /// ```rust
@@ -55,29 +55,29 @@ fn eii_(
     ecx: &mut ExtCtxt<'_>,
     eii_attr_span: Span,
     meta_item: &ast::MetaItem,
-    item: Annotatable,
+    orig_item: Annotatable,
     impl_unsafe: bool,
 ) -> Vec<Annotatable> {
     let eii_attr_span = ecx.with_def_site_ctxt(eii_attr_span);
 
-    let (item, wrap_item): (_, &dyn Fn(_) -> _) = if let Annotatable::Item(item) = item {
-        (item, &Annotatable::Item)
-    } else if let Annotatable::Stmt(ref stmt) = item
+    let item = if let Annotatable::Item(item) = orig_item {
+        item
+    } else if let Annotatable::Stmt(ref stmt) = orig_item
         && let StmtKind::Item(ref item) = stmt.kind
+        && let ItemKind::Fn(ref f) = item.kind
     {
-        (item.clone(), &|item| {
-            Annotatable::Stmt(Box::new(Stmt {
-                id: DUMMY_NODE_ID,
-                kind: StmtKind::Item(item),
-                span: eii_attr_span,
-            }))
-        })
+        ecx.dcx().emit_err(EiiSharedMacroInStatementPosition {
+            span: eii_attr_span.to(item.span),
+            name: path_to_string(&meta_item.path),
+            item_span: f.ident.span,
+        });
+        return vec![orig_item];
     } else {
         ecx.dcx().emit_err(EiiSharedMacroExpectedFunction {
             span: eii_attr_span,
             name: path_to_string(&meta_item.path),
         });
-        return vec![item];
+        return vec![orig_item];
     };
 
     let ast::Item { attrs, id: _, span: _, vis, kind: ItemKind::Fn(func), tokens: _ } =
@@ -87,7 +87,7 @@ fn eii_(
             span: eii_attr_span,
             name: path_to_string(&meta_item.path),
         });
-        return vec![wrap_item(item)];
+        return vec![Annotatable::Item(item)];
     };
     // only clone what we need
     let attrs = attrs.clone();
@@ -98,17 +98,19 @@ fn eii_(
         filter_attrs_for_multiple_eii_attr(ecx, attrs, eii_attr_span, &meta_item.path);
 
     let Ok(macro_name) = name_for_impl_macro(ecx, &func, &meta_item) else {
-        return vec![wrap_item(item)];
+        // we don't need to wrap in Annotatable::Stmt conditionally since
+        // EII can't be used on items in statement position
+        return vec![Annotatable::Item(item)];
     };
 
     // span of the declaring item without attributes
     let item_span = func.sig.span;
     let foreign_item_name = func.ident;
 
-    let mut return_items = Vec::new();
+    let mut module_items = Vec::new();
 
     if func.body.is_some() {
-        return_items.push(generate_default_impl(
+        module_items.push(generate_default_impl(
             ecx,
             &func,
             impl_unsafe,
@@ -119,15 +121,15 @@ fn eii_(
         ))
     }
 
-    return_items.push(generate_foreign_item(
+    module_items.push(generate_foreign_item(
         ecx,
         eii_attr_span,
         item_span,
         func,
         vis,
         &attrs_from_decl,
     ));
-    return_items.push(generate_attribute_macro_to_implement(
+    module_items.push(generate_attribute_macro_to_implement(
         ecx,
         eii_attr_span,
         macro_name,
@@ -136,7 +138,9 @@ fn eii_(
         &attrs_from_decl,
     ));
 
-    return_items.into_iter().map(wrap_item).collect()
+    // we don't need to wrap in Annotatable::Stmt conditionally since
+    // EII can't be used on items in statement position
+    module_items.into_iter().map(Annotatable::Item).collect()
 }
 
 /// Decide on the name of the macro that can be used to implement the EII.
@@ -213,29 +217,14 @@ fn generate_default_impl(
         known_eii_macro_resolution: Some(ast::EiiDecl {
             foreign_item: ecx.path(
                 foreign_item_name.span,
-                // prefix super to escape the `dflt` module generated below
-                vec![Ident::from_str_and_span("super", foreign_item_name.span), foreign_item_name],
+                // prefix self to explicitly escape the const block generated below
+                // NOTE: this is why EIIs can't be used on statements
+                vec![Ident::from_str_and_span("self", foreign_item_name.span), foreign_item_name],
             ),
             impl_unsafe,
         }),
     });
 
-    let item_mod = |span: Span, name: Ident, items: ThinVec<Box<ast::Item>>| {
-        ecx.item(
-            item_span,
-            ThinVec::new(),
-            ItemKind::Mod(
-                ast::Safety::Default,
-                name,
-                ast::ModKind::Loaded(
-                    items,
-                    ast::Inline::Yes,
-                    ast::ModSpans { inner_span: span, inject_use_span: span },
-                ),
-            ),
-        )
-    };
-
     let anon_mod = |span: Span, stmts: ThinVec<ast::Stmt>| {
         let unit = ecx.ty(item_span, ast::TyKind::Tup(ThinVec::new()));
         let underscore = Ident::new(kw::Underscore, item_span);
@@ -248,33 +237,13 @@ fn generate_default_impl(
     };
 
     // const _: () = {
-    //     mod dflt {
-    //         use super::*;
-    //         <orig fn>
-    //     }
+    //     <orig fn>
     // }
     anon_mod(
         item_span,
         thin_vec![ecx.stmt_item(
             item_span,
-            item_mod(
-                item_span,
-                Ident::from_str_and_span("dflt", item_span),
-                thin_vec![
-                    ecx.item(
-                        item_span,
-                        thin_vec![ecx.attr_nested_word(sym::allow, sym::unused_imports, item_span)],
-                        ItemKind::Use(ast::UseTree {
-                            prefix: ast::Path::from_ident(Ident::from_str_and_span(
-                                "super", item_span,
-                            )),
-                            kind: ast::UseTreeKind::Glob,
-                            span: item_span,
-                        })
-                    ),
-                    ecx.item(item_span, attrs, ItemKind::Fn(Box::new(default_func)))
-                ]
-            )
+            ecx.item(item_span, attrs, ItemKind::Fn(Box::new(default_func)))
         ),],
     )
 }

compiler/rustc_builtin_macros/src/errors.rs

@@ -1039,6 +1039,16 @@ pub(crate) struct EiiSharedMacroExpectedFunction {
     pub name: String,
 }
 
+#[derive(Diagnostic)]
+#[diag(builtin_macros_eii_shared_macro_in_statement_position)]
+pub(crate) struct EiiSharedMacroInStatementPosition {
+    #[primary_span]
+    pub span: Span,
+    pub name: String,
+    #[label]
+    pub item_span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(builtin_macros_eii_only_once)]
 pub(crate) struct EiiOnlyOnce {

tests/ui/eii/error_statement_position.rs

@@ -1,16 +1,17 @@
 #![feature(extern_item_impls)]
-// EIIs can, despite not being super useful, be declared in statement position
-// nested inside items. Items in statement position, when expanded as part of a macro,
-// need to be wrapped slightly differently (in an `ast::Statement`).
-// We did this on the happy path (no errors), but when there was an error, we'd
-// replace it with *just* an `ast::Item` not wrapped in an `ast::Statement`.
-// This caused an ICE (https://github.com/rust-lang/rust/issues/149980).
-// this test fails to build, but demonstrates that no ICE is produced.
+// EIIs cannot be used in statement position.
+// This is also a regression test for an ICE (https://github.com/rust-lang/rust/issues/149980).
 
 fn main() {
     struct Bar;
 
     #[eii]
     //~^ ERROR `#[eii]` is only valid on functions
     impl Bar {}
+
+
+    // Even on functions, eiis in statement position are rejected
+    #[eii]
+    //~^ ERROR `#[eii]` can only be used on functions inside a module
+    fn foo() {}
 }

tests/ui/eii/error_statement_position.stderr

@@ -1,8 +1,17 @@
 error: `#[eii]` is only valid on functions
-  --> $DIR/error_statement_position.rs:13:5
+  --> $DIR/error_statement_position.rs:8:5
    |
 LL |     #[eii]
    |     ^^^^^^
 
-error: aborting due to 1 previous error
+error: `#[eii]` can only be used on functions inside a module
+  --> $DIR/error_statement_position.rs:14:5
+   |
+LL |     #[eii]
+   |     ^^^^^^
+LL |
+LL |     fn foo() {}
+   |        --- `#[eii]` is used on this item, which is part of another item's local scope
+
+error: aborting due to 2 previous errors