From ad0770eeee26ff4b38bd643044d702c236a08901 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 24 Nov 2023 23:39:42 +0300 Subject: [PATCH] resolve: Avoid clones of `MacroData` And move declarative macro compilation to an earlier point in def collector, which is required for #118188. --- .../rustc_resolve/src/build_reduced_graph.rs | 55 +++++++------------ compiler/rustc_resolve/src/def_collector.rs | 9 ++- compiler/rustc_resolve/src/ident.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 18 ++++-- compiler/rustc_resolve/src/macros.rs | 23 ++++---- 5 files changed, 52 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 0b1a43f328264..60a28e3a1a4b5 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/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 { + 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); diff --git a/compiler/rustc_resolve/src/def_collector.rs b/compiler/rustc_resolve/src/def_collector.rs index 13df7efe636d8..b1ee7f438d296 100644 --- a/compiler/rustc_resolve/src/def_collector.rs +++ b/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, _) => { diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 1a50bd5ec9865..a069c6f8e0436 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/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) diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 23e8ebc49b0b5..29e3697dc7066 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -927,9 +927,16 @@ struct DeriveData { #[derive(Clone)] struct MacroData { ext: Lrc, + rule_spans: Vec<(usize, Span)>, macro_rules: bool, } +impl MacroData { + fn new(ext: Lrc) -> 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, dummy_ext_bang: Lrc, dummy_ext_derive: Lrc, - non_macro_attr: Lrc, + non_macro_attr: MacroData, local_macro_def_scopes: FxHashMap>, ast_transform_scopes: FxHashMap>, unused_macros: FxHashMap, @@ -1321,6 +1328,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let features = tcx.features(); let pub_vis = ty::Visibility::::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(), } } diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 7242aa890f82b..f8274e7d7d0df 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/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 } } }