Auto merge of #33060 - jseyfried:cleanup_resolve, r=nrc

resolve: miscellaneous clean-ups

This PR consists of some small, miscellaneous clean-ups in `resolve`.
r? @eddyb

Author: bors

src/librustc_resolve/build_reduced_graph.rs

@@ -177,13 +177,9 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                         }
 
                         let subclass = ImportDirectiveSubclass::single(binding, source_name);
+                        let span = view_path.span;
+                        parent.add_import_directive(module_path, subclass, span, item.id, vis);
                         self.unresolved_imports += 1;
-                        parent.add_import_directive(module_path,
-                                                    subclass,
-                                                    view_path.span,
-                                                    item.id,
-                                                    vis,
-                                                    is_prelude);
                     }
                     ViewPathList(_, ref source_items) => {
                         // Make sure there's at most one `mod` import in the list.
@@ -228,23 +224,16 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                                 }
                             };
                             let subclass = ImportDirectiveSubclass::single(rename, name);
+                            let (span, id) = (source_item.span, source_item.node.id());
+                            parent.add_import_directive(module_path, subclass, span, id, vis);
                             self.unresolved_imports += 1;
-                            parent.add_import_directive(module_path,
-                                                        subclass,
-                                                        source_item.span,
-                                                        source_item.node.id(),
-                                                        vis,
-                                                        is_prelude);
                         }
                     }
                     ViewPathGlob(_) => {
+                        let subclass = GlobImport { is_prelude: is_prelude };
+                        let span = view_path.span;
+                        parent.add_import_directive(module_path, subclass, span, item.id, vis);
                         self.unresolved_imports += 1;
-                        parent.add_import_directive(module_path,
-                                                    GlobImport,
-                                                    view_path.span,
-                                                    item.id,
-                                                    vis,
-                                                    is_prelude);
                     }
                 }
             }
@@ -271,7 +260,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                 let def = Def::Mod(self.ast_map.local_def_id(item.id));
                 let module = self.new_module(parent_link, Some(def), false, vis);
                 self.define(parent, name, TypeNS, (module, sp));
-                parent.module_children.borrow_mut().insert(item.id, module);
+                self.module_map.insert(item.id, module);
                 *parent_ref = module;
             }
 
@@ -409,7 +398,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
 
             let parent_link = BlockParentLink(parent, block_id);
             let new_module = self.new_module(parent_link, None, false, parent.vis);
-            parent.module_children.borrow_mut().insert(block_id, new_module);
+            self.module_map.insert(block_id, new_module);
             *parent = new_module;
         }
     }

src/librustc_resolve/lib.rs

@@ -827,22 +827,6 @@ pub struct ModuleS<'a> {
     resolutions: RefCell<HashMap<(Name, Namespace), &'a RefCell<NameResolution<'a>>>>,
     unresolved_imports: RefCell<Vec<&'a ImportDirective<'a>>>,
 
-    // The module children of this node, including normal modules and anonymous modules.
-    // Anonymous children are pseudo-modules that are implicitly created around items
-    // contained within blocks.
-    //
-    // For example, if we have this:
-    //
-    //  fn f() {
-    //      fn g() {
-    //          ...
-    //      }
-    //  }
-    //
-    // There will be an anonymous module created around `g` with the ID of the
-    // entry block for `f`.
-    module_children: RefCell<NodeMap<Module<'a>>>,
-
     prelude: RefCell<Option<Module<'a>>>,
 
     glob_importers: RefCell<Vec<(Module<'a>, &'a ImportDirective<'a>)>>,
@@ -874,7 +858,6 @@ impl<'a> ModuleS<'a> {
             extern_crate_id: None,
             resolutions: RefCell::new(HashMap::new()),
             unresolved_imports: RefCell::new(Vec::new()),
-            module_children: RefCell::new(NodeMap()),
             prelude: RefCell::new(None),
             glob_importers: RefCell::new(Vec::new()),
             globs: RefCell::new((Vec::new())),
@@ -1082,6 +1065,22 @@ pub struct Resolver<'a, 'tcx: 'a> {
     export_map: ExportMap,
     trait_map: TraitMap,
 
+    // A map from nodes to modules, both normal (`mod`) modules and anonymous modules.
+    // Anonymous modules are pseudo-modules that are implicitly created around items
+    // contained within blocks.
+    //
+    // For example, if we have this:
+    //
+    //  fn f() {
+    //      fn g() {
+    //          ...
+    //      }
+    //  }
+    //
+    // There will be an anonymous module created around `g` with the ID of the
+    // entry block for `f`.
+    module_map: NodeMap<Module<'a>>,
+
     // Whether or not to print error messages. Can be set to true
     // when getting additional info for error message suggestions,
     // so as to avoid printing duplicate errors
@@ -1107,14 +1106,22 @@ pub struct Resolver<'a, 'tcx: 'a> {
 
 struct ResolverArenas<'a> {
     modules: arena::TypedArena<ModuleS<'a>>,
+    local_modules: RefCell<Vec<Module<'a>>>,
     name_bindings: arena::TypedArena<NameBinding<'a>>,
     import_directives: arena::TypedArena<ImportDirective<'a>>,
     name_resolutions: arena::TypedArena<RefCell<NameResolution<'a>>>,
 }
 
 impl<'a> ResolverArenas<'a> {
     fn alloc_module(&'a self, module: ModuleS<'a>) -> Module<'a> {
-        self.modules.alloc(module)
+        let module = self.modules.alloc(module);
+        if module.def_id().map(|def_id| def_id.is_local()).unwrap_or(true) {
+            self.local_modules.borrow_mut().push(module);
+        }
+        module
+    }
+    fn local_modules(&'a self) -> ::std::cell::Ref<'a, Vec<Module<'a>>> {
+        self.local_modules.borrow()
     }
     fn alloc_name_binding(&'a self, name_binding: NameBinding<'a>) -> &'a NameBinding<'a> {
         self.name_bindings.alloc(name_binding)
@@ -1175,6 +1182,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             freevars_seen: NodeMap(),
             export_map: NodeMap(),
             trait_map: NodeMap(),
+            module_map: NodeMap(),
             used_imports: HashSet::new(),
             used_crates: HashSet::new(),
 
@@ -1193,6 +1201,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     fn arenas() -> ResolverArenas<'a> {
         ResolverArenas {
             modules: arena::TypedArena::new(),
+            local_modules: RefCell::new(Vec::new()),
             name_bindings: arena::TypedArena::new(),
             import_directives: arena::TypedArena::new(),
             name_resolutions: arena::TypedArena::new(),
@@ -1573,7 +1582,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     fn with_scope<F>(&mut self, id: NodeId, f: F)
         where F: FnOnce(&mut Resolver)
     {
-        if let Some(module) = self.current_module.module_children.borrow().get(&id) {
+        let module = self.module_map.get(&id).cloned(); // clones a reference
+        if let Some(module) = module {
             // Move down in the graph.
             let orig_module = ::std::mem::replace(&mut self.current_module, module);
             self.value_ribs.push(Rib::new(ModuleRibKind(module)));
@@ -2124,8 +2134,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         debug!("(resolving block) entering block");
         // Move down in the graph, if there's an anonymous module rooted here.
         let orig_module = self.current_module;
-        let anonymous_module =
-            orig_module.module_children.borrow().get(&block.id).map(|module| *module);
+        let anonymous_module = self.module_map.get(&block.id).cloned(); // clones a reference
 
         if let Some(anonymous_module) = anonymous_module {
             debug!("(resolving block) found anonymous module, moving down");

src/librustc_resolve/resolve_imports.rs

@@ -29,7 +29,6 @@ use syntax::attr::AttrMetaMethods;
 use syntax::codemap::Span;
 use syntax::util::lev_distance::find_best_match_for_name;
 
-use std::mem::replace;
 use std::cell::{Cell, RefCell};
 
 /// Contains data for specific types of import directives.
@@ -41,7 +40,7 @@ pub enum ImportDirectiveSubclass {
         type_determined: Cell<bool>,
         value_determined: Cell<bool>,
     },
-    GlobImport,
+    GlobImport { is_prelude: bool },
 }
 
 impl ImportDirectiveSubclass {
@@ -64,7 +63,6 @@ pub struct ImportDirective<'a> {
     subclass: ImportDirectiveSubclass,
     span: Span,
     vis: ty::Visibility, // see note in ImportResolutionPerNamespace about how to use this
-    is_prelude: bool,
 }
 
 impl<'a> ImportDirective<'a> {
@@ -84,7 +82,7 @@ impl<'a> ImportDirective<'a> {
     }
 
     pub fn is_glob(&self) -> bool {
-        match self.subclass { ImportDirectiveSubclass::GlobImport => true, _ => false }
+        match self.subclass { ImportDirectiveSubclass::GlobImport { .. } => true, _ => false }
     }
 }
 
@@ -191,7 +189,7 @@ impl<'a> NameResolution<'a> {
                 };
                 let name = match directive.subclass {
                     SingleImport { source, .. } => source,
-                    GlobImport => unreachable!(),
+                    GlobImport { .. } => unreachable!(),
                 };
                 match target_module.resolve_name(name, ns, false) {
                     Failed(_) => {}
@@ -282,16 +280,14 @@ impl<'a> ::ModuleS<'a> {
                                 subclass: ImportDirectiveSubclass,
                                 span: Span,
                                 id: NodeId,
-                                vis: ty::Visibility,
-                                is_prelude: bool) {
+                                vis: ty::Visibility) {
         let directive = self.arenas.alloc_import_directive(ImportDirective {
             module_path: module_path,
             target_module: Cell::new(None),
             subclass: subclass,
             span: span,
             id: id,
             vis: vis,
-            is_prelude: is_prelude,
         });
 
         self.unresolved_imports.borrow_mut().push(directive);
@@ -304,8 +300,8 @@ impl<'a> ::ModuleS<'a> {
             }
             // We don't add prelude imports to the globs since they only affect lexical scopes,
             // which are not relevant to import resolution.
-            GlobImport if directive.is_prelude => {}
-            GlobImport => self.globs.borrow_mut().push(directive),
+            GlobImport { is_prelude: true } => {}
+            GlobImport { .. } => self.globs.borrow_mut().push(directive),
         }
     }
 
@@ -374,11 +370,17 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                    i,
                    self.resolver.unresolved_imports);
 
-            self.resolve_imports_for_module_subtree(self.resolver.graph_root, &mut errors);
+            // Attempt to resolve imports in all local modules.
+            for module in self.resolver.arenas.local_modules().iter() {
+                self.resolver.current_module = module;
+                self.resolve_imports_in_current_module(&mut errors);
+            }
 
             if self.resolver.unresolved_imports == 0 {
                 debug!("(resolving imports) success");
-                self.finalize_resolutions(self.resolver.graph_root, false);
+                for module in self.resolver.arenas.local_modules().iter() {
+                    self.finalize_resolutions_in(module, false);
+                }
                 break;
             }
 
@@ -388,7 +390,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                 // to avoid generating multiple errors on the same import.
                 // Imports that are still indeterminate at this point are actually blocked
                 // by errored imports, so there is no point reporting them.
-                self.finalize_resolutions(self.resolver.graph_root, errors.len() == 0);
+                for module in self.resolver.arenas.local_modules().iter() {
+                    self.finalize_resolutions_in(module, errors.len() == 0);
+                }
                 for e in errors {
                     self.import_resolving_error(e)
                 }
@@ -425,22 +429,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                       ResolutionError::UnresolvedImport(Some((&path, &e.help))));
     }
 
-    /// Attempts to resolve imports for the given module and all of its
-    /// submodules.
-    fn resolve_imports_for_module_subtree(&mut self,
-                                          module_: Module<'b>,
-                                          errors: &mut Vec<ImportResolvingError<'b>>) {
-        debug!("(resolving imports for module subtree) resolving {}",
-               module_to_string(&module_));
-        let orig_module = replace(&mut self.resolver.current_module, module_);
-        self.resolve_imports_in_current_module(errors);
-        self.resolver.current_module = orig_module;
-
-        for (_, child_module) in module_.module_children.borrow().iter() {
-            self.resolve_imports_for_module_subtree(child_module, errors);
-        }
-    }
-
     /// Attempts to resolve imports for the given module only.
     fn resolve_imports_in_current_module(&mut self, errors: &mut Vec<ImportResolvingError<'b>>) {
         let mut imports = Vec::new();
@@ -496,7 +484,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         let (source, target, value_determined, type_determined) = match directive.subclass {
             SingleImport { source, target, ref value_determined, ref type_determined } =>
                 (source, target, value_determined, type_determined),
-            GlobImport => return self.resolve_glob_import(target_module, directive),
+            GlobImport { .. } => return self.resolve_glob_import(target_module, directive),
         };
 
         // We need to resolve both namespaces for this to succeed.
@@ -644,7 +632,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         }
         self.resolver.populate_module_if_necessary(target_module);
 
-        if directive.is_prelude {
+        if let GlobImport { is_prelude: true } = directive.subclass {
             *module_.prelude.borrow_mut() = Some(target_module);
             return Success(());
         }
@@ -676,9 +664,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         return Success(());
     }
 
-    // Miscellaneous post-processing, including recording reexports, recording shadowed traits,
-    // reporting conflicts, reporting the PRIVATE_IN_PUBLIC lint, and reporting unresolved imports.
-    fn finalize_resolutions(&mut self, module: Module<'b>, report_unresolved_imports: bool) {
+    // Miscellaneous post-processing, including recording reexports, reporting conflicts,
+    // reporting the PRIVATE_IN_PUBLIC lint, and reporting unresolved imports.
+    fn finalize_resolutions_in(&mut self, module: Module<'b>, report_unresolved_imports: bool) {
         // Since import resolution is finished, globs will not define any more names.
         *module.globs.borrow_mut() = Vec::new();
 
@@ -726,10 +714,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                 break;
             }
         }
-
-        for (_, child) in module.module_children.borrow().iter() {
-            self.finalize_resolutions(child, report_unresolved_imports);
-        }
     }
 }
 
@@ -747,7 +731,7 @@ fn import_path_to_string(names: &[Name], subclass: &ImportDirectiveSubclass) ->
 fn import_directive_subclass_to_string(subclass: &ImportDirectiveSubclass) -> String {
     match *subclass {
         SingleImport { source, .. } => source.to_string(),
-        GlobImport => "*".to_string(),
+        GlobImport { .. } => "*".to_string(),
     }
 }