Make import resolution and error resolution reporting deterministic.

These tasks used to depend on the iteration order of `module_children`.

Author: jseyfried

src/librustc_resolve/lib.rs

@@ -1103,14 +1103,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)
@@ -1189,6 +1197,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(),

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.
@@ -371,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;
             }
 
@@ -385,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)
                 }
@@ -422,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();
@@ -675,7 +666,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
     // 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) {
+    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();
 
@@ -723,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);
-        }
     }
 }