resolve: Populate external modules in more automatic and lazy way

The modules are now populated implicitly on the first access

Author: petrochenkov

src/librustc_resolve/build_reduced_graph.rs

@@ -157,19 +157,6 @@ impl<'a> Resolver<'a> {
         self.macro_map.insert(def_id, ext.clone());
         Some(ext)
     }
-
-    /// Ensures that the reduced graph rooted at the given external module
-    /// is built, building it if it is not.
-    pub fn populate_module_if_necessary(&mut self, module: Module<'a>) {
-        if module.populated.get() { return }
-        let def_id = module.def_id().unwrap();
-        for child in self.cstore.item_children_untracked(def_id, self.session) {
-            let child = child.map_id(|_| panic!("unexpected id"));
-            BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self }
-                .build_reduced_graph_for_external_crate_res(module, child);
-        }
-        module.populated.set(true)
-    }
 }
 
 pub struct BuildReducedGraphVisitor<'a, 'b> {
@@ -595,7 +582,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                     self.r.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX })
                 };
 
-                self.r.populate_module_if_necessary(module);
                 if let Some(name) = self.r.session.parse_sess.injected_crate_name.try_get() {
                     if name.as_str() == ident.name.as_str() {
                         self.r.injected_crate = Some(module);
@@ -868,7 +854,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
     }
 
     /// Builds the reduced graph for a single item in an external crate.
-    fn build_reduced_graph_for_external_crate_res(
+    crate fn build_reduced_graph_for_external_crate_res(
         &mut self,
         parent: Module<'a>,
         child: Export<ast::NodeId>,
@@ -922,6 +908,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                                              span);
                 self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion));
 
+                module.populate_on_access.set(false);
                 for child in self.r.cstore.item_children_untracked(def_id, self.r.session) {
                     let res = child.res.map_id(|_| panic!("unexpected id"));
                     let ns = if let Res::Def(DefKind::AssocTy, _) = res {
@@ -935,7 +922,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                         self.r.has_self.insert(res.def_id());
                     }
                 }
-                module.populated.set(true);
             }
             Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => {
                 self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion));
@@ -951,19 +937,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         }
     }
 
-    fn legacy_import_macro(&mut self,
-                           name: Name,
-                           binding: &'a NameBinding<'a>,
-                           span: Span,
-                           allow_shadowing: bool) {
-        if self.r.macro_use_prelude.insert(name, binding).is_some() && !allow_shadowing {
-            let msg = format!("`{}` is already in scope", name);
-            let note =
-                "macro-expanded `#[macro_use]`s may not shadow existing macros (see RFC 1560)";
-            self.r.session.struct_span_err(span, &msg).note(note).emit();
-        }
-    }
-
     /// Returns `true` if we should consider the underlying `extern crate` to be used.
     fn process_legacy_macro_imports(&mut self, item: &Item, module: Module<'a>) -> bool {
         let mut import_all = None;
@@ -1021,9 +994,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         if let Some(span) = import_all {
             let directive = macro_use_directive(self, span);
             self.r.potentially_unused_imports.push(directive);
-            module.for_each_child(|ident, ns, binding| if ns == MacroNS {
-                let imported_binding = self.r.import(binding, directive);
-                self.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
+            module.for_each_child(&mut self.r, |this, ident, ns, binding| if ns == MacroNS {
+                let imported_binding = this.import(binding, directive);
+                this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing);
             });
         } else {
             for ident in single_imports.iter().cloned() {
@@ -1039,8 +1012,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                     let directive = macro_use_directive(self, ident.span);
                     self.r.potentially_unused_imports.push(directive);
                     let imported_binding = self.r.import(binding, directive);
-                    self.legacy_import_macro(ident.name, imported_binding,
-                                             ident.span, allow_shadowing);
+                    self.r.legacy_import_macro(ident.name, imported_binding,
+                                               ident.span, allow_shadowing);
                 } else {
                     span_err!(self.r.session, ident.span, E0469, "imported macro not found");
                 }

src/librustc_resolve/diagnostics.rs

@@ -73,10 +73,13 @@ crate fn add_typo_suggestion(
     false
 }
 
-crate fn add_module_candidates(
-    module: Module<'_>, names: &mut Vec<TypoSuggestion>, filter_fn: &impl Fn(Res) -> bool
+crate fn add_module_candidates<'a>(
+    resolver: &mut Resolver<'a>,
+    module: Module<'a>,
+    names: &mut Vec<TypoSuggestion>,
+    filter_fn: &impl Fn(Res) -> bool,
 ) {
-    for (&(ident, _), resolution) in module.resolutions.borrow().iter() {
+    for (&(ident, _), resolution) in resolver.resolutions(module).borrow().iter() {
         if let Some(binding) = resolution.borrow().binding {
             let res = binding.res();
             if filter_fn(res) {
@@ -391,10 +394,10 @@ impl<'a> Resolver<'a> {
                 Scope::CrateRoot => {
                     let root_ident = Ident::new(kw::PathRoot, ident.span);
                     let root_module = this.resolve_crate_root(root_ident);
-                    add_module_candidates(root_module, &mut suggestions, filter_fn);
+                    add_module_candidates(this, root_module, &mut suggestions, filter_fn);
                 }
                 Scope::Module(module) => {
-                    add_module_candidates(module, &mut suggestions, filter_fn);
+                    add_module_candidates(this, module, &mut suggestions, filter_fn);
                 }
                 Scope::MacroUsePrelude => {
                     suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| {
@@ -442,7 +445,7 @@ impl<'a> Resolver<'a> {
                 Scope::StdLibPrelude => {
                     if let Some(prelude) = this.prelude {
                         let mut tmp_suggestions = Vec::new();
-                        add_module_candidates(prelude, &mut tmp_suggestions, filter_fn);
+                        add_module_candidates(this, prelude, &mut tmp_suggestions, filter_fn);
                         suggestions.extend(tmp_suggestions.into_iter().filter(|s| {
                             use_prelude || this.is_builtin_macro(s.res.opt_def_id())
                         }));
@@ -498,11 +501,9 @@ impl<'a> Resolver<'a> {
         while let Some((in_module,
                         path_segments,
                         in_module_is_extern)) = worklist.pop() {
-            self.populate_module_if_necessary(in_module);
-
             // We have to visit module children in deterministic order to avoid
             // instabilities in reported imports (#43552).
-            in_module.for_each_child_stable(|ident, ns, name_binding| {
+            in_module.for_each_child_stable(self, |this, ident, ns, name_binding| {
                 // avoid imports entirely
                 if name_binding.is_import() && !name_binding.is_extern_crate() { return; }
                 // avoid non-importable candidates as well
@@ -536,7 +537,7 @@ impl<'a> Resolver<'a> {
                         // outside crate private modules => no need to check this)
                         if !in_module_is_extern || name_binding.vis == ty::Visibility::Public {
                             let did = match res {
-                                Res::Def(DefKind::Ctor(..), did) => self.parent(did),
+                                Res::Def(DefKind::Ctor(..), did) => this.parent(did),
                                 _ => res.opt_def_id(),
                             };
                             candidates.push(ImportSuggestion { did, path });
@@ -596,8 +597,6 @@ impl<'a> Resolver<'a> {
                         krate: crate_id,
                         index: CRATE_DEF_INDEX,
                     });
-                    self.populate_module_if_necessary(&crate_root);
-
                     suggestions.extend(self.lookup_import_candidates_from_module(
                         lookup_ident, namespace, crate_root, ident, &filter_fn));
                 }
@@ -794,7 +793,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
     ///            at the root of the crate instead of the module where it is defined
     /// ```
     pub(crate) fn check_for_module_export_macro(
-        &self,
+        &mut self,
         directive: &'b ImportDirective<'b>,
         module: ModuleOrUniformRoot<'b>,
         ident: Ident,
@@ -815,7 +814,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
             return None;
         }
 
-        let resolutions = crate_module.resolutions.borrow();
+        let resolutions = self.r.resolutions(crate_module).borrow();
         let resolution = resolutions.get(&(ident, MacroNS))?;
         let binding = resolution.borrow().binding()?;
         if let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() {

src/librustc_resolve/late.rs

@@ -1934,7 +1934,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
         let mut traits = module.traits.borrow_mut();
         if traits.is_none() {
             let mut collected_traits = Vec::new();
-            module.for_each_child(|name, ns, binding| {
+            module.for_each_child(&mut self.r, |_, name, ns, binding| {
                 if ns != TypeNS { return }
                 match binding.res() {
                     Res::Def(DefKind::Trait, _) |

src/librustc_resolve/late/diagnostics.rs

@@ -548,7 +548,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
                 // Items in scope
                 if let RibKind::ModuleRibKind(module) = rib.kind {
                     // Items from this module
-                    add_module_candidates(module, &mut names, &filter_fn);
+                    add_module_candidates(&mut self.r, module, &mut names, &filter_fn);
 
                     if let ModuleKind::Block(..) = module.kind {
                         // We can see through blocks
@@ -577,7 +577,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
                             }));
 
                             if let Some(prelude) = self.r.prelude {
-                                add_module_candidates(prelude, &mut names, &filter_fn);
+                                add_module_candidates(&mut self.r, prelude, &mut names, &filter_fn);
                             }
                         }
                         break;
@@ -599,7 +599,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
                 mod_path, Some(TypeNS), false, span, CrateLint::No
             ) {
                 if let ModuleOrUniformRoot::Module(module) = module {
-                    add_module_candidates(module, &mut names, &filter_fn);
+                    add_module_candidates(&mut self.r, module, &mut names, &filter_fn);
                 }
             }
         }
@@ -717,9 +717,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
             // abort if the module is already found
             if result.is_some() { break; }
 
-            self.r.populate_module_if_necessary(in_module);
-
-            in_module.for_each_child_stable(|ident, _, name_binding| {
+            in_module.for_each_child_stable(&mut self.r, |_, ident, _, name_binding| {
                 // abort if the module is already found or if name_binding is private external
                 if result.is_some() || !name_binding.vis.is_visible_locally() {
                     return
@@ -750,10 +748,8 @@ impl<'a> LateResolutionVisitor<'a, '_> {
 
     fn collect_enum_variants(&mut self, def_id: DefId) -> Option<Vec<Path>> {
         self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| {
-            self.r.populate_module_if_necessary(enum_module);
-
             let mut variants = Vec::new();
-            enum_module.for_each_child_stable(|ident, _, name_binding| {
+            enum_module.for_each_child_stable(&mut self.r, |_, ident, _, name_binding| {
                 if let Res::Def(DefKind::Variant, _) = name_binding.res() {
                     let mut segms = enum_import_suggestion.path.segments.clone();
                     segms.push(ast::PathSegment::from_ident(ident));

src/librustc_resolve/lib.rs

@@ -408,6 +408,8 @@ impl ModuleKind {
     }
 }
 
+type Resolutions<'a> = RefCell<FxHashMap<(Ident, Namespace), &'a RefCell<NameResolution<'a>>>>;
+
 /// One node in the tree of modules.
 pub struct ModuleData<'a> {
     parent: Option<Module<'a>>,
@@ -416,7 +418,11 @@ pub struct ModuleData<'a> {
     // The def id of the closest normal module (`mod`) ancestor (including this module).
     normal_ancestor_id: DefId,
 
-    resolutions: RefCell<FxHashMap<(Ident, Namespace), &'a RefCell<NameResolution<'a>>>>,
+    // Mapping between names and their (possibly in-progress) resolutions in this module.
+    // Resolutions in modules from other crates are not populated until accessed.
+    lazy_resolutions: Resolutions<'a>,
+    // True if this is a module from other crate that needs to be populated on access.
+    populate_on_access: Cell<bool>,
     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>,
@@ -434,11 +440,6 @@ pub struct ModuleData<'a> {
     // Used to memoize the traits in this module for faster searches through all traits in scope.
     traits: RefCell<Option<Box<[(Ident, &'a NameBinding<'a>)]>>>,
 
-    // Whether this module is populated. If not populated, any attempt to
-    // access the children must be preceded with a
-    // `populate_module_if_necessary` call.
-    populated: Cell<bool>,
-
     /// Span of the module itself. Used for error reporting.
     span: Span,
 
@@ -457,7 +458,8 @@ impl<'a> ModuleData<'a> {
             parent,
             kind,
             normal_ancestor_id,
-            resolutions: Default::default(),
+            lazy_resolutions: Default::default(),
+            populate_on_access: Cell::new(!normal_ancestor_id.is_local()),
             single_segment_macro_resolutions: RefCell::new(Vec::new()),
             multi_segment_macro_resolutions: RefCell::new(Vec::new()),
             builtin_attrs: RefCell::new(Vec::new()),
@@ -466,24 +468,27 @@ impl<'a> ModuleData<'a> {
             glob_importers: RefCell::new(Vec::new()),
             globs: RefCell::new(Vec::new()),
             traits: RefCell::new(None),
-            populated: Cell::new(normal_ancestor_id.is_local()),
             span,
             expansion,
         }
     }
 
-    fn for_each_child<F: FnMut(Ident, Namespace, &'a NameBinding<'a>)>(&self, mut f: F) {
-        for (&(ident, ns), name_resolution) in self.resolutions.borrow().iter() {
-            name_resolution.borrow().binding.map(|binding| f(ident, ns, binding));
+    fn for_each_child<F>(&'a self, resolver: &mut Resolver<'a>, mut f: F)
+        where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
+    {
+        for (&(ident, ns), name_resolution) in resolver.resolutions(self).borrow().iter() {
+            name_resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding));
         }
     }
 
-    fn for_each_child_stable<F: FnMut(Ident, Namespace, &'a NameBinding<'a>)>(&self, mut f: F) {
-        let resolutions = self.resolutions.borrow();
+    fn for_each_child_stable<F>(&'a self, resolver: &mut Resolver<'a>, mut f: F)
+        where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>)
+    {
+        let resolutions = resolver.resolutions(self).borrow();
         let mut resolutions = resolutions.iter().collect::<Vec<_>>();
         resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns));
         for &(&(ident, ns), &resolution) in resolutions.iter() {
-            resolution.borrow().binding.map(|binding| f(ident, ns, binding));
+            resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding));
         }
     }
 
@@ -2608,7 +2613,6 @@ impl<'a> Resolver<'a> {
                     return None;
                 };
                 let crate_root = self.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX });
-                self.populate_module_if_necessary(&crate_root);
                 Some((crate_root, ty::Visibility::Public, DUMMY_SP, ExpnId::root())
                     .to_name_binding(self.arenas))
             }

src/librustc_resolve/macros.rs

@@ -869,4 +869,17 @@ impl<'a> Resolver<'a> {
 
         Lrc::new(result)
     }
+
+    crate fn legacy_import_macro(&mut self,
+                           name: ast::Name,
+                           binding: &'a NameBinding<'a>,
+                           span: Span,
+                           allow_shadowing: bool) {
+        if self.macro_use_prelude.insert(name, binding).is_some() && !allow_shadowing {
+            let msg = format!("`{}` is already in scope", name);
+            let note =
+                "macro-expanded `#[macro_use]`s may not shadow existing macros (see RFC 1560)";
+            self.session.struct_span_err(span, &msg).note(note).emit();
+        }
+    }
 }