More perseverant resolve (second try)

src/librustc_resolve/lib.rs

@@ -20,6 +20,7 @@
       html_root_url = "http://doc.rust-lang.org/nightly/")]
 
 #![feature(associated_consts)]
+#![feature(borrow_state)]
 #![feature(rc_weak)]
 #![feature(rustc_diagnostic_macros)]
 #![feature(rustc_private)]
@@ -176,7 +177,7 @@ pub enum ResolutionError<'a> {
     /// error E0431: `self` import can only appear in an import list with a non-empty prefix
     SelfImportOnlyInImportListWithNonEmptyPrefix,
     /// error E0432: unresolved import
-    UnresolvedImport(Option<(&'a str, Option<&'a str>)>),
+    UnresolvedImport(Option<(&'a str, &'a str)>),
     /// error E0433: failed to resolve
     FailedToResolve(&'a str),
     /// error E0434: can't capture dynamic environment in a fn item
@@ -359,8 +360,7 @@ fn resolve_error<'b, 'a:'b, 'tcx:'a>(resolver: &'b Resolver<'a, 'tcx>, span: syn
         }
         ResolutionError::UnresolvedImport(name) => {
             let msg = match name {
-                Some((n, Some(p))) => format!("unresolved import `{}`{}", n, p),
-                Some((n, None)) => format!("unresolved import (maybe you meant `{}::*`?)", n),
+                Some((n, p)) => format!("unresolved import `{}`{}", n, p),
                 None => "unresolved import".to_owned()
             };
             span_err!(resolver.session, span, E0432, "{}", msg);
@@ -539,8 +539,8 @@ enum ResolveResult<T> {
 }
 
 impl<T> ResolveResult<T> {
-    fn indeterminate(&self) -> bool {
-        match *self { Indeterminate => true, _ => false }
+    fn success(&self) -> bool {
+        match *self { Success(_) => true, _ => false }
     }
 }
 
@@ -732,7 +732,12 @@ impl Module {
     }
 
     fn all_imports_resolved(&self) -> bool {
-        self.imports.borrow().len() == self.resolved_import_count.get()
+        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()
+        }
     }
 }
 
@@ -1815,19 +1820,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         let imports = module_.imports.borrow();
         let import_count = imports.len();
         if index != import_count {
-            let sn = self.session
-                         .codemap()
-                         .span_to_snippet((*imports)[index].span)
-                         .unwrap();
-            if sn.contains("::") {
-                resolve_error(self,
-                              (*imports)[index].span,
-                              ResolutionError::UnresolvedImport(None));
-            } else {
-                resolve_error(self,
-                              (*imports)[index].span,
-                              ResolutionError::UnresolvedImport(Some((&*sn, None))));
-            }
+            resolve_error(self,
+                          (*imports)[index].span,
+                          ResolutionError::UnresolvedImport(None));
         }
 
         // Descend into children and anonymous children.

src/librustc_resolve/resolve_imports.rs

@@ -184,6 +184,11 @@ impl ImportResolution {
     }
 }
 
+struct ImportResolvingError {
+    span: Span,
+    path: String,
+    help: String,
+}
 
 struct ImportResolver<'a, 'b:'a, 'tcx:'b> {
     resolver: &'a mut Resolver<'b, 'tcx>
@@ -208,15 +213,28 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                    i, self.resolver.unresolved_imports);
 
             let module_root = self.resolver.graph_root.get_module();
-            self.resolve_imports_for_module_subtree(module_root.clone());
+            let errors = self.resolve_imports_for_module_subtree(module_root.clone());
 
             if self.resolver.unresolved_imports == 0 {
                 debug!("(resolving imports) success");
                 break;
             }
 
             if self.resolver.unresolved_imports == prev_unresolved_imports {
-                self.resolver.report_unresolved_imports(module_root);
+                // resolving failed
+                if errors.len() > 0 {
+                    for e in errors {
+                        resolve_error(self.resolver,
+                                      e.span,
+                                      ResolutionError::UnresolvedImport(Some((&e.path, &e.help))));
+                    }
+                } 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(module_root);
+                }
                 break;
             }
 
@@ -227,11 +245,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
     /// Attempts to resolve imports for the given module and all of its
     /// submodules.
-    fn resolve_imports_for_module_subtree(&mut self, module_: Rc<Module>) {
+    fn resolve_imports_for_module_subtree(&mut self, module_: Rc<Module>)
+                                          -> Vec<ImportResolvingError> {
+        let mut errors = Vec::new();
         debug!("(resolving imports for module subtree) resolving {}",
                module_to_string(&*module_));
         let orig_module = replace(&mut self.resolver.current_module, module_.clone());
-        self.resolve_imports_for_module(module_.clone());
+        errors.extend(self.resolve_imports_for_module(module_.clone()));
         self.resolver.current_module = orig_module;
 
         build_reduced_graph::populate_module_if_necessary(self.resolver, &module_);
@@ -241,53 +261,67 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     // Nothing to do.
                 }
                 Some(child_module) => {
-                    self.resolve_imports_for_module_subtree(child_module);
+                    errors.extend(self.resolve_imports_for_module_subtree(child_module));
                 }
             }
         }
 
         for (_, child_module) in module_.anonymous_children.borrow().iter() {
-            self.resolve_imports_for_module_subtree(child_module.clone());
+            errors.extend(self.resolve_imports_for_module_subtree(child_module.clone()));
         }
+
+        errors
     }
 
     /// Attempts to resolve imports for the given module only.
-    fn resolve_imports_for_module(&mut self, module: Rc<Module>) {
+    fn resolve_imports_for_module(&mut self, module: Rc<Module>) -> Vec<ImportResolvingError> {
+        let mut errors = Vec::new();
+
         if module.all_imports_resolved() {
             debug!("(resolving imports for module) all imports resolved for \
                    {}",
                    module_to_string(&*module));
-            return;
+            return errors;
         }
 
-        let imports = module.imports.borrow();
+        let mut imports = module.imports.borrow_mut();
         let import_count = imports.len();
-        while module.resolved_import_count.get() < import_count {
+        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();
-            let import_directive = &(*imports)[import_index];
             match self.resolve_import_for_module(module.clone(),
-                                                 import_directive) {
+                                                 &imports[import_index]) {
                 ResolveResult::Failed(err) => {
+                    let import_directive = &imports[import_index];
                     let (span, help) = match err {
                         Some((span, msg)) => (span, format!(". {}", msg)),
                         None => (import_directive.span, String::new())
                     };
-                    resolve_error(self.resolver,
-                                    span,
-                                    ResolutionError::UnresolvedImport(
-                                                Some((&*import_path_to_string(
-                                                        &import_directive.module_path,
-                                                        import_directive.subclass),
-                                                      Some(&*help))))
-                                   );
+                    errors.push(ImportResolvingError {
+                                    span: span,
+                                    path: import_path_to_string(
+                                            &import_directive.module_path,
+                                            import_directive.subclass
+                                         ),
+                                    help: help
+                                });
+                }
+                ResolveResult::Indeterminate => {}
+                ResolveResult::Success(()) => {
+                    // count success
+                    module.resolved_import_count
+                          .set(module.resolved_import_count.get() + 1);
+                    continue;
                 }
-                ResolveResult::Indeterminate => break, // Bail out. We'll come around next time.
-                ResolveResult::Success(()) => () // Good. Continue.
             }
+            // This resolution was not successful, keep it for later
+            indeterminate_imports.push(imports.swap_remove(import_index));
 
-            module.resolved_import_count
-                  .set(module.resolved_import_count.get() + 1);
         }
+
+        imports.extend(indeterminate_imports);
+
+        errors
     }
 
     /// Attempts to resolve the given import. The return value indicates
@@ -367,11 +401,10 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         }
 
         // Decrement the count of unresolved globs if necessary. But only if
-        // the resolution result is indeterminate -- otherwise we'll stop
-        // processing imports here. (See the loop in
-        // resolve_imports_for_module).
+        // the resolution result is a success -- other cases will
+        // be handled by the main loop.
 
-        if !resolution_result.indeterminate() {
+        if resolution_result.success() {
             match import_directive.subclass {
                 GlobImport => {
                     assert!(module_.glob_count.get() >= 1);

src/test/compile-fail/import-shadow-6.rs

@@ -12,8 +12,8 @@
 
 #![no_implicit_prelude]
 
-use qux::*;
-use foo::*; //~ERROR a type named `Baz` has already been imported in this module
+use qux::*; //~ERROR a type named `Baz` has already been imported in this module
+use foo::*;
 
 mod foo {
     pub type Baz = isize;

src/test/compile-fail/issue-25396.rs

@@ -11,14 +11,14 @@
 use foo::baz;
 use bar::baz; //~ ERROR a module named `baz` has already been imported
 
-use foo::Quux;
 use bar::Quux; //~ ERROR a trait named `Quux` has already been imported
+use foo::Quux;
 
-use foo::blah;
-use bar::blah; //~ ERROR a type named `blah` has already been imported
+use foo::blah; //~ ERROR a type named `blah` has already been imported
+use bar::blah;
 
-use foo::WOMP;
-use bar::WOMP; //~ ERROR a value named `WOMP` has already been imported
+use foo::WOMP; //~ ERROR a value named `WOMP` has already been imported
+use bar::WOMP;
 
 fn main() {}
 

src/test/run-pass/import-glob-1.rs

@@ -0,0 +1,34 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// This should resolve fine. Prior to fix, the last import
+// was being tried too early, and marked as unrsolved before
+// the glob import had a chance to be resolved.
+
+mod bar {
+    pub use self::middle::*;
+
+    mod middle {
+        pub use self::baz::Baz;
+
+        mod baz {
+            pub enum Baz {
+                Baz1,
+                Baz2
+            }
+        }
+    }
+}
+
+mod foo {
+    use bar::Baz::{Baz1, Baz2};
+}
+
+fn main() {}

src/test/run-pass/issue-18083.rs

@@ -0,0 +1,32 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// These crossed imports should resolve fine, and not block on
+// each other and be reported as unresolved.
+
+mod a {
+    use b::{B};
+    pub use self::inner::A;
+
+    mod inner {
+        pub struct A;
+    }
+}
+
+mod b {
+    use a::{A};
+    pub use self::inner::B;
+
+    mod inner {
+        pub struct B;
+    }
+}
+
+fn main() {}

src/test/run-pass/issue-4865-1.rs

@@ -0,0 +1,41 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// This should resolve fine.
+// Prior to fix, the crossed imports between a and b
+// would block on the glob import, itself never being resolved
+// because these previous imports were not resolved.
+
+pub mod a {
+    use b::fn_b;
+    use c::*;
+
+    pub fn fn_a(){
+    }
+}
+
+pub mod b {
+    use a::fn_a;
+    use c::*;
+
+    pub fn fn_b(){
+    }
+}
+
+pub mod c{
+    pub fn fn_c(){
+    }
+}
+
+use a::fn_a;
+use b::fn_b;
+
+fn main() {
+}