Replace the field imports in Module with unresolved_imports and refactor away resolved_import_count

jseyfried · jseyfried · commit 5ad84f130169 · 2016-02-18T00:02:50.000Z
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
@@ -19,7 +19,7 @@ use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport};
 use Module;
 use Namespace::{self, TypeNS, ValueNS};
 use {NameBinding, NameBindingKind};
-use {names_to_string, module_to_string};
+use module_to_string;
 use ParentLink::{ModuleParentLink, BlockParentLink};
 use Resolver;
 use resolve_imports::Shadowable;
@@ -682,7 +682,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                               id: NodeId,
                               is_public: bool,
                               shadowable: Shadowable) {
-        module_.imports
+        module_.unresolved_imports
                .borrow_mut()
                .push(ImportDirective::new(module_path, subclass, span, id, is_public, shadowable));
         self.unresolved_imports += 1;
@@ -696,9 +696,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
         match subclass {
             SingleImport(target, _) => {
-                debug!("(building import directive) building import directive: {}::{}",
-                       names_to_string(&module_.imports.borrow().last().unwrap().module_path),
-                       target);
                 module_.increment_outstanding_references_for(target, ValueNS);
                 module_.increment_outstanding_references_for(target, TypeNS);
             }
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
@@ -18,7 +18,6 @@
 #![cfg_attr(not(stage0), deny(warnings))]
 
 #![feature(associated_consts)]
-#![feature(borrow_state)]
 #![feature(rustc_diagnostic_macros)]
 #![feature(rustc_private)]
 #![feature(staged_api)]
@@ -799,7 +798,7 @@ pub struct ModuleS<'a> {
     is_extern_crate: bool,
 
     resolutions: RefCell<HashMap<(Name, Namespace), NameResolution<'a>>>,
-    imports: RefCell<Vec<ImportDirective>>,
+    unresolved_imports: RefCell<Vec<ImportDirective>>,
 
     // The module children of this node, including normal modules and anonymous modules.
     // Anonymous children are pseudo-modules that are implicitly created around items
@@ -828,9 +827,6 @@ pub struct ModuleS<'a> {
     // The number of unresolved pub glob imports in this module
     pub_glob_count: Cell<usize>,
 
-    // The index of the import we're resolving.
-    resolved_import_count: Cell<usize>,
-
     // Whether this module is populated. If not populated, any attempt to
     // access the children must be preceded with a
     // `populate_module_if_necessary` call.
@@ -847,13 +843,12 @@ impl<'a> ModuleS<'a> {
             is_public: is_public,
             is_extern_crate: false,
             resolutions: RefCell::new(HashMap::new()),
-            imports: RefCell::new(Vec::new()),
+            unresolved_imports: RefCell::new(Vec::new()),
             module_children: RefCell::new(NodeMap()),
             shadowed_traits: RefCell::new(Vec::new()),
             glob_count: Cell::new(0),
             pub_count: Cell::new(0),
             pub_glob_count: Cell::new(0),
-            resolved_import_count: Cell::new(0),
             populated: Cell::new(!external),
         }
     }
@@ -924,15 +919,6 @@ impl<'a> ModuleS<'a> {
         }
     }
 
-    fn all_imports_resolved(&self) -> bool {
-        if self.imports.borrow_state() == ::std::cell::BorrowState::Writing {
-            // it is currently being resolved ! so nope
-            false
-        } else {
-            self.imports.borrow().len() == self.resolved_import_count.get()
-        }
-    }
-
     pub fn inc_glob_count(&self) {
         self.glob_count.set(self.glob_count.get() + 1);
     }
@@ -1622,13 +1608,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     }
 
     fn report_unresolved_imports(&mut self, module_: Module<'a>) {
-        let index = module_.resolved_import_count.get();
-        let imports = module_.imports.borrow();
-        let import_count = imports.len();
-        if index != import_count {
-            resolve_error(self,
-                          (*imports)[index].span,
-                          ResolutionError::UnresolvedImport(None));
+        for import in module_.unresolved_imports.borrow().iter() {
+            resolve_error(self, import.span, ResolutionError::UnresolvedImport(None));
+            break;
         }
 
         // Descend into children and anonymous children.
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
@@ -252,47 +252,28 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     fn resolve_imports_for_module(&mut self,
                                   module: Module<'b>,
                                   errors: &mut Vec<ImportResolvingError<'b>>) {
-        if module.all_imports_resolved() {
-            debug!("(resolving imports for module) all imports resolved for {}",
-                   module_to_string(&module));
-            return;
-        }
+        let mut imports = Vec::new();
+        let mut unresolved_imports = module.unresolved_imports.borrow_mut();
+        ::std::mem::swap(&mut imports, &mut unresolved_imports);
 
-        let mut imports = module.imports.borrow_mut();
-        let import_count = imports.len();
-        let mut indeterminate_imports = Vec::new();
-        while module.resolved_import_count.get() + indeterminate_imports.len() < import_count {
-            let import_index = module.resolved_import_count.get();
-            match self.resolve_import_for_module(module, &imports[import_index]) {
-                ResolveResult::Failed(err) => {
-                    let import_directive = &imports[import_index];
+        for import_directive in imports {
+            match self.resolve_import_for_module(module, &import_directive) {
+                Failed(err) => {
                     let (span, help) = match err {
                         Some((span, msg)) => (span, format!(". {}", msg)),
                         None => (import_directive.span, String::new()),
                     };
                     errors.push(ImportResolvingError {
                         source_module: module,
-                        import_directive: import_directive.clone(),
+                        import_directive: import_directive,
                         span: span,
                         help: help,
                     });
-                    module.resolved_import_count.set(module.resolved_import_count.get() + 1);
-                    continue;
-                }
-                ResolveResult::Indeterminate => {}
-                ResolveResult::Success(()) => {
-                    // count success
-                    module.resolved_import_count
-                          .set(module.resolved_import_count.get() + 1);
-                    continue;
                 }
+                Indeterminate => unresolved_imports.push(import_directive),
+                Success(()) => {}
             }
-            // This resolution was not successful, keep it for later
-            indeterminate_imports.push(imports.swap_remove(import_index));
-
         }
-
-        imports.extend(indeterminate_imports);
     }
 
     /// Attempts to resolve the given import. The return value indicates
