resolve: Eliminate InvocationData

petrochenkov · petrochenkov · commit 59dd07ae2bbc · 2019-08-15T20:40:18.000+03:00
It was very similar to `ParentScope` and mostly could be replaced by it.
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
@@ -3,7 +3,7 @@
 //! Here we build the "reduced graph": the graph of the module tree without
 //! any imports resolved.
 
-use crate::macros::{InvocationData, LegacyBinding, LegacyScope};
+use crate::macros::{LegacyBinding, LegacyScope};
 use crate::resolve_imports::ImportDirective;
 use crate::resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleImport};
 use crate::{Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, Segment, ToNameBinding};
@@ -1063,20 +1063,17 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         false
     }
 
-    fn visit_invoc(&mut self, id: ast::NodeId) -> &'a InvocationData<'a> {
+    fn visit_invoc(&mut self, id: ast::NodeId) -> LegacyScope<'a> {
         let invoc_id = id.placeholder_to_expn_id();
 
-        self.parent_scope.module.unresolved_invocations.borrow_mut().insert(invoc_id);
+        let parent_scope = self.parent_scope.clone();
+        parent_scope.module.unresolved_invocations.borrow_mut().insert(invoc_id);
 
-        let invocation_data = self.r.arenas.alloc_invocation_data(InvocationData {
-            module: self.parent_scope.module,
-            parent_legacy_scope: self.parent_scope.legacy,
-            output_legacy_scope: Cell::new(None),
-        });
-        let old_invocation_data = self.r.invocations.insert(invoc_id, invocation_data);
-        assert!(old_invocation_data.is_none(), "invocation data is reset for an invocation");
+        let old_parent_scope =
+            self.r.invocation_parent_scopes.insert(invoc_id, parent_scope.clone());
+        assert!(old_parent_scope.is_none(), "invocation data is reset for an invocation");
 
-        invocation_data
+        LegacyScope::Invocation(invoc_id)
     }
 
     fn proc_macro_stub(item: &ast::Item) -> Option<(MacroKind, Ident, Span)> {
@@ -1177,7 +1174,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> {
                 return
             }
             ItemKind::Mac(..) => {
-                self.parent_scope.legacy = LegacyScope::Invocation(self.visit_invoc(item.id));
+                self.parent_scope.legacy = self.visit_invoc(item.id);
                 return
             }
             ItemKind::Mod(..) => self.contains_macro_use(&item.attrs),
@@ -1196,7 +1193,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> {
 
     fn visit_stmt(&mut self, stmt: &'b ast::Stmt) {
         if let ast::StmtKind::Mac(..) = stmt.node {
-            self.parent_scope.legacy = LegacyScope::Invocation(self.visit_invoc(stmt.id));
+            self.parent_scope.legacy = self.visit_invoc(stmt.id);
         } else {
             visit::walk_stmt(self, stmt);
         }
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
@@ -54,7 +54,7 @@ use diagnostics::{Suggestion, ImportSuggestion};
 use diagnostics::{find_span_of_binding_until_next_binding, extend_span_to_previous_binding};
 use late::{PathSource, Rib, RibKind::*};
 use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver};
-use macros::{InvocationData, LegacyBinding, LegacyScope};
+use macros::{LegacyBinding, LegacyScope};
 
 type Res = def::Res<NodeId>;
 
@@ -911,9 +911,12 @@ pub struct Resolver<'a> {
     /// FIXME: Find a way for `PartialEq` and `Eq` to emulate `#[structural_match]`
     /// by marking the produced impls rather than the original items.
     special_derives: FxHashMap<ExpnId, SpecialDerives>,
-
-    /// Maps the `ExpnId` of an expansion to its containing module or block.
-    invocations: FxHashMap<ExpnId, &'a InvocationData<'a>>,
+    /// Parent scopes in which the macros were invoked.
+    /// FIXME: `derives` are missing in these parent scopes and need to be taken from elsewhere.
+    invocation_parent_scopes: FxHashMap<ExpnId, ParentScope<'a>>,
+    /// Legacy scopes *produced* by expanding the macro invocations,
+    /// include all the `macro_rules` items and other invocations generated by them.
+    output_legacy_scopes: FxHashMap<ExpnId, LegacyScope<'a>>,
 
     /// Avoid duplicated errors for "name already defined".
     name_already_seen: FxHashMap<Name, Span>,
@@ -936,7 +939,6 @@ pub struct ResolverArenas<'a> {
     name_bindings: arena::TypedArena<NameBinding<'a>>,
     import_directives: arena::TypedArena<ImportDirective<'a>>,
     name_resolutions: arena::TypedArena<RefCell<NameResolution<'a>>>,
-    invocation_data: arena::TypedArena<InvocationData<'a>>,
     legacy_bindings: arena::TypedArena<LegacyBinding<'a>>,
 }
 
@@ -961,10 +963,6 @@ impl<'a> ResolverArenas<'a> {
     fn alloc_name_resolution(&'a self) -> &'a RefCell<NameResolution<'a>> {
         self.name_resolutions.alloc(Default::default())
     }
-    fn alloc_invocation_data(&'a self, expansion_data: InvocationData<'a>)
-                             -> &'a InvocationData<'a> {
-        self.invocation_data.alloc(expansion_data)
-    }
     fn alloc_legacy_binding(&'a self, binding: LegacyBinding<'a>) -> &'a LegacyBinding<'a> {
         self.legacy_bindings.alloc(binding)
     }
@@ -1078,9 +1076,8 @@ impl<'a> Resolver<'a> {
             }
         }
 
-        let mut invocations = FxHashMap::default();
-        invocations.insert(ExpnId::root(),
-                           arenas.alloc_invocation_data(InvocationData::default(graph_root)));
+        let mut invocation_parent_scopes = FxHashMap::default();
+        invocation_parent_scopes.insert(ExpnId::root(), ParentScope::default(graph_root));
 
         let mut macro_defs = FxHashMap::default();
         macro_defs.insert(ExpnId::root(), root_def_id);
@@ -1152,7 +1149,8 @@ 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)],
-            invocations,
+            invocation_parent_scopes,
+            output_legacy_scopes: Default::default(),
             macro_defs,
             local_macro_def_scopes: FxHashMap::default(),
             name_already_seen: FxHashMap::default(),
@@ -1370,8 +1368,9 @@ impl<'a> Resolver<'a> {
                     LegacyScope::Binding(binding) => Scope::MacroRules(
                         binding.parent_legacy_scope
                     ),
-                    LegacyScope::Invocation(invoc) => Scope::MacroRules(
-                        invoc.output_legacy_scope.get().unwrap_or(invoc.parent_legacy_scope)
+                    LegacyScope::Invocation(invoc_id) => Scope::MacroRules(
+                        self.output_legacy_scopes.get(&invoc_id).cloned()
+                            .unwrap_or(self.invocation_parent_scopes[&invoc_id].legacy)
                     ),
                     LegacyScope::Empty => Scope::Module(module),
                 }
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
@@ -1,6 +1,6 @@
 use crate::{AmbiguityError, AmbiguityKind, AmbiguityErrorMisc, Determinacy};
 use crate::{CrateLint, Resolver, ResolutionError, Scope, ScopeSet, ParentScope, Weak};
-use crate::{Module, ModuleKind, NameBinding, PathResult, Segment, ToNameBinding};
+use crate::{ModuleKind, NameBinding, PathResult, Segment, ToNameBinding};
 use crate::{ModuleOrUniformRoot, KNOWN_TOOLS};
 use crate::Namespace::*;
 use crate::build_reduced_graph::BuildReducedGraphVisitor;
@@ -22,36 +22,11 @@ use syntax::feature_gate::GateIssue;
 use syntax::symbol::{Symbol, kw, sym};
 use syntax_pos::{Span, DUMMY_SP};
 
-use std::cell::Cell;
 use std::{mem, ptr};
 use rustc_data_structures::sync::Lrc;
 
 type Res = def::Res<ast::NodeId>;
 
-// FIXME: Merge this with `ParentScope`.
-#[derive(Clone, Debug)]
-pub struct InvocationData<'a> {
-    /// The module in which the macro was invoked.
-    crate module: Module<'a>,
-    /// The legacy scope in which the macro was invoked.
-    /// The invocation path is resolved in this scope.
-    crate parent_legacy_scope: LegacyScope<'a>,
-    /// The legacy scope *produced* by expanding this macro invocation,
-    /// includes all the macro_rules items, other invocations, etc generated by it.
-    /// `None` if the macro is not expanded yet.
-    crate output_legacy_scope: Cell<Option<LegacyScope<'a>>>,
-}
-
-impl<'a> InvocationData<'a> {
-    pub fn default(module: Module<'a>) -> Self {
-        InvocationData {
-            module,
-            parent_legacy_scope: LegacyScope::Empty,
-            output_legacy_scope: Cell::new(None),
-        }
-    }
-}
-
 /// Binding produced by a `macro_rules` item.
 /// Not modularized, can shadow previous legacy bindings, etc.
 #[derive(Debug)]
@@ -75,7 +50,7 @@ pub enum LegacyScope<'a> {
     Binding(&'a LegacyBinding<'a>),
     /// The scope introduced by a macro invocation that can potentially
     /// create a `macro_rules!` macro definition.
-    Invocation(&'a InvocationData<'a>),
+    Invocation(ExpnId),
 }
 
 // Macro namespace is separated into two sub-namespaces, one for bang macros and
@@ -124,9 +99,8 @@ impl<'a> base::Resolver for Resolver<'a> {
             ExpnKind::Macro(MacroKind::Attr, sym::test_case), DUMMY_SP, self.session.edition()
         )));
         let module = self.module_map[&self.definitions.local_def_id(id)];
-        let invocation_data = self.arenas.alloc_invocation_data(InvocationData::default(module));
+        self.invocation_parent_scopes.insert(expn_id, ParentScope::default(module));
         self.definitions.set_invocation_parent(expn_id, module.def_id().unwrap().index);
-        self.invocations.insert(expn_id, invocation_data);
         expn_id
     }
 
@@ -140,29 +114,29 @@ impl<'a> base::Resolver for Resolver<'a> {
         });
     }
 
-    fn visit_ast_fragment_with_placeholders(&mut self, expn_id: ExpnId, fragment: &AstFragment,
-                                            derives: &[ExpnId]) {
-        fragment.visit_with(&mut DefCollector::new(&mut self.definitions, expn_id));
-
-        let invocation = self.invocations[&expn_id];
-        invocation.module.unresolved_invocations.borrow_mut().remove(&expn_id);
-        invocation.module.unresolved_invocations.borrow_mut().extend(derives);
-        let parent_def = self.definitions.invocation_parent(expn_id);
+    fn visit_ast_fragment_with_placeholders(
+        &mut self, expansion: ExpnId, fragment: &AstFragment, derives: &[ExpnId]
+    ) {
+        // Fill in some data for derives if the fragment is from a derive container.
+        let parent_scope = self.invocation_parent_scopes[&expansion].clone();
+        let parent_def = self.definitions.invocation_parent(expansion);
+        self.invocation_parent_scopes.extend(
+            derives.iter().map(|&derive| (derive, parent_scope.clone()))
+        );
         for &derive_invoc_id in derives {
             self.definitions.set_invocation_parent(derive_invoc_id, parent_def);
         }
-        self.invocations.extend(derives.iter().map(|&derive| (derive, invocation)));
-        let mut visitor = BuildReducedGraphVisitor {
-            r: self,
-            parent_scope: ParentScope {
-                module: invocation.module,
-                expansion: expn_id,
-                legacy: invocation.parent_legacy_scope,
-                derives: Vec::new(),
-            },
-        };
+        parent_scope.module.unresolved_invocations.borrow_mut().remove(&expansion);
+        parent_scope.module.unresolved_invocations.borrow_mut().extend(derives);
+
+        // Integrate the new AST fragment into all the definition and module structures.
+        // We are inside the `expansion` new, but other parent scope components are still the same.
+        fragment.visit_with(&mut DefCollector::new(&mut self.definitions, expansion));
+        let parent_scope = ParentScope { expansion, ..parent_scope };
+        let mut visitor = BuildReducedGraphVisitor { r: self, parent_scope };
         fragment.visit_with(&mut visitor);
-        invocation.output_legacy_scope.set(Some(visitor.parent_scope.legacy));
+        let output_legacy_scope = visitor.parent_scope.legacy;
+        self.output_legacy_scopes.insert(expansion, output_legacy_scope);
     }
 
     fn register_builtin_macro(&mut self, ident: ast::Ident, ext: SyntaxExtension) {
@@ -178,7 +152,8 @@ impl<'a> base::Resolver for Resolver<'a> {
 
     fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: ExpnId, force: bool)
                                 -> Result<Option<Lrc<SyntaxExtension>>, Indeterminate> {
-        let (path, kind, derives_in_scope, after_derive) = match invoc.kind {
+        let parent_scope = &self.invocation_parent_scopes[&invoc_id].clone();
+        let (path, kind, derives, after_derive) = match invoc.kind {
             InvocationKind::Attr { ref attr, ref derives, after_derive, .. } =>
                 (&attr.path, MacroKind::Attr, derives.clone(), after_derive),
             InvocationKind::Bang { ref mac, .. } =>
@@ -192,7 +167,6 @@ impl<'a> base::Resolver for Resolver<'a> {
                 // will automatically knows about itself.
                 let mut result = Ok(None);
                 if derives.len() > 1 {
-                    let parent_scope = &self.invoc_parent_scope(invoc_id, Vec::new());
                     for path in derives {
                         match self.resolve_macro_path(path, Some(MacroKind::Derive),
                                                       parent_scope, true, force) {
@@ -209,7 +183,8 @@ impl<'a> base::Resolver for Resolver<'a> {
             }
         };
 
-        let parent_scope = &self.invoc_parent_scope(invoc_id, derives_in_scope);
+        // Derives are not included when `invocations` are collected, so we have to add them here.
+        let parent_scope = &ParentScope { derives, ..parent_scope.clone() };
         let (ext, res) = self.smart_resolve_macro_path(path, kind, parent_scope, force)?;
 
         let span = invoc.span();
@@ -247,16 +222,6 @@ impl<'a> base::Resolver for Resolver<'a> {
 }
 
 impl<'a> Resolver<'a> {
-    fn invoc_parent_scope(&self, invoc_id: ExpnId, derives: Vec<ast::Path>) -> ParentScope<'a> {
-        let invoc = self.invocations[&invoc_id];
-        ParentScope {
-            module: invoc.module,
-            expansion: invoc_id.parent(),
-            legacy: invoc.parent_legacy_scope,
-            derives,
-        }
-    }
-
     /// Resolve macro path with error reporting and recovery.
     fn smart_resolve_macro_path(
         &mut self,
@@ -466,8 +431,9 @@ impl<'a> Resolver<'a> {
                 Scope::MacroRules(legacy_scope) => match legacy_scope {
                     LegacyScope::Binding(legacy_binding) if ident == legacy_binding.ident =>
                         Ok((legacy_binding.binding, Flags::MACRO_RULES)),
-                    LegacyScope::Invocation(invoc) if invoc.output_legacy_scope.get().is_none() =>
-                        Err(Determinacy::Undetermined),
+                    LegacyScope::Invocation(invoc_id)
+                        if !this.output_legacy_scopes.contains_key(&invoc_id) =>
+                            Err(Determinacy::Undetermined),
                     _ => Err(Determinacy::Determined),
                 }
                 Scope::CrateRoot => {
