Skip to content

Commit

Permalink
resolve: Collapse macro_rules scope chains on the fly
Browse files Browse the repository at this point in the history
  • Loading branch information
petrochenkov committed Nov 6, 2020
1 parent a601302 commit 9221079
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 30 deletions.
27 changes: 16 additions & 11 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use crate::def_collector::collect_definitions;
use crate::imports::{Import, ImportKind};
use crate::macros::{MacroRulesBinding, MacroRulesScope};
use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
use crate::{CrateLint, Determinacy, PathResult, ResolutionError, VisResolutionError};
use crate::{
Expand Down Expand Up @@ -209,7 +209,7 @@ impl<'a> Resolver<'a> {
&mut self,
fragment: &AstFragment,
parent_scope: ParentScope<'a>,
) -> MacroRulesScope<'a> {
) -> MacroRulesScopeRef<'a> {
collect_definitions(self, fragment, parent_scope.expansion);
let mut visitor = BuildReducedGraphVisitor { r: self, parent_scope };
fragment.visit_with(&mut visitor);
Expand All @@ -220,7 +220,8 @@ impl<'a> Resolver<'a> {
let def_id = module.def_id().expect("unpopulated module without a def-id");
for child in self.cstore().item_children_untracked(def_id, self.session) {
let child = child.map_id(|_| panic!("unexpected id"));
BuildReducedGraphVisitor { r: self, parent_scope: ParentScope::module(module) }
let parent_scope = ParentScope::module(module, self);
BuildReducedGraphVisitor { r: self, parent_scope }
.build_reduced_graph_for_external_crate_res(child);
}
}
Expand Down Expand Up @@ -1154,15 +1155,17 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
false
}

fn visit_invoc(&mut self, id: NodeId) -> MacroRulesScope<'a> {
fn visit_invoc(&mut self, id: NodeId) -> MacroRulesScopeRef<'a> {
let invoc_id = id.placeholder_to_expn_id();

self.parent_scope.module.unexpanded_invocations.borrow_mut().insert(invoc_id);

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");

MacroRulesScope::Invocation(invoc_id)
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
}

fn proc_macro_stub(&self, item: &ast::Item) -> Option<(MacroKind, Ident, Span)> {
Expand Down Expand Up @@ -1196,7 +1199,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
}
}

fn define_macro(&mut self, item: &ast::Item) -> MacroRulesScope<'a> {
fn define_macro(&mut self, item: &ast::Item) -> MacroRulesScopeRef<'a> {
let parent_scope = self.parent_scope;
let expansion = parent_scope.expansion;
let def_id = self.r.local_def_id(item.id);
Expand Down Expand Up @@ -1239,11 +1242,13 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
self.insert_unused_macro(ident, def_id, item.id, span);
}
self.r.visibilities.insert(def_id, vis);
MacroRulesScope::Binding(self.r.arenas.alloc_macro_rules_binding(MacroRulesBinding {
parent_macro_rules_scope: parent_scope.macro_rules,
binding,
ident,
}))
self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding(
self.r.arenas.alloc_macro_rules_binding(MacroRulesBinding {
parent_macro_rules_scope: parent_scope.macro_rules,
binding,
ident,
}),
))
} else {
let module = parent_scope.module;
let vis = match item.kind {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ impl<'a> Resolver<'a> {
}
}
Scope::MacroRules(macro_rules_scope) => {
if let MacroRulesScope::Binding(macro_rules_binding) = macro_rules_scope {
if let MacroRulesScope::Binding(macro_rules_binding) = macro_rules_scope.get() {
let res = macro_rules_binding.binding.res();
if filter_fn(res) {
suggestions
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
// During late resolution we only track the module component of the parent scope,
// although it may be useful to track other components as well for diagnostics.
let graph_root = resolver.graph_root;
let parent_scope = ParentScope::module(graph_root);
let parent_scope = ParentScope::module(graph_root, resolver);
let start_rib_kind = ModuleRibKind(graph_root);
LateResolutionVisitor {
r: resolver,
Expand Down
37 changes: 23 additions & 14 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ use diagnostics::{extend_span_to_previous_binding, find_span_of_binding_until_ne
use diagnostics::{ImportSuggestion, LabelSuggestion, Suggestion};
use imports::{Import, ImportKind, ImportResolver, NameResolution};
use late::{HasGenericParams, PathSource, Rib, RibKind::*};
use macros::{MacroRulesBinding, MacroRulesScope};
use macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};

type Res = def::Res<NodeId>;

Expand Down Expand Up @@ -100,7 +100,7 @@ impl Determinacy {
enum Scope<'a> {
DeriveHelpers(ExpnId),
DeriveHelpersCompat,
MacroRules(MacroRulesScope<'a>),
MacroRules(MacroRulesScopeRef<'a>),
CrateRoot,
Module(Module<'a>),
RegisteredAttrs,
Expand Down Expand Up @@ -133,18 +133,18 @@ enum ScopeSet {
pub struct ParentScope<'a> {
module: Module<'a>,
expansion: ExpnId,
macro_rules: MacroRulesScope<'a>,
macro_rules: MacroRulesScopeRef<'a>,
derives: &'a [ast::Path],
}

impl<'a> ParentScope<'a> {
/// Creates a parent scope with the passed argument used as the module scope component,
/// and other scope components set to default empty values.
pub fn module(module: Module<'a>) -> ParentScope<'a> {
pub fn module(module: Module<'a>, resolver: &Resolver<'a>) -> ParentScope<'a> {
ParentScope {
module,
expansion: ExpnId::root(),
macro_rules: MacroRulesScope::Empty,
macro_rules: resolver.arenas.alloc_macro_rules_scope(MacroRulesScope::Empty),
derives: &[],
}
}
Expand Down Expand Up @@ -974,7 +974,10 @@ pub struct Resolver<'a> {
invocation_parent_scopes: FxHashMap<ExpnId, ParentScope<'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<ExpnId, MacroRulesScope<'a>>,
output_macro_rules_scopes: FxHashMap<ExpnId, MacroRulesScopeRef<'a>>,
/// 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<ExpnId, FxHashSet<MacroRulesScopeRef<'a>>>,
/// Helper attributes that are in scope for the given expansion.
helper_attrs: FxHashMap<ExpnId, Vec<Ident>>,

Expand Down Expand Up @@ -1043,6 +1046,9 @@ impl<'a> ResolverArenas<'a> {
fn alloc_name_resolution(&'a self) -> &'a RefCell<NameResolution<'a>> {
self.name_resolutions.alloc(Default::default())
}
fn alloc_macro_rules_scope(&'a self, scope: MacroRulesScope<'a>) -> MacroRulesScopeRef<'a> {
PtrKey(self.dropless.alloc(Cell::new(scope)))
}
fn alloc_macro_rules_binding(
&'a self,
binding: MacroRulesBinding<'a>,
Expand Down Expand Up @@ -1230,14 +1236,11 @@ impl<'a> Resolver<'a> {
let (registered_attrs, registered_tools) =
macros::registered_attrs_and_tools(session, &krate.attrs);

let mut invocation_parent_scopes = FxHashMap::default();
invocation_parent_scopes.insert(ExpnId::root(), ParentScope::module(graph_root));

let features = session.features_untracked();
let non_macro_attr =
|mark_used| Lrc::new(SyntaxExtension::non_macro_attr(mark_used, session.edition()));

Resolver {
let mut resolver = Resolver {
session,

definitions,
Expand Down Expand Up @@ -1304,8 +1307,9 @@ impl<'a> Resolver<'a> {
dummy_ext_bang: Lrc::new(SyntaxExtension::dummy_bang(session.edition())),
dummy_ext_derive: Lrc::new(SyntaxExtension::dummy_derive(session.edition())),
non_macro_attrs: [non_macro_attr(false), non_macro_attr(true)],
invocation_parent_scopes,
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(),
Expand All @@ -1332,7 +1336,12 @@ impl<'a> Resolver<'a> {
invocation_parents,
next_disambiguator: Default::default(),
trait_impl_items: Default::default(),
}
};

let root_parent_scope = ParentScope::module(graph_root, &resolver);
resolver.invocation_parent_scopes.insert(ExpnId::root(), root_parent_scope);

resolver
}

pub fn next_node_id(&mut self) -> NodeId {
Expand Down Expand Up @@ -1702,7 +1711,7 @@ impl<'a> Resolver<'a> {
}
Scope::DeriveHelpers(..) => Scope::DeriveHelpersCompat,
Scope::DeriveHelpersCompat => Scope::MacroRules(parent_scope.macro_rules),
Scope::MacroRules(macro_rules_scope) => match macro_rules_scope {
Scope::MacroRules(macro_rules_scope) => match macro_rules_scope.get() {
MacroRulesScope::Binding(binding) => {
Scope::MacroRules(binding.parent_macro_rules_scope)
}
Expand Down Expand Up @@ -3199,7 +3208,7 @@ impl<'a> Resolver<'a> {
}
};
let module = self.get_module(module_id);
let parent_scope = &ParentScope::module(module);
let parent_scope = &ParentScope::module(module, self);
let res = self.resolve_ast_path(&path, ns, parent_scope).map_err(|_| ())?;
Ok((path, res))
}
Expand Down
30 changes: 28 additions & 2 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rustc_ast_lowering::ResolverAstLowering;
use rustc_ast_pretty::pprust;
use rustc_attr::StabilityLevel;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::ptr_key::PtrKey;
use rustc_errors::struct_span_err;
use rustc_expand::base::{Indeterminate, InvocationRes, ResolverExpand, SyntaxExtension};
use rustc_expand::compile_declarative_macro;
Expand All @@ -29,6 +30,7 @@ use rustc_span::{Span, DUMMY_SP};

use rustc_data_structures::sync::Lrc;
use rustc_span::hygiene::{AstPass, MacroKind};
use std::cell::Cell;
use std::{mem, ptr};

type Res = def::Res<NodeId>;
Expand All @@ -39,7 +41,7 @@ type Res = def::Res<NodeId>;
pub struct MacroRulesBinding<'a> {
crate binding: &'a NameBinding<'a>,
/// `macro_rules` scope into which the `macro_rules` item was planted.
crate parent_macro_rules_scope: MacroRulesScope<'a>,
crate parent_macro_rules_scope: MacroRulesScopeRef<'a>,
crate ident: Ident,
}

Expand All @@ -59,6 +61,14 @@ pub enum MacroRulesScope<'a> {
Invocation(ExpnId),
}

/// `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.
/// 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.
pub(crate) type MacroRulesScopeRef<'a> = PtrKey<'a, Cell<MacroRulesScope<'a>>>;

// Macro namespace is separated into two sub-namespaces, one for bang macros and
// one for attribute-like macros (attributes, derives).
// We ignore resolutions from one sub-namespace when searching names in scope for another.
Expand Down Expand Up @@ -163,6 +173,22 @@ 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);
}

Expand Down Expand Up @@ -655,7 +681,7 @@ impl<'a> Resolver<'a> {
}
result
}
Scope::MacroRules(macro_rules_scope) => match macro_rules_scope {
Scope::MacroRules(macro_rules_scope) => match macro_rules_scope.get() {
MacroRulesScope::Binding(macro_rules_binding)
if ident == macro_rules_binding.ident =>
{
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
if let Ok((Some(ext), res)) = resolver.resolve_macro_path(
&path,
None,
&ParentScope::module(resolver.graph_root()),
&ParentScope::module(resolver.graph_root(), resolver),
false,
false,
) {
Expand Down

0 comments on commit 9221079

Please sign in to comment.