Rollup merge of #118272 - petrochenkov:macrodata, r=cjgillot

resolve: Avoid clones of `MacroData`

And move declarative macro compilation to an earlier point in def collector, which is required for #118188.

Author: GuillaumeGomez

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -156,33 +156,26 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         }
     }
 
-    pub(crate) fn get_macro(&mut self, res: Res) -> Option<MacroData> {
+    pub(crate) fn get_macro(&mut self, res: Res) -> Option<&MacroData> {
         match res {
             Res::Def(DefKind::Macro(..), def_id) => Some(self.get_macro_by_def_id(def_id)),
-            Res::NonMacroAttr(_) => {
-                Some(MacroData { ext: self.non_macro_attr.clone(), macro_rules: false })
-            }
+            Res::NonMacroAttr(_) => Some(&self.non_macro_attr),
             _ => None,
         }
     }
 
-    pub(crate) fn get_macro_by_def_id(&mut self, def_id: DefId) -> MacroData {
-        if let Some(macro_data) = self.macro_map.get(&def_id) {
-            return macro_data.clone();
+    pub(crate) fn get_macro_by_def_id(&mut self, def_id: DefId) -> &MacroData {
+        if self.macro_map.contains_key(&def_id) {
+            return &self.macro_map[&def_id];
         }
 
-        let load_macro_untracked = self.cstore().load_macro_untracked(def_id, self.tcx);
-        let (ext, macro_rules) = match load_macro_untracked {
-            LoadedMacro::MacroDef(item, edition) => (
-                Lrc::new(self.compile_macro(&item, edition).0),
-                matches!(item.kind, ItemKind::MacroDef(def) if def.macro_rules),
-            ),
-            LoadedMacro::ProcMacro(extz) => (Lrc::new(extz), false),
+        let loaded_macro = self.cstore().load_macro_untracked(def_id, self.tcx);
+        let macro_data = match loaded_macro {
+            LoadedMacro::MacroDef(item, edition) => self.compile_macro(&item, edition),
+            LoadedMacro::ProcMacro(ext) => MacroData::new(Lrc::new(ext)),
         };
 
-        let macro_data = MacroData { ext, macro_rules };
-        self.macro_map.insert(def_id, macro_data.clone());
-        macro_data
+        self.macro_map.entry(def_id).or_insert(macro_data)
     }
 
     pub(crate) fn build_reduced_graph(
@@ -1175,16 +1168,10 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
     // Mark the given macro as unused unless its name starts with `_`.
     // Macro uses will remove items from this set, and the remaining
     // items will be reported as `unused_macros`.
-    fn insert_unused_macro(
-        &mut self,
-        ident: Ident,
-        def_id: LocalDefId,
-        node_id: NodeId,
-        rule_spans: &[(usize, Span)],
-    ) {
+    fn insert_unused_macro(&mut self, ident: Ident, def_id: LocalDefId, node_id: NodeId) {
         if !ident.as_str().starts_with('_') {
             self.r.unused_macros.insert(def_id, (node_id, ident));
-            for (rule_i, rule_span) in rule_spans.iter() {
+            for (rule_i, rule_span) in &self.r.macro_map[&def_id.to_def_id()].rule_spans {
                 self.r.unused_macro_rules.insert((def_id, *rule_i), (ident, *rule_span));
             }
         }
@@ -1194,24 +1181,24 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
         let parent_scope = self.parent_scope;
         let expansion = parent_scope.expansion;
         let def_id = self.r.local_def_id(item.id);
-        let (ext, ident, span, macro_rules, rule_spans) = match &item.kind {
+        let (macro_kind, ident, span, macro_rules) = match &item.kind {
             ItemKind::MacroDef(def) => {
-                let (ext, rule_spans) = self.r.compile_macro(item, self.r.tcx.sess.edition());
-                let ext = Lrc::new(ext);
-                (ext, item.ident, item.span, def.macro_rules, rule_spans)
+                let macro_kind = self.r.macro_map[&def_id.to_def_id()].ext.macro_kind();
+                (macro_kind, item.ident, item.span, def.macro_rules)
             }
             ItemKind::Fn(..) => match self.proc_macro_stub(item) {
                 Some((macro_kind, ident, span)) => {
+                    let macro_data = MacroData::new(self.r.dummy_ext(macro_kind));
+                    self.r.macro_map.insert(def_id.to_def_id(), macro_data);
                     self.r.proc_macro_stubs.insert(def_id);
-                    (self.r.dummy_ext(macro_kind), ident, span, false, Vec::new())
+                    (macro_kind, ident, span, false)
                 }
                 None => return parent_scope.macro_rules,
             },
             _ => unreachable!(),
         };
 
-        let res = Res::Def(DefKind::Macro(ext.macro_kind()), def_id.to_def_id());
-        self.r.macro_map.insert(def_id.to_def_id(), MacroData { ext, macro_rules });
+        let res = Res::Def(DefKind::Macro(macro_kind), def_id.to_def_id());
         self.r.local_macro_def_scopes.insert(def_id, parent_scope.module);
 
         if macro_rules {
@@ -1245,7 +1232,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
                 self.r.define(self.r.graph_root, ident, MacroNS, import_binding);
             } else {
                 self.r.check_reserved_macro_name(ident, res);
-                self.insert_unused_macro(ident, def_id, item.id, &rule_spans);
+                self.insert_unused_macro(ident, def_id, item.id);
             }
             self.r.visibilities.insert(def_id, vis);
             let scope = self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding(
@@ -1268,7 +1255,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
                 _ => self.resolve_visibility(&item.vis),
             };
             if !vis.is_public() {
-                self.insert_unused_macro(ident, def_id, item.id, &rule_spans);
+                self.insert_unused_macro(ident, def_id, item.id);
             }
             self.r.define(module, ident, MacroNS, (res, vis, span, expansion));
             self.r.visibilities.insert(def_id, vis);

compiler/rustc_resolve/src/def_collector.rs

@@ -111,9 +111,14 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
                 return visit::walk_item(self, i);
             }
         };
-        let def = self.create_def(i.id, def_data, i.span);
+        let def_id = self.create_def(i.id, def_data, i.span);
 
-        self.with_parent(def, |this| {
+        if let ItemKind::MacroDef(..) = i.kind {
+            let macro_data = self.resolver.compile_macro(i, self.resolver.tcx.sess.edition());
+            self.resolver.macro_map.insert(def_id.to_def_id(), macro_data);
+        }
+
+        self.with_parent(def_id, |this| {
             this.with_impl_trait(ImplTraitContext::Existential, |this| {
                 match i.kind {
                     ItemKind::Struct(ref struct_def, _) | ItemKind::Union(ref struct_def, _) => {

compiler/rustc_resolve/src/ident.rs

@@ -240,7 +240,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 {
                     // The macro is a proc macro derive
                     if let Some(def_id) = module.expansion.expn_data().macro_def_id {
-                        let ext = self.get_macro_by_def_id(def_id).ext;
+                        let ext = &self.get_macro_by_def_id(def_id).ext;
                         if ext.builtin_name.is_none()
                             && ext.macro_kind() == MacroKind::Derive
                             && parent.expansion.outer_expn_is_descendant_of(*ctxt)

compiler/rustc_resolve/src/lib.rs

@@ -927,9 +927,16 @@ struct DeriveData {
 #[derive(Clone)]
 struct MacroData {
     ext: Lrc<SyntaxExtension>,
+    rule_spans: Vec<(usize, Span)>,
     macro_rules: bool,
 }
 
+impl MacroData {
+    fn new(ext: Lrc<SyntaxExtension>) -> MacroData {
+        MacroData { ext, rule_spans: Vec::new(), macro_rules: false }
+    }
+}
+
 /// The main resolver class.
 ///
 /// This is the visitor that walks the whole crate.
@@ -1038,7 +1045,7 @@ pub struct Resolver<'a, 'tcx> {
     macro_map: FxHashMap<DefId, MacroData>,
     dummy_ext_bang: Lrc<SyntaxExtension>,
     dummy_ext_derive: Lrc<SyntaxExtension>,
-    non_macro_attr: Lrc<SyntaxExtension>,
+    non_macro_attr: MacroData,
     local_macro_def_scopes: FxHashMap<LocalDefId, Module<'a>>,
     ast_transform_scopes: FxHashMap<LocalExpnId, Module<'a>>,
     unused_macros: FxHashMap<LocalDefId, (NodeId, Ident)>,
@@ -1321,6 +1328,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
         let features = tcx.features();
         let pub_vis = ty::Visibility::<DefId>::Public;
+        let edition = tcx.sess.edition();
 
         let mut resolver = Resolver {
             tcx,
@@ -1402,9 +1410,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             registered_tools,
             macro_use_prelude: FxHashMap::default(),
             macro_map: FxHashMap::default(),
-            dummy_ext_bang: Lrc::new(SyntaxExtension::dummy_bang(tcx.sess.edition())),
-            dummy_ext_derive: Lrc::new(SyntaxExtension::dummy_derive(tcx.sess.edition())),
-            non_macro_attr: Lrc::new(SyntaxExtension::non_macro_attr(tcx.sess.edition())),
+            dummy_ext_bang: Lrc::new(SyntaxExtension::dummy_bang(edition)),
+            dummy_ext_derive: Lrc::new(SyntaxExtension::dummy_derive(edition)),
+            non_macro_attr: MacroData::new(Lrc::new(SyntaxExtension::non_macro_attr(edition))),
             invocation_parent_scopes: Default::default(),
             output_macro_rules_scopes: Default::default(),
             macro_rules_scopes: Default::default(),
@@ -1564,7 +1572,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         match macro_kind {
             MacroKind::Bang => self.dummy_ext_bang.clone(),
             MacroKind::Derive => self.dummy_ext_derive.clone(),
-            MacroKind::Attr => self.non_macro_attr.clone(),
+            MacroKind::Attr => self.non_macro_attr.ext.clone(),
         }
     }
 

compiler/rustc_resolve/src/macros.rs

@@ -6,7 +6,7 @@ use crate::errors::{
     MacroExpectedFound, RemoveSurroundingDerive,
 };
 use crate::Namespace::*;
-use crate::{BuiltinMacroState, Determinacy};
+use crate::{BuiltinMacroState, Determinacy, MacroData};
 use crate::{DeriveData, Finalize, ParentScope, ResolutionError, Resolver, ScopeSet};
 use crate::{ModuleKind, ModuleOrUniformRoot, NameBinding, PathResult, Segment, ToNameBinding};
 use rustc_ast::expand::StrippedCfgItem;
@@ -695,7 +695,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             res
         };
 
-        res.map(|res| (self.get_macro(res).map(|macro_data| macro_data.ext), res))
+        res.map(|res| (self.get_macro(res).map(|macro_data| macro_data.ext.clone()), res))
     }
 
     pub(crate) fn finalize_macro_resolutions(&mut self, krate: &Crate) {
@@ -936,27 +936,23 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     /// Compile the macro into a `SyntaxExtension` and its rule spans.
     ///
     /// Possibly replace its expander to a pre-defined one for built-in macros.
-    pub(crate) fn compile_macro(
-        &mut self,
-        item: &ast::Item,
-        edition: Edition,
-    ) -> (SyntaxExtension, Vec<(usize, Span)>) {
-        let (mut result, mut rule_spans) =
+    pub(crate) fn compile_macro(&mut self, item: &ast::Item, edition: Edition) -> MacroData {
+        let (mut ext, mut rule_spans) =
             compile_declarative_macro(self.tcx.sess, self.tcx.features(), item, edition);
 
-        if let Some(builtin_name) = result.builtin_name {
+        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) {
                 // 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(item.span)) {
-                    BuiltinMacroState::NotYetSeen(ext) => {
-                        result.kind = ext;
+                    BuiltinMacroState::NotYetSeen(builtin_ext) => {
+                        ext.kind = builtin_ext;
                         rule_spans = Vec::new();
                         if item.id != ast::DUMMY_NODE_ID {
                             self.builtin_macro_kinds
-                                .insert(self.local_def_id(item.id), result.macro_kind());
+                                .insert(self.local_def_id(item.id), ext.macro_kind());
                         }
                     }
                     BuiltinMacroState::AlreadySeen(span) => {
@@ -976,6 +972,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             }
         }
 
-        (result, rule_spans)
+        let ItemKind::MacroDef(def) = &item.kind else { unreachable!() };
+        MacroData { ext: Lrc::new(ext), rule_spans, macro_rules: def.macro_rules }
     }
 }