Finish encapsulating the details of import resolution in resolve_imports

Author: jseyfried

src/librustc_resolve/build_reduced_graph.rs

@@ -676,10 +676,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                               id: NodeId,
                               is_public: bool,
                               shadowable: Shadowable) {
-        if is_public {
-            module_.inc_pub_count();
-        }
-
         // Bump the reference count on the name. Or, if this is a glob, set
         // the appropriate flag.
 
@@ -691,11 +687,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
             GlobImport => {
                 // Set the glob flag. This tells us that we don't know the
                 // module's exports ahead of time.
-
-                module_.inc_glob_count();
-                if is_public {
-                    module_.inc_pub_glob_count();
-                }
+                module_.inc_glob_count(is_public)
             }
         }
 

src/librustc_resolve/lib.rs

@@ -836,14 +836,11 @@ pub struct ModuleS<'a> {
     glob_importers: RefCell<Vec<(Module<'a>, &'a ImportDirective)>>,
     resolved_globs: RefCell<(Vec<Module<'a>> /* public */, Vec<Module<'a>> /* private */)>,
 
-    // The number of unresolved globs that this module exports.
-    glob_count: Cell<usize>,
+    // The number of public glob imports in this module.
+    public_glob_count: Cell<usize>,
 
-    // The number of unresolved pub imports (both regular and globs) in this module
-    pub_count: Cell<usize>,
-
-    // The number of unresolved pub glob imports in this module
-    pub_glob_count: Cell<usize>,
+    // The number of private glob imports in this module.
+    private_glob_count: Cell<usize>,
 
     // Whether this module is populated. If not populated, any attempt to
     // access the children must be preceded with a
@@ -872,9 +869,8 @@ impl<'a> ModuleS<'a> {
             shadowed_traits: RefCell::new(Vec::new()),
             glob_importers: RefCell::new(Vec::new()),
             resolved_globs: RefCell::new((Vec::new(), Vec::new())),
-            glob_count: Cell::new(0),
-            pub_count: Cell::new(0),
-            pub_glob_count: Cell::new(0),
+            public_glob_count: Cell::new(0),
+            private_glob_count: Cell::new(0),
             populated: Cell::new(!external),
             arenas: arenas
         }
@@ -918,26 +914,9 @@ impl<'a> ModuleS<'a> {
         }
     }
 
-    pub fn inc_glob_count(&self) {
-        self.glob_count.set(self.glob_count.get() + 1);
-    }
-    pub fn dec_glob_count(&self) {
-        assert!(self.glob_count.get() > 0);
-        self.glob_count.set(self.glob_count.get() - 1);
-    }
-    pub fn inc_pub_count(&self) {
-        self.pub_count.set(self.pub_count.get() + 1);
-    }
-    pub fn dec_pub_count(&self) {
-        assert!(self.pub_count.get() > 0);
-        self.pub_count.set(self.pub_count.get() - 1);
-    }
-    pub fn inc_pub_glob_count(&self) {
-        self.pub_glob_count.set(self.pub_glob_count.get() + 1);
-    }
-    pub fn dec_pub_glob_count(&self) {
-        assert!(self.pub_glob_count.get() > 0);
-        self.pub_glob_count.set(self.pub_glob_count.get() - 1);
+    fn inc_glob_count(&self, is_public: bool) {
+        let glob_count = if is_public { &self.public_glob_count } else { &self.private_glob_count };
+        glob_count.set(glob_count.get() + 1);
     }
 }
 
@@ -1612,18 +1591,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         })
     }
 
-    fn report_unresolved_imports(&mut self, module_: Module<'a>) {
-        for import in module_.unresolved_imports.borrow().iter() {
-            resolve_error(self, import.span, ResolutionError::UnresolvedImport(None));
-            break;
-        }
-
-        // Descend into children and anonymous children.
-        for (_, module_) in module_.module_children.borrow().iter() {
-            self.report_unresolved_imports(module_);
-        }
-    }
-
     // AST resolution
     //
     // We maintain a list of value ribs and type ribs.

src/librustc_resolve/resolve_imports.rs

@@ -211,7 +211,7 @@ impl<'a> ::ModuleS<'a> {
         let (ref mut public_globs, ref mut private_globs) = *self.resolved_globs.borrow_mut();
 
         // Check if the public globs are determined
-        if self.pub_glob_count.get() > 0 {
+        if public_globs.len() < self.public_glob_count.get() {
             return Indeterminate;
         }
         for module in public_globs.iter() {
@@ -225,7 +225,7 @@ impl<'a> ::ModuleS<'a> {
         }
 
         // Check if the private globs are determined
-        if self.glob_count.get() > 0 {
+        if private_globs.len() < self.private_glob_count.get() {
             return Indeterminate;
         }
         for module in private_globs.iter() {
@@ -322,23 +322,19 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
             if self.resolver.unresolved_imports == 0 {
                 debug!("(resolving imports) success");
-                self.finalize_resolutions(self.resolver.graph_root);
+                self.finalize_resolutions(self.resolver.graph_root, false);
                 break;
             }
 
             if self.resolver.unresolved_imports == prev_unresolved_imports {
                 // resolving failed
-                self.finalize_resolutions(self.resolver.graph_root);
-                if errors.len() > 0 {
-                    for e in errors {
-                        self.import_resolving_error(e)
-                    }
-                } else {
-                    // Report unresolved imports only if no hard error was already reported
-                    // 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.resolver.report_unresolved_imports(self.resolver.graph_root);
+                // Report unresolved imports only if no hard error was already reported
+                // 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 e in errors {
+                    self.import_resolving_error(e)
                 }
                 break;
             }
@@ -444,16 +440,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                 // Decrement the count of unresolved imports.
                 assert!(self.resolver.unresolved_imports >= 1);
                 self.resolver.unresolved_imports -= 1;
-
-                if let GlobImport = import_directive.subclass {
-                    module_.dec_glob_count();
-                    if import_directive.is_public {
-                        module_.dec_pub_glob_count();
-                    }
-                }
-                if import_directive.is_public {
-                    module_.dec_pub_count();
-                }
                 Success(())
             })
     }
@@ -697,10 +683,10 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     }
 
     // Miscellaneous post-processing, including recording reexports, recording shadowed traits,
-    // reporting conflicts, and reporting the PRIVATE_IN_PUBLIC lint.
-    fn finalize_resolutions(&mut self, module: Module<'b>) {
+    // reporting conflicts, reporting the PRIVATE_IN_PUBLIC lint, and reporting unresolved imports.
+    fn finalize_resolutions(&mut self, module: Module<'b>, report_unresolved_imports: bool) {
         // Since import resolution is finished, globs will not define any more names.
-        module.pub_glob_count.set(0); module.glob_count.set(0);
+        module.public_glob_count.set(0); module.private_glob_count.set(0);
         *module.resolved_globs.borrow_mut() = (Vec::new(), Vec::new());
 
         let mut reexports = Vec::new();
@@ -743,8 +729,15 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             }
         }
 
+        if report_unresolved_imports {
+            for import in module.unresolved_imports.borrow().iter() {
+                resolve_error(self.resolver, import.span, ResolutionError::UnresolvedImport(None));
+                break;
+            }
+        }
+
         for (_, child) in module.module_children.borrow().iter() {
-            self.finalize_resolutions(child);
+            self.finalize_resolutions(child, report_unresolved_imports);
         }
     }
 }