Rollup merge of rust-lang#138613 - m-ou-se:no-more-e0773, r=jdonszelmann,petrochenkov

Remove E0773 "A builtin-macro was defined more than once."

Error E0773 "A builtin-macro was defined more than once" is triggered when using the same `#[rustc_builtin_macro(..)]` twice. However, it can only be triggered in unstable code (using a `rustc_` attribute), and there doesn't seem to be any harm in using the same implementation from `compiler/rustc_builtin_macros/…` for multiple macro definitions.

By changing the Box to an Arc in `SyntaxExtensionKind`, we can throw away the `BuiltinMacroState::{NotYetSeen, AlreadySeen}` logic, simplifying things.

Author: matthiaskrgr

Diff for: compiler/rustc_builtin_macros/src/lib.rs

@@ -23,6 +23,8 @@
 
 extern crate proc_macro;
 
+use std::sync::Arc;
+
 use rustc_expand::base::{MacroExpanderFn, ResolverExpand, SyntaxExtensionKind};
 use rustc_expand::proc_macro::BangProcMacro;
 use rustc_span::sym;
@@ -67,13 +69,13 @@ rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
 pub fn register_builtin_macros(resolver: &mut dyn ResolverExpand) {
     let mut register = |name, kind| resolver.register_builtin_macro(name, kind);
     macro register_bang($($name:ident: $f:expr,)*) {
-        $(register(sym::$name, SyntaxExtensionKind::LegacyBang(Box::new($f as MacroExpanderFn)));)*
+        $(register(sym::$name, SyntaxExtensionKind::LegacyBang(Arc::new($f as MacroExpanderFn)));)*
     }
     macro register_attr($($name:ident: $f:expr,)*) {
-        $(register(sym::$name, SyntaxExtensionKind::LegacyAttr(Box::new($f)));)*
+        $(register(sym::$name, SyntaxExtensionKind::LegacyAttr(Arc::new($f)));)*
     }
     macro register_derive($($name:ident: $f:expr,)*) {
-        $(register(sym::$name, SyntaxExtensionKind::LegacyDerive(Box::new(BuiltinDerive($f))));)*
+        $(register(sym::$name, SyntaxExtensionKind::LegacyDerive(Arc::new(BuiltinDerive($f))));)*
     }
 
     register_bang! {
@@ -139,9 +141,9 @@ pub fn register_builtin_macros(resolver: &mut dyn ResolverExpand) {
     }
 
     let client = proc_macro::bridge::client::Client::expand1(proc_macro::quote);
-    register(sym::quote, SyntaxExtensionKind::Bang(Box::new(BangProcMacro { client })));
-    let requires = SyntaxExtensionKind::Attr(Box::new(contracts::ExpandRequires));
+    register(sym::quote, SyntaxExtensionKind::Bang(Arc::new(BangProcMacro { client })));
+    let requires = SyntaxExtensionKind::Attr(Arc::new(contracts::ExpandRequires));
     register(sym::contracts_requires, requires);
-    let ensures = SyntaxExtensionKind::Attr(Box::new(contracts::ExpandEnsures));
+    let ensures = SyntaxExtensionKind::Attr(Arc::new(contracts::ExpandEnsures));
     register(sym::contracts_ensures, ensures);
 }

Diff for: compiler/rustc_error_codes/src/error_codes/E0773.md

@@ -1,40 +1,4 @@
-A builtin-macro was defined more than once.
+#### this error code is no longer emitted by the compiler.
 
-Erroneous code example:
-
-```compile_fail,E0773
-#![feature(decl_macro)]
-#![feature(rustc_attrs)]
-#![allow(internal_features)]
-
-#[rustc_builtin_macro]
-pub macro test($item:item) {
-    /* compiler built-in */
-}
-
-mod inner {
-    #[rustc_builtin_macro]
-    pub macro test($item:item) {
-        /* compiler built-in */
-    }
-}
-```
-
-To fix the issue, remove the duplicate declaration:
-
-```
-#![feature(decl_macro)]
-#![feature(rustc_attrs)]
-#![allow(internal_features)]
-
-#[rustc_builtin_macro]
-pub macro test($item:item) {
-    /* compiler built-in */
-}
-```
-
-In very rare edge cases, this may happen when loading `core` or `std` twice,
-once with `check` metadata and once with `build` metadata.
-For more information, see [#75176].
-
-[#75176]: https://github.com/rust-lang/rust/pull/75176#issuecomment-683234468
+This was triggered when multiple macro definitions used the same
+`#[rustc_builtin_macro(..)]`. This is no longer an error.

Diff for: compiler/rustc_expand/src/base.rs

@@ -681,33 +681,34 @@ impl MacResult for DummyResult {
 }
 
 /// A syntax extension kind.
+#[derive(Clone)]
 pub enum SyntaxExtensionKind {
     /// A token-based function-like macro.
     Bang(
         /// An expander with signature TokenStream -> TokenStream.
-        Box<dyn BangProcMacro + sync::DynSync + sync::DynSend>,
+        Arc<dyn BangProcMacro + sync::DynSync + sync::DynSend>,
     ),
 
     /// An AST-based function-like macro.
     LegacyBang(
         /// An expander with signature TokenStream -> AST.
-        Box<dyn TTMacroExpander + sync::DynSync + sync::DynSend>,
+        Arc<dyn TTMacroExpander + sync::DynSync + sync::DynSend>,
     ),
 
     /// A token-based attribute macro.
     Attr(
         /// An expander with signature (TokenStream, TokenStream) -> TokenStream.
         /// The first TokenStream is the attribute itself, the second is the annotated item.
         /// The produced TokenStream replaces the input TokenStream.
-        Box<dyn AttrProcMacro + sync::DynSync + sync::DynSend>,
+        Arc<dyn AttrProcMacro + sync::DynSync + sync::DynSend>,
     ),
 
     /// An AST-based attribute macro.
     LegacyAttr(
         /// An expander with signature (AST, AST) -> AST.
         /// The first AST fragment is the attribute itself, the second is the annotated item.
         /// The produced AST fragment replaces the input AST fragment.
-        Box<dyn MultiItemModifier + sync::DynSync + sync::DynSend>,
+        Arc<dyn MultiItemModifier + sync::DynSync + sync::DynSend>,
     ),
 
     /// A trivial attribute "macro" that does nothing,
@@ -724,18 +725,18 @@ pub enum SyntaxExtensionKind {
         /// is handled identically to `LegacyDerive`. It should be migrated to
         /// a token-based representation like `Bang` and `Attr`, instead of
         /// using `MultiItemModifier`.
-        Box<dyn MultiItemModifier + sync::DynSync + sync::DynSend>,
+        Arc<dyn MultiItemModifier + sync::DynSync + sync::DynSend>,
     ),
 
     /// An AST-based derive macro.
     LegacyDerive(
         /// An expander with signature AST -> AST.
         /// The produced AST fragment is appended to the input AST fragment.
-        Box<dyn MultiItemModifier + sync::DynSync + sync::DynSend>,
+        Arc<dyn MultiItemModifier + sync::DynSync + sync::DynSend>,
     ),
 
     /// A glob delegation.
-    GlobDelegation(Box<dyn GlobDelegationExpander + sync::DynSync + sync::DynSend>),
+    GlobDelegation(Arc<dyn GlobDelegationExpander + sync::DynSync + sync::DynSend>),
 }
 
 /// A struct representing a macro definition in "lowered" form ready for expansion.
@@ -937,7 +938,7 @@ impl SyntaxExtension {
                 cx.dcx().span_delayed_bug(span, "expanded a dummy bang macro"),
             ))
         }
-        SyntaxExtension::default(SyntaxExtensionKind::LegacyBang(Box::new(expander)), edition)
+        SyntaxExtension::default(SyntaxExtensionKind::LegacyBang(Arc::new(expander)), edition)
     }
 
     /// A dummy derive macro `#[derive(Foo)]`.
@@ -950,7 +951,7 @@ impl SyntaxExtension {
         ) -> Vec<Annotatable> {
             Vec::new()
         }
-        SyntaxExtension::default(SyntaxExtensionKind::Derive(Box::new(expander)), edition)
+        SyntaxExtension::default(SyntaxExtensionKind::Derive(Arc::new(expander)), edition)
     }
 
     pub fn non_macro_attr(edition: Edition) -> SyntaxExtension {
@@ -980,7 +981,7 @@ impl SyntaxExtension {
         }
 
         let expander = GlobDelegationExpanderImpl { trait_def_id, impl_def_id };
-        SyntaxExtension::default(SyntaxExtensionKind::GlobDelegation(Box::new(expander)), edition)
+        SyntaxExtension::default(SyntaxExtensionKind::GlobDelegation(Arc::new(expander)), edition)
     }
 
     pub fn expn_data(

Diff for: compiler/rustc_expand/src/mbe/macro_rules.rs

@@ -1,5 +1,6 @@
 use std::borrow::Cow;
 use std::collections::hash_map::Entry;
+use std::sync::Arc;
 use std::{mem, slice};
 
 use ast::token::IdentIsRaw;
@@ -388,7 +389,7 @@ pub fn compile_declarative_macro(
             node_id != DUMMY_NODE_ID,
         )
     };
-    let dummy_syn_ext = |guar| (mk_syn_ext(Box::new(DummyExpander(guar))), Vec::new());
+    let dummy_syn_ext = |guar| (mk_syn_ext(Arc::new(DummyExpander(guar))), Vec::new());
 
     let lhs_nm = Ident::new(sym::lhs, span);
     let rhs_nm = Ident::new(sym::rhs, span);
@@ -582,7 +583,7 @@ pub fn compile_declarative_macro(
         })
         .collect();
 
-    let expander = Box::new(MacroRulesMacroExpander {
+    let expander = Arc::new(MacroRulesMacroExpander {
         name: ident,
         span,
         node_id,

Diff for: compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -1053,15 +1053,15 @@ impl<'a> CrateMetadataRef<'a> {
                     attributes.iter().cloned().map(Symbol::intern).collect::<Vec<_>>();
                 (
                     trait_name,
-                    SyntaxExtensionKind::Derive(Box::new(DeriveProcMacro { client })),
+                    SyntaxExtensionKind::Derive(Arc::new(DeriveProcMacro { client })),
                     helper_attrs,
                 )
             }
             ProcMacro::Attr { name, client } => {
-                (name, SyntaxExtensionKind::Attr(Box::new(AttrProcMacro { client })), Vec::new())
+                (name, SyntaxExtensionKind::Attr(Arc::new(AttrProcMacro { client })), Vec::new())
             }
             ProcMacro::Bang { name, client } => {
-                (name, SyntaxExtensionKind::Bang(Box::new(BangProcMacro { client })), Vec::new())
+                (name, SyntaxExtensionKind::Bang(Arc::new(BangProcMacro { client })), Vec::new())
             }
         };
 

Diff for: compiler/rustc_resolve/messages.ftl

@@ -26,10 +26,6 @@ resolve_associated_fn_with_similar_name_exists =
 resolve_associated_type_with_similar_name_exists =
     there is an associated type with a similar name
 
-resolve_attempt_to_define_builtin_macro_twice =
-    attempted to define built-in macro more than once
-    .note = previously defined here
-
 resolve_attempt_to_use_non_constant_value_in_constant =
     attempt to use a non-constant value in a constant
 

Diff for: compiler/rustc_resolve/src/errors.rs

@@ -965,15 +965,6 @@ pub(crate) struct StaticLifetimeIsReserved {
     pub(crate) lifetime: Ident,
 }
 
-#[derive(Diagnostic)]
-#[diag(resolve_attempt_to_define_builtin_macro_twice, code = E0773)]
-pub(crate) struct AttemptToDefineBuiltinMacroTwice {
-    #[primary_span]
-    pub(crate) span: Span,
-    #[note]
-    pub(crate) note_span: Span,
-}
-
 #[derive(Diagnostic)]
 #[diag(resolve_variable_is_not_bound_in_all_patterns, code = E0408)]
 pub(crate) struct VariableIsNotBoundInAllPatterns {

Diff for: compiler/rustc_resolve/src/lib.rs

@@ -1010,12 +1010,6 @@ impl ExternPreludeEntry<'_> {
     }
 }
 
-/// Used for better errors for E0773
-enum BuiltinMacroState {
-    NotYetSeen(SyntaxExtensionKind),
-    AlreadySeen(Span),
-}
-
 struct DeriveData {
     resolutions: Vec<DeriveResolution>,
     helper_attrs: Vec<(usize, Ident)>,
@@ -1134,7 +1128,7 @@ pub struct Resolver<'ra, 'tcx> {
 
     used_extern_options: FxHashSet<Symbol>,
     macro_names: FxHashSet<Ident>,
-    builtin_macros: FxHashMap<Symbol, BuiltinMacroState>,
+    builtin_macros: FxHashMap<Symbol, SyntaxExtensionKind>,
     registered_tools: &'tcx RegisteredTools,
     macro_use_prelude: FxIndexMap<Symbol, NameBinding<'ra>>,
     macro_map: FxHashMap<DefId, MacroData>,

Diff for: compiler/rustc_resolve/src/macros.rs

@@ -40,9 +40,9 @@ use crate::errors::{
 };
 use crate::imports::Import;
 use crate::{
-    BindingKey, BuiltinMacroState, DeriveData, Determinacy, Finalize, InvocationParent, MacroData,
-    ModuleKind, ModuleOrUniformRoot, NameBinding, NameBindingKind, ParentScope, PathResult,
-    ResolutionError, Resolver, ScopeSet, Segment, ToNameBinding, Used,
+    BindingKey, DeriveData, Determinacy, Finalize, InvocationParent, MacroData, ModuleKind,
+    ModuleOrUniformRoot, NameBinding, NameBindingKind, ParentScope, PathResult, ResolutionError,
+    Resolver, ScopeSet, Segment, ToNameBinding, Used,
 };
 
 type Res = def::Res<NodeId>;
@@ -194,7 +194,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
     }
 
     fn register_builtin_macro(&mut self, name: Symbol, ext: SyntaxExtensionKind) {
-        if self.builtin_macros.insert(name, BuiltinMacroState::NotYetSeen(ext)).is_some() {
+        if self.builtin_macros.insert(name, ext).is_some() {
             self.dcx().bug(format!("built-in macro `{name}` was already registered"));
         }
     }
@@ -1127,20 +1127,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
 
         if let Some(builtin_name) = ext.builtin_name {
             // The macro was marked with `#[rustc_builtin_macro]`.
-            if let Some(builtin_macro) = self.builtin_macros.get_mut(&builtin_name) {
+            if let Some(builtin_ext_kind) = self.builtin_macros.get(&builtin_name) {
                 // The macro is a built-in, replace its expander function
                 // while still taking everything else from the source code.
-                // If we already loaded this builtin macro, give a better error message than 'no such builtin macro'.
-                match mem::replace(builtin_macro, BuiltinMacroState::AlreadySeen(span)) {
-                    BuiltinMacroState::NotYetSeen(builtin_ext) => {
-                        ext.kind = builtin_ext;
-                        rule_spans = Vec::new();
-                    }
-                    BuiltinMacroState::AlreadySeen(note_span) => {
-                        self.dcx()
-                            .emit_err(errors::AttemptToDefineBuiltinMacroTwice { span, note_span });
-                    }
-                }
+                ext.kind = builtin_ext_kind.clone();
+                rule_spans = Vec::new();
             } else {
                 self.dcx().emit_err(errors::CannotFindBuiltinMacroWithName { span, ident });
             }

Diff for: tests/ui/macros/duplicate-builtin.rs

@@ -1,12 +1,11 @@
-//@ error-pattern: attempted to define built-in macro more than once
-
 #![feature(rustc_attrs)]
 
 #[rustc_builtin_macro]
 macro_rules! unknown { () => () } //~ ERROR cannot find a built-in macro with name `unknown`
 
+// Defining another `line` builtin macro should not cause an error.
 #[rustc_builtin_macro]
-macro_rules! line { () => () } //~ NOTE previously defined here
+macro_rules! line { () => () }
 
 fn main() {
     line!();

Diff for: tests/ui/macros/duplicate-builtin.stderr

@@ -1,18 +1,8 @@
 error: cannot find a built-in macro with name `unknown`
-  --> $DIR/unknown-builtin.rs:6:1
+  --> $DIR/unknown-builtin.rs:4:1
    |
 LL | macro_rules! unknown { () => () }
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error[E0773]: attempted to define built-in macro more than once
-  --> $SRC_DIR/core/src/macros/mod.rs:LL:COL
-   |
-note: previously defined here
-  --> $DIR/unknown-builtin.rs:9:1
-   |
-LL | macro_rules! line { () => () }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-error: aborting due to 2 previous errors
+error: aborting due to 1 previous error
 
-For more information about this error, try `rustc --explain E0773`.