resolve: Move macro resolution traces from Modules to Resolver

petrochenkov · petrochenkov · commit 23b82c322980 · 2019-08-15T20:39:27.000+03:00
Traces already contain module info without that.
It's easy to forget to call `finalize_*` on a module.
In particular, macros enum and trait modules weren't finalized.
By happy accident macros weren't placed into those modules until now.
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
@@ -1267,9 +1267,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> {
 
     fn visit_attribute(&mut self, attr: &'b ast::Attribute) {
         if !attr.is_sugared_doc && is_builtin_attr(attr) {
-            self.parent_scope.module.builtin_attrs.borrow_mut().push((
-                attr.path.segments[0].ident, self.parent_scope.clone()
-            ));
+            self.r.builtin_attrs.push((attr.path.segments[0].ident, self.parent_scope.clone()));
         }
         visit::walk_attribute(self, attr);
     }
diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs
@@ -574,7 +574,6 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
             self.ribs[ValueNS].push(Rib::new(ModuleRibKind(module)));
             self.ribs[TypeNS].push(Rib::new(ModuleRibKind(module)));
 
-            self.r.finalize_current_module_macro_resolutions(module);
             let ret = f(self);
 
             self.parent_scope.module = orig_module;
@@ -1227,7 +1226,6 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
             self.ribs[ValueNS].push(Rib::new(ModuleRibKind(anonymous_module)));
             self.ribs[TypeNS].push(Rib::new(ModuleRibKind(anonymous_module)));
             self.parent_scope.module = anonymous_module;
-            self.r.finalize_current_module_macro_resolutions(anonymous_module);
         } else {
             self.ribs[ValueNS].push(Rib::new(NormalRibKind));
         }
@@ -1984,7 +1982,6 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
 
 impl<'a> Resolver<'a> {
     pub(crate) fn late_resolve_crate(&mut self, krate: &Crate) {
-        self.finalize_current_module_macro_resolutions(self.graph_root);
         let mut late_resolution_visitor = LateResolutionVisitor::new(self);
         visit::walk_crate(&mut late_resolution_visitor, krate);
         for (id, span) in late_resolution_visitor.unused_labels.iter() {
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
@@ -418,11 +418,6 @@ pub struct ModuleData<'a> {
     normal_ancestor_id: DefId,
 
     resolutions: RefCell<FxHashMap<(Ident, Namespace), &'a RefCell<NameResolution<'a>>>>,
-    single_segment_macro_resolutions: RefCell<Vec<(Ident, MacroKind, ParentScope<'a>,
-                                                   Option<&'a NameBinding<'a>>)>>,
-    multi_segment_macro_resolutions: RefCell<Vec<(Vec<Segment>, Span, MacroKind, ParentScope<'a>,
-                                                  Option<Res>)>>,
-    builtin_attrs: RefCell<Vec<(Ident, ParentScope<'a>)>>,
 
     // Macro invocations that can expand into items in this module.
     unresolved_invocations: RefCell<FxHashSet<ExpnId>>,
@@ -459,9 +454,6 @@ impl<'a> ModuleData<'a> {
             kind,
             normal_ancestor_id,
             resolutions: Default::default(),
-            single_segment_macro_resolutions: RefCell::new(Vec::new()),
-            multi_segment_macro_resolutions: RefCell::new(Vec::new()),
-            builtin_attrs: RefCell::new(Vec::new()),
             unresolved_invocations: Default::default(),
             no_implicit_prelude: false,
             glob_importers: RefCell::new(Vec::new()),
@@ -896,6 +888,12 @@ pub struct Resolver<'a> {
     local_macro_def_scopes: FxHashMap<NodeId, Module<'a>>,
     unused_macros: NodeMap<Span>,
     proc_macro_stubs: NodeSet,
+    /// Traces collected during macro resolution and validated when it's complete.
+    single_segment_macro_resolutions: Vec<(Ident, MacroKind, ParentScope<'a>,
+                                           Option<&'a NameBinding<'a>>)>,
+    multi_segment_macro_resolutions: Vec<(Vec<Segment>, Span, MacroKind, ParentScope<'a>,
+                                          Option<Res>)>,
+    builtin_attrs: Vec<(Ident, ParentScope<'a>)>,
     /// Some built-in derives mark items they are applied to so they are treated specially later.
     /// Derive macros cannot modify the item themselves and have to store the markers in the global
     /// context, so they attach the markers to derive container IDs using this resolver table.
@@ -1151,6 +1149,9 @@ impl<'a> Resolver<'a> {
             struct_constructors: Default::default(),
             unused_macros: Default::default(),
             proc_macro_stubs: Default::default(),
+            single_segment_macro_resolutions: Default::default(),
+            multi_segment_macro_resolutions: Default::default(),
+            builtin_attrs: Default::default(),
             special_derives: Default::default(),
             active_features:
                 features.declared_lib_features.iter().map(|(feat, ..)| *feat)
@@ -1203,6 +1204,7 @@ impl<'a> Resolver<'a> {
     /// Entry point to crate resolution.
     pub fn resolve_crate(&mut self, krate: &Crate) {
         ImportResolver { r: self }.finalize_imports();
+        self.finalize_macro_resolutions();
 
         self.late_resolve_crate(krate);
 
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
@@ -366,7 +366,7 @@ impl<'a> Resolver<'a> {
 
             if trace {
                 let kind = kind.expect("macro kind must be specified if tracing is enabled");
-                parent_scope.module.multi_segment_macro_resolutions.borrow_mut()
+                self.multi_segment_macro_resolutions
                     .push((path, path_span, kind, parent_scope.clone(), res.ok()));
             }
 
@@ -383,7 +383,7 @@ impl<'a> Resolver<'a> {
 
             if trace {
                 let kind = kind.expect("macro kind must be specified if tracing is enabled");
-                parent_scope.module.single_segment_macro_resolutions.borrow_mut()
+                self.single_segment_macro_resolutions
                     .push((path[0].ident, kind, parent_scope.clone(), binding.ok()));
             }
 
@@ -693,7 +693,7 @@ impl<'a> Resolver<'a> {
         }
     }
 
-    pub fn finalize_current_module_macro_resolutions(&mut self, module: Module<'a>) {
+    crate fn finalize_macro_resolutions(&mut self) {
         let check_consistency = |this: &mut Self, path: &[Segment], span, kind: MacroKind,
                                  initial_res: Option<Res>, res: Res| {
             if let Some(initial_res) = initial_res {
@@ -729,8 +729,7 @@ impl<'a> Resolver<'a> {
             }
         };
 
-        let macro_resolutions =
-            mem::take(&mut *module.multi_segment_macro_resolutions.borrow_mut());
+        let macro_resolutions = mem::take(&mut self.multi_segment_macro_resolutions);
         for (mut path, path_span, kind, parent_scope, initial_res) in macro_resolutions {
             // FIXME: Path resolution will ICE if segment IDs present.
             for seg in &mut path { seg.id = None; }
@@ -757,8 +756,7 @@ impl<'a> Resolver<'a> {
             }
         }
 
-        let macro_resolutions =
-            mem::take(&mut *module.single_segment_macro_resolutions.borrow_mut());
+        let macro_resolutions = mem::take(&mut self.single_segment_macro_resolutions);
         for (ident, kind, parent_scope, initial_binding) in macro_resolutions {
             match self.early_resolve_ident_in_lexical_scope(ident, ScopeSet::Macro(kind),
                                                             &parent_scope, true, true, ident.span) {
@@ -783,7 +781,7 @@ impl<'a> Resolver<'a> {
             }
         }
 
-        let builtin_attrs = mem::take(&mut *module.builtin_attrs.borrow_mut());
+        let builtin_attrs = mem::take(&mut self.builtin_attrs);
         for (ident, parent_scope) in builtin_attrs {
             let _ = self.early_resolve_ident_in_lexical_scope(
                 ident, ScopeSet::Macro(MacroKind::Attr), &parent_scope, true, true, ident.span
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
@@ -848,7 +848,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         directive.vis.set(orig_vis);
         let module = match path_res {
             PathResult::Module(module) => {
-                // Consistency checks, analogous to `finalize_current_module_macro_resolutions`.
+                // Consistency checks, analogous to `finalize_macro_resolutions`.
                 if let Some(initial_module) = directive.imported_module.get() {
                     if !ModuleOrUniformRoot::same_def(module, initial_module) && no_ambiguity {
                         span_bug!(directive.span, "inconsistent resolution for an import");
@@ -973,7 +973,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
 
             match binding {
                 Ok(binding) => {
-                    // Consistency checks, analogous to `finalize_current_module_macro_resolutions`.
+                    // Consistency checks, analogous to `finalize_macro_resolutions`.
                     let initial_res = source_bindings[ns].get().map(|initial_binding| {
                         all_ns_err = false;
                         if let Some(target_binding) = target_bindings[ns].get() {
diff --git a/src/test/ui/feature-gates/feature-gate-rustc-diagnostic-macros.stderr b/src/test/ui/feature-gates/feature-gate-rustc-diagnostic-macros.stderr
@@ -4,17 +4,17 @@ error: cannot find macro `__build_diagnostic_array!` in this scope
 LL | __build_diagnostic_array!(DIAGNOSTICS);
    | ^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: cannot find macro `__register_diagnostic!` in this scope
-  --> $DIR/feature-gate-rustc-diagnostic-macros.rs:4:1
-   |
-LL | __register_diagnostic!(E0001);
-   | ^^^^^^^^^^^^^^^^^^^^^
-
 error: cannot find macro `__diagnostic_used!` in this scope
   --> $DIR/feature-gate-rustc-diagnostic-macros.rs:8:5
    |
 LL |     __diagnostic_used!(E0001);
    |     ^^^^^^^^^^^^^^^^^
 
+error: cannot find macro `__register_diagnostic!` in this scope
+  --> $DIR/feature-gate-rustc-diagnostic-macros.rs:4:1
+   |
+LL | __register_diagnostic!(E0001);
+   | ^^^^^^^^^^^^^^^^^^^^^
+
 error: aborting due to 3 previous errors
 
diff --git a/src/test/ui/hygiene/no_implicit_prelude.stderr b/src/test/ui/hygiene/no_implicit_prelude.stderr
@@ -1,3 +1,11 @@
+error: cannot find macro `panic!` in this scope
+  --> $DIR/no_implicit_prelude.rs:16:9
+   |
+LL |         assert_eq!(0, 0);
+   |         ^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+
 error[E0433]: failed to resolve: use of undeclared type or module `Vec`
   --> $DIR/no_implicit_prelude.rs:11:9
    |
@@ -7,14 +15,6 @@ LL |     fn f() { ::bar::m!(); }
 LL |         Vec::new();
    |         ^^^ use of undeclared type or module `Vec`
 
-error: cannot find macro `panic!` in this scope
-  --> $DIR/no_implicit_prelude.rs:16:9
-   |
-LL |         assert_eq!(0, 0);
-   |         ^^^^^^^^^^^^^^^^^
-   |
-   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
-
 error[E0599]: no method named `clone` found for type `()` in the current scope
   --> $DIR/no_implicit_prelude.rs:12:12
    |
diff --git a/src/test/ui/imports/local-modularized-tricky-fail-1.stderr b/src/test/ui/imports/local-modularized-tricky-fail-1.stderr
@@ -21,25 +21,6 @@ LL | use inner1::*;
    |     ^^^^^^^^^
    = help: consider adding an explicit import of `exported` to disambiguate
 
-error[E0659]: `include` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution)
-  --> $DIR/local-modularized-tricky-fail-1.rs:46:1
-   |
-LL | include!();
-   | ^^^^^^^ ambiguous name
-   |
-   = note: `include` could refer to a macro from prelude
-note: `include` could also refer to the macro defined here
-  --> $DIR/local-modularized-tricky-fail-1.rs:17:5
-   |
-LL | /     macro_rules! include {
-LL | |         () => ()
-LL | |     }
-   | |_____^
-...
-LL |       define_include!();
-   |       ------------------ in this macro invocation
-   = help: use `crate::include` to refer to this macro unambiguously
-
 error[E0659]: `panic` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution)
   --> $DIR/local-modularized-tricky-fail-1.rs:35:5
    |
@@ -59,6 +40,25 @@ LL |       define_panic!();
    |       ---------------- in this macro invocation
    = help: use `crate::panic` to refer to this macro unambiguously
 
+error[E0659]: `include` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution)
+  --> $DIR/local-modularized-tricky-fail-1.rs:46:1
+   |
+LL | include!();
+   | ^^^^^^^ ambiguous name
+   |
+   = note: `include` could refer to a macro from prelude
+note: `include` could also refer to the macro defined here
+  --> $DIR/local-modularized-tricky-fail-1.rs:17:5
+   |
+LL | /     macro_rules! include {
+LL | |         () => ()
+LL | |     }
+   | |_____^
+...
+LL |       define_include!();
+   |       ------------------ in this macro invocation
+   = help: use `crate::include` to refer to this macro unambiguously
+
 error: aborting due to 3 previous errors
 
 For more information about this error, try `rustc --explain E0659`.
diff --git a/src/test/ui/imports/shadow_builtin_macros.stderr b/src/test/ui/imports/shadow_builtin_macros.stderr
@@ -13,20 +13,6 @@ LL |     use foo::*;
    = help: consider adding an explicit import of `panic` to disambiguate
    = help: or use `self::panic` to refer to this macro unambiguously
 
-error[E0659]: `panic` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution)
-  --> $DIR/shadow_builtin_macros.rs:20:14
-   |
-LL |     fn f() { panic!(); }
-   |              ^^^^^ ambiguous name
-   |
-   = note: `panic` could refer to a macro from prelude
-note: `panic` could also refer to the macro imported here
-  --> $DIR/shadow_builtin_macros.rs:19:26
-   |
-LL |     ::two_macros::m!(use foo::panic;);
-   |                          ^^^^^^^^^^
-   = help: use `self::panic` to refer to this macro unambiguously
-
 error[E0659]: `panic` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution)
   --> $DIR/shadow_builtin_macros.rs:33:5
    |
@@ -62,6 +48,20 @@ note: `n` could also refer to the macro imported here
 LL | #[macro_use(n)]
    |             ^
 
+error[E0659]: `panic` is ambiguous (macro-expanded name vs less macro-expanded name from outer scope during import/macro resolution)
+  --> $DIR/shadow_builtin_macros.rs:20:14
+   |
+LL |     fn f() { panic!(); }
+   |              ^^^^^ ambiguous name
+   |
+   = note: `panic` could refer to a macro from prelude
+note: `panic` could also refer to the macro imported here
+  --> $DIR/shadow_builtin_macros.rs:19:26
+   |
+LL |     ::two_macros::m!(use foo::panic;);
+   |                          ^^^^^^^^^^
+   = help: use `self::panic` to refer to this macro unambiguously
+
 error: aborting due to 4 previous errors
 
 For more information about this error, try `rustc --explain E0659`.
diff --git a/src/test/ui/issues/issue-49074.stderr b/src/test/ui/issues/issue-49074.stderr
@@ -1,9 +1,3 @@
-error: cannot find attribute macro `marco_use` in this scope
-  --> $DIR/issue-49074.rs:3:3
-   |
-LL | #[marco_use] // typo
-   |   ^^^^^^^^^ help: a built-in attribute with a similar name exists: `macro_use`
-
 error: cannot find macro `bar!` in this scope
   --> $DIR/issue-49074.rs:12:4
    |
@@ -12,5 +6,11 @@ LL |    bar!();
    |
    = help: have you added the `#[macro_use]` on the module/import?
 
+error: cannot find attribute macro `marco_use` in this scope
+  --> $DIR/issue-49074.rs:3:3
+   |
+LL | #[marco_use] // typo
+   |   ^^^^^^^^^ help: a built-in attribute with a similar name exists: `macro_use`
+
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/proc-macro/macro-namespace-reserved-2.stderr b/src/test/ui/proc-macro/macro-namespace-reserved-2.stderr
@@ -88,18 +88,6 @@ error: expected derive macro, found macro `crate::my_macro`
 LL | #[derive(crate::my_macro)]
    |          ^^^^^^^^^^^^^^^ not a derive macro
 
-error: cannot find attribute macro `my_macro` in this scope
-  --> $DIR/macro-namespace-reserved-2.rs:38:3
-   |
-LL | #[my_macro]
-   |   ^^^^^^^^
-
-error: cannot find derive macro `my_macro` in this scope
-  --> $DIR/macro-namespace-reserved-2.rs:48:10
-   |
-LL | #[derive(my_macro)]
-   |          ^^^^^^^^
-
 error: cannot find macro `my_macro_attr!` in this scope
   --> $DIR/macro-namespace-reserved-2.rs:28:5
    |
@@ -112,5 +100,17 @@ error: cannot find macro `MyTrait!` in this scope
 LL |     MyTrait!();
    |     ^^^^^^^
 
+error: cannot find attribute macro `my_macro` in this scope
+  --> $DIR/macro-namespace-reserved-2.rs:38:3
+   |
+LL | #[my_macro]
+   |   ^^^^^^^^
+
+error: cannot find derive macro `my_macro` in this scope
+  --> $DIR/macro-namespace-reserved-2.rs:48:10
+   |
+LL | #[derive(my_macro)]
+   |          ^^^^^^^^
+
 error: aborting due to 19 previous errors
 
diff --git a/src/test/ui/reserved/reserved-attr-on-macro.stderr b/src/test/ui/reserved/reserved-attr-on-macro.stderr
@@ -7,12 +7,6 @@ LL | #[rustc_attribute_should_be_reserved]
    = note: for more information, see https://github.com/rust-lang/rust/issues/29642
    = help: add `#![feature(rustc_attrs)]` to the crate attributes to enable
 
-error: cannot find attribute macro `rustc_attribute_should_be_reserved` in this scope
-  --> $DIR/reserved-attr-on-macro.rs:1:3
-   |
-LL | #[rustc_attribute_should_be_reserved]
-   |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
 error: cannot determine resolution for the macro `foo`
   --> $DIR/reserved-attr-on-macro.rs:10:5
    |
@@ -21,6 +15,12 @@ LL |     foo!();
    |
    = note: import resolution is stuck, try simplifying macro imports
 
+error: cannot find attribute macro `rustc_attribute_should_be_reserved` in this scope
+  --> $DIR/reserved-attr-on-macro.rs:1:3
+   |
+LL | #[rustc_attribute_should_be_reserved]
+   |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
 error: aborting due to 3 previous errors
 
 For more information about this error, try `rustc --explain E0658`.

Original file line number	Diff line number	Diff line change
`@@ -1267,9 +1267,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> {`
`1267`	`1267`
`1268`	`1268`	`fn visit_attribute(&mut self, attr: &'b ast::Attribute) {`
`1269`	`1269`	`if !attr.is_sugared_doc && is_builtin_attr(attr) {`
`1270`		`- self.parent_scope.module.builtin_attrs.borrow_mut().push((`
`1271`		`- attr.path.segments[0].ident, self.parent_scope.clone()`
`1272`		`- ));`
	`1270`	`+ self.r.builtin_attrs.push((attr.path.segments[0].ident, self.parent_scope.clone()));`
`1273`	`1271`	`}`
`1274`	`1272`	`visit::walk_attribute(self, attr);`
`1275`	`1273`	`}`
Original file line number	Diff line number	Diff line change
`@@ -366,7 +366,7 @@ impl<'a> Resolver<'a> {`
`366`	`366`
`367`	`367`	`if trace {`
`368`	`368`	`let kind = kind.expect("macro kind must be specified if tracing is enabled");`
`369`		`- parent_scope.module.multi_segment_macro_resolutions.borrow_mut()`
	`369`	`+ self.multi_segment_macro_resolutions`
`370`	`370`	`.push((path, path_span, kind, parent_scope.clone(), res.ok()));`
`371`	`371`	`}`
`372`	`372`
`@@ -383,7 +383,7 @@ impl<'a> Resolver<'a> {`
`383`	`383`
`384`	`384`	`if trace {`
`385`	`385`	`let kind = kind.expect("macro kind must be specified if tracing is enabled");`
`386`		`- parent_scope.module.single_segment_macro_resolutions.borrow_mut()`
	`386`	`+ self.single_segment_macro_resolutions`
`387`	`387`	`.push((path[0].ident, kind, parent_scope.clone(), binding.ok()));`
`388`	`388`	`}`
`389`	`389`
`@@ -693,7 +693,7 @@ impl<'a> Resolver<'a> {`
`693`	`693`	`}`
`694`	`694`	`}`
`695`	`695`
`696`		`- pub fn finalize_current_module_macro_resolutions(&mut self, module: Module<'a>) {`
	`696`	`+ crate fn finalize_macro_resolutions(&mut self) {`
`697`	`697`	`let check_consistency = \|this: &mut Self, path: &[Segment], span, kind: MacroKind,`
`698`	`698`	`initial_res: Option<Res>, res: Res\| {`
`699`	`699`	`if let Some(initial_res) = initial_res {`
`@@ -729,8 +729,7 @@ impl<'a> Resolver<'a> {`
`729`	`729`	`}`
`730`	`730`	`};`
`731`	`731`
`732`		`- let macro_resolutions =`
`733`		`- mem::take(&mut *module.multi_segment_macro_resolutions.borrow_mut());`
	`732`	`+ let macro_resolutions = mem::take(&mut self.multi_segment_macro_resolutions);`
`734`	`733`	`for (mut path, path_span, kind, parent_scope, initial_res) in macro_resolutions {`
`735`	`734`	`// FIXME: Path resolution will ICE if segment IDs present.`
`736`	`735`	`for seg in &mut path { seg.id = None; }`
`@@ -757,8 +756,7 @@ impl<'a> Resolver<'a> {`
`757`	`756`	`}`
`758`	`757`	`}`
`759`	`758`
`760`		`- let macro_resolutions =`
`761`		`- mem::take(&mut *module.single_segment_macro_resolutions.borrow_mut());`
	`759`	`+ let macro_resolutions = mem::take(&mut self.single_segment_macro_resolutions);`
`762`	`760`	`for (ident, kind, parent_scope, initial_binding) in macro_resolutions {`
`763`	`761`	`match self.early_resolve_ident_in_lexical_scope(ident, ScopeSet::Macro(kind),`
`764`	`762`	`&parent_scope, true, true, ident.span) {`
`@@ -783,7 +781,7 @@ impl<'a> Resolver<'a> {`
`783`	`781`	`}`
`784`	`782`	`}`
`785`	`783`
`786`		`- let builtin_attrs = mem::take(&mut *module.builtin_attrs.borrow_mut());`
	`784`	`+ let builtin_attrs = mem::take(&mut self.builtin_attrs);`
`787`	`785`	`for (ident, parent_scope) in builtin_attrs {`
`788`	`786`	`let _ = self.early_resolve_ident_in_lexical_scope(`
`789`	`787`	`ident, ScopeSet::Macro(MacroKind::Attr), &parent_scope, true, true, ident.span`