resolve: Further simplify legacy scopes, add comments

src/librustc_resolve/build_reduced_graph.rs

@@ -961,6 +961,7 @@ impl<'a, 'b, 'cl> BuildReducedGraphVisitor<'a, 'b, 'cl> {
         let invocation = self.resolver.invocations[&mark];
         invocation.module.set(self.resolver.current_module);
         invocation.parent_legacy_scope.set(self.current_legacy_scope);
+        invocation.output_legacy_scope.set(self.current_legacy_scope);
         invocation
     }
 }
@@ -990,7 +991,7 @@ impl<'a, 'b, 'cl> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b, 'cl> {
                 return
             }
             ItemKind::Mac(..) => {
-                self.current_legacy_scope = LegacyScope::Expansion(self.visit_invoc(item.id));
+                self.current_legacy_scope = LegacyScope::Invocation(self.visit_invoc(item.id));
                 return
             }
             ItemKind::Mod(..) => self.resolver.contains_macro_use(&item.attrs),
@@ -1009,7 +1010,7 @@ impl<'a, 'b, 'cl> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b, 'cl> {
 
     fn visit_stmt(&mut self, stmt: &'a ast::Stmt) {
         if let ast::StmtKind::Mac(..) = stmt.node {
-            self.current_legacy_scope = LegacyScope::Expansion(self.visit_invoc(stmt.id));
+            self.current_legacy_scope = LegacyScope::Invocation(self.visit_invoc(stmt.id));
         } else {
             visit::walk_stmt(self, stmt);
         }

src/librustc_resolve/macros.rs

@@ -50,15 +50,16 @@ crate struct FromPrelude(bool);
 
 #[derive(Clone)]
 pub struct InvocationData<'a> {
-    crate module: Cell<Module<'a>>,
     def_index: DefIndex,
-    // Legacy scope in which the macro was invoked.
-    // The invocation path is resolved in this scope.
+    /// Module in which the macro was invoked.
+    crate module: Cell<Module<'a>>,
+    /// Legacy scope in which the macro was invoked.
+    /// The invocation path is resolved in this scope.
     crate parent_legacy_scope: Cell<LegacyScope<'a>>,
-    // Legacy scope *produced* by expanding this macro invocation,
-    // includes all the macro_rules items, other invocations, etc generated by it.
-    // `Empty` is used if for invocations that has not been expanded yet.
-    output_legacy_scope: Cell<LegacyScope<'a>>,
+    /// Legacy scope *produced* by expanding this macro invocation,
+    /// includes all the macro_rules items, other invocations, etc generated by it.
+    /// Set to the parent scope if the macro is not expanded yet (as if the macro produced nothing).
+    crate output_legacy_scope: Cell<LegacyScope<'a>>,
 }
 
 impl<'a> InvocationData<'a> {
@@ -72,21 +73,32 @@ impl<'a> InvocationData<'a> {
     }
 }
 
-// Binding produced by a `macro_rules` item.
-// Not modularized, can shadow previous legacy bindings, etc.
+/// Binding produced by a `macro_rules` item.
+/// Not modularized, can shadow previous legacy bindings, etc.
 pub struct LegacyBinding<'a> {
     binding: &'a NameBinding<'a>,
-    // Legacy scope into which the `macro_rules` item was planted.
-    parent_legacy_scope: Cell<LegacyScope<'a>>,
+    /// Legacy scope into which the `macro_rules` item was planted.
+    parent_legacy_scope: LegacyScope<'a>,
     ident: Ident,
 }
 
+/// Scope introduced by a `macro_rules!` macro.
+/// Starts at the macro's definition and ends at the end of the macro's parent module
+/// (named or unnamed), or even further if it escapes with `#[macro_use]`.
+/// Some macro invocations need to introduce legacy scopes too because they
+/// potentially can expand into macro definitions.
 #[derive(Copy, Clone)]
 pub enum LegacyScope<'a> {
+    /// Created when invocation data is allocated in the arena,
+    /// must be replaced with a proper scope later.
+    Uninitialized,
+    /// Empty "root" scope at the crate start containing no names.
     Empty,
-    Invocation(&'a InvocationData<'a>), // The scope of the invocation, not including its expansion
-    Expansion(&'a InvocationData<'a>), // The scope of the invocation, including its expansion
+    /// Scope introduced by a `macro_rules!` macro definition.
     Binding(&'a LegacyBinding<'a>),
+    /// Scope introduced by a macro invocation that can potentially
+    /// create a `macro_rules!` macro definition.
+    Invocation(&'a InvocationData<'a>),
 }
 
 pub struct ProcMacError {
@@ -181,7 +193,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
         }
         let mut visitor = BuildReducedGraphVisitor {
             resolver: self,
-            current_legacy_scope: LegacyScope::Invocation(invocation),
+            current_legacy_scope: invocation.parent_legacy_scope.get(),
             expansion: mark,
         };
         fragment.visit_with(&mut visitor);
@@ -483,8 +495,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             }
         }
 
-        let legacy_resolution =
-            self.resolve_legacy_scope(path[0], invoc_id, &invocation.parent_legacy_scope, false);
+        let legacy_resolution = self.resolve_legacy_scope(
+            path[0], invoc_id, invocation.parent_legacy_scope.get(), false
+        );
         let result = if let Some(legacy_binding) = legacy_resolution {
             Ok(legacy_binding.def())
         } else {
@@ -788,7 +801,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
     fn resolve_legacy_scope(&mut self,
                             ident: Ident,
                             invoc_id: Mark,
-                            invoc_parent_legacy_scope: &'a Cell<LegacyScope<'a>>,
+                            invoc_parent_legacy_scope: LegacyScope<'a>,
                             record_used: bool)
                             -> Option<&'a NameBinding<'a>> {
         let ident = ident.modern();
@@ -809,28 +822,18 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         // Go through all the scopes and try to resolve the name.
         let mut where_to_resolve = invoc_parent_legacy_scope;
         loop {
-            let result = match where_to_resolve.get() {
+            let result = match where_to_resolve {
                 LegacyScope::Binding(legacy_binding) if ident == legacy_binding.ident =>
                     Some(legacy_binding.binding),
                 _ => None,
             };
 
             macro_rules! continue_search { () => {
-                where_to_resolve = match where_to_resolve.get() {
+                where_to_resolve = match where_to_resolve {
                     LegacyScope::Empty => break, // nowhere else to search
-                    LegacyScope::Binding(binding) => &binding.parent_legacy_scope,
-                    LegacyScope::Invocation(invocation) => &invocation.parent_legacy_scope,
-                    LegacyScope::Expansion(invocation) => {
-                        match invocation.output_legacy_scope.get() {
-                            LegacyScope::Empty => &invocation.parent_legacy_scope,
-                            LegacyScope::Binding(..) |
-                            LegacyScope::Expansion(..) => &invocation.output_legacy_scope,
-                            LegacyScope::Invocation(..) => {
-                                where_to_resolve.set(invocation.parent_legacy_scope.get());
-                                where_to_resolve
-                            }
-                        }
-                    }
+                    LegacyScope::Binding(binding) => binding.parent_legacy_scope,
+                    LegacyScope::Invocation(invocation) => invocation.output_legacy_scope.get(),
+                    LegacyScope::Uninitialized => unreachable!(),
                 };
 
                 continue;
@@ -885,9 +888,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
 
         for &(invoc_id, ident, kind, def) in module.legacy_macro_resolutions.borrow().iter() {
             let span = ident.span;
-            let invoc_parent_legacy_scope = &self.invocations[&invoc_id].parent_legacy_scope;
-            let legacy_resolution =
-                self.resolve_legacy_scope(ident, invoc_id, invoc_parent_legacy_scope, true);
+            let invocation = self.invocations[&invoc_id];
+            let legacy_resolution = self.resolve_legacy_scope(
+                ident, invoc_id, invocation.parent_legacy_scope.get(), true
+            );
             let resolution = self.resolve_lexical_macro_path_segment(
                 ident, MacroNS, invoc_id, true, true, kind == MacroKind::Attr, span
             );
@@ -1013,8 +1017,8 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                 arenas.alloc_invocation_data(InvocationData {
                     def_index: invoc.def_index,
                     module: Cell::new(graph_root),
-                    parent_legacy_scope: Cell::new(LegacyScope::Empty),
-                    output_legacy_scope: Cell::new(LegacyScope::Empty),
+                    parent_legacy_scope: Cell::new(LegacyScope::Uninitialized),
+                    output_legacy_scope: Cell::new(LegacyScope::Uninitialized),
                 })
             });
         };
@@ -1050,7 +1054,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             let vis = ty::Visibility::Invisible; // Doesn't matter for legacy bindings
             let binding = (def, vis, item.span, expansion).to_name_binding(self.arenas);
             let legacy_binding = self.arenas.alloc_legacy_binding(LegacyBinding {
-                parent_legacy_scope: Cell::new(*current_legacy_scope), binding, ident
+                parent_legacy_scope: *current_legacy_scope, binding, ident
             });
             *current_legacy_scope = LegacyScope::Binding(legacy_binding);
             self.all_macros.insert(ident.name, def);