From ac4c1f58b9fe1b4182b8af598751afd88caa5db5 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 14 Nov 2020 00:05:05 +0300 Subject: [PATCH] rustc_resolve: Make `macro_rules` scope chain compression lazy --- .../rustc_resolve/src/build_reduced_graph.rs | 4 +-- compiler/rustc_resolve/src/lib.rs | 27 ++++++++++++------- compiler/rustc_resolve/src/macros.rs | 26 +++--------------- 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 34145c3c138..493b9f15271 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -1163,9 +1163,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { let old_parent_scope = self.r.invocation_parent_scopes.insert(invoc_id, self.parent_scope); assert!(old_parent_scope.is_none(), "invocation data is reset for an invocation"); - let scope = self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Invocation(invoc_id)); - self.r.invocation_macro_rules_scopes.entry(invoc_id).or_default().insert(scope); - scope + self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Invocation(invoc_id)) } fn proc_macro_stub(&self, item: &ast::Item) -> Option<(MacroKind, Ident, Span)> { diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 4e85c88c0e5..d18335ef2e6 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -976,9 +976,6 @@ pub struct Resolver<'a> { /// `macro_rules` scopes *produced* by expanding the macro invocations, /// include all the `macro_rules` items and other invocations generated by them. output_macro_rules_scopes: FxHashMap>, - /// References to all `MacroRulesScope::Invocation(invoc_id)`s, used to update such scopes - /// when their corresponding `invoc_id`s get expanded. - invocation_macro_rules_scopes: FxHashMap>>, /// Helper attributes that are in scope for the given expansion. helper_attrs: FxHashMap>, @@ -1310,7 +1307,6 @@ impl<'a> Resolver<'a> { non_macro_attrs: [non_macro_attr(false), non_macro_attr(true)], invocation_parent_scopes: Default::default(), output_macro_rules_scopes: Default::default(), - invocation_macro_rules_scopes: Default::default(), helper_attrs: Default::default(), local_macro_def_scopes: FxHashMap::default(), name_already_seen: FxHashMap::default(), @@ -1680,7 +1676,20 @@ impl<'a> Resolver<'a> { !(expn_id == parent_scope.expansion && macro_kind == Some(MacroKind::Derive)) } Scope::DeriveHelpersCompat => true, - Scope::MacroRules(..) => true, + Scope::MacroRules(macro_rules_scope) => { + // Use "path compression" on `macro_rules` scope chains. This is an optimization + // used to avoid long scope chains, see the comments on `MacroRulesScopeRef`. + // As another consequence of this optimization visitors never observe invocation + // scopes for macros that were already expanded. + while let MacroRulesScope::Invocation(invoc_id) = macro_rules_scope.get() { + if let Some(next_scope) = self.output_macro_rules_scopes.get(&invoc_id) { + macro_rules_scope.set(next_scope.get()); + } else { + break; + } + } + true + } Scope::CrateRoot => true, Scope::Module(..) => true, Scope::RegisteredAttrs => use_prelude, @@ -1716,11 +1725,9 @@ impl<'a> Resolver<'a> { MacroRulesScope::Binding(binding) => { Scope::MacroRules(binding.parent_macro_rules_scope) } - MacroRulesScope::Invocation(invoc_id) => Scope::MacroRules( - self.output_macro_rules_scopes.get(&invoc_id).cloned().unwrap_or_else( - || self.invocation_parent_scopes[&invoc_id].macro_rules, - ), - ), + MacroRulesScope::Invocation(invoc_id) => { + Scope::MacroRules(self.invocation_parent_scopes[&invoc_id].macro_rules) + } MacroRulesScope::Empty => Scope::Module(module), }, Scope::CrateRoot => match ns { diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 6bc9419ea84..e052b6b3345 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -62,8 +62,8 @@ pub enum MacroRulesScope<'a> { } /// `macro_rules!` scopes are always kept by reference and inside a cell. -/// The reason is that we update all scopes with value `MacroRulesScope::Invocation(invoc_id)` -/// in-place immediately after `invoc_id` gets expanded. +/// The reason is that we update scopes with value `MacroRulesScope::Invocation(invoc_id)` +/// in-place after `invoc_id` gets expanded. /// This helps to avoid uncontrollable growth of `macro_rules!` scope chains, /// which usually grow lineraly with the number of macro invocations /// in a module (including derives) and hurt performance. @@ -173,22 +173,6 @@ impl<'a> ResolverExpand for Resolver<'a> { let output_macro_rules_scope = self.build_reduced_graph(fragment, parent_scope); self.output_macro_rules_scopes.insert(expansion, output_macro_rules_scope); - // Update all `macro_rules` scopes referring to this invocation. This is an optimization - // used to avoid long scope chains, see the comments on `MacroRulesScopeRef`. - if let Some(invocation_scopes) = self.invocation_macro_rules_scopes.remove(&expansion) { - for invocation_scope in &invocation_scopes { - invocation_scope.set(output_macro_rules_scope.get()); - } - // All `macro_rules` scopes that previously referred to `expansion` - // are now rerouted to its output scope, if it's also an invocation. - if let MacroRulesScope::Invocation(invoc_id) = output_macro_rules_scope.get() { - self.invocation_macro_rules_scopes - .entry(invoc_id) - .or_default() - .extend(invocation_scopes); - } - } - parent_scope.module.unexpanded_invocations.borrow_mut().remove(&expansion); } @@ -687,11 +671,7 @@ impl<'a> Resolver<'a> { { Ok((macro_rules_binding.binding, Flags::MACRO_RULES)) } - MacroRulesScope::Invocation(invoc_id) - if !this.output_macro_rules_scopes.contains_key(&invoc_id) => - { - Err(Determinacy::Undetermined) - } + MacroRulesScope::Invocation(_) => Err(Determinacy::Undetermined), _ => Err(Determinacy::Determined), }, Scope::CrateRoot => {