Addressing nits & tests explanations.

4 files changed, 34 insertions, 17 deletions

diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index b368e16d6fe..412643ba945 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/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>
@@ -218,16 +223,16 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             if self.resolver.unresolved_imports == prev_unresolved_imports {
                 // resolving failed
                 if errors.len() > 0 {
-                    for (span, path, help) in errors {
+                    for e in errors {
                         resolve_error(self.resolver,
-                                      span,
-                                      ResolutionError::UnresolvedImport(Some((&*path, &*help))));
+                                      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 undeterminate at this point are actually blocked
-                    // by errored imports, so there is no point reporting them
+                    // 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;
@@ -241,7 +246,7 @@ 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>)
-                                          -> Vec<(Span, String, String)> {
+                                          -> Vec<ImportResolvingError> {
         let mut errors = Vec::new();
         debug!("(resolving imports for module subtree) resolving {}",
                module_to_string(&*module_));
@@ -269,7 +274,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     }
 
     /// Attempts to resolve imports for the given module only.
-    fn resolve_imports_for_module(&mut self, module: Rc<Module>) -> Vec<(Span, String, String)> {
+    fn resolve_imports_for_module(&mut self, module: Rc<Module>) -> Vec<ImportResolvingError> {
         let mut errors = Vec::new();
 
         if module.all_imports_resolved() {
@@ -292,12 +297,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                         Some((span, msg)) => (span, format!(". {}", msg)),
                         None => (import_directive.span, String::new())
                     };
-                    errors.push((span,
-                                 import_path_to_string(
-                                    &import_directive.module_path,
-                                    import_directive.subclass
-                                 ),
-                                 help))
+                    errors.push(ImportResolvingError {
+                                    span: span,
+                                    path: import_path_to_string(
+                                            &import_directive.module_path,
+                                            import_directive.subclass
+                                         ),
+                                    help: help
+                                });
                 }
                 ResolveResult::Indeterminate => {}
                 ResolveResult::Success(()) => {
diff --git a/src/test/run-pass/import-glob-1.rs b/src/test/run-pass/import-glob-1.rs
index 9dc570fbe51..a7949e7d6e3 100644
--- a/src/test/run-pass/import-glob-1.rs
+++ b/src/test/run-pass/import-glob-1.rs
@@ -1,4 +1,4 @@
-// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
+// 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.
 //
@@ -8,7 +8,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![allow(unused_imports, dead_code)]
+// 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::*;
diff --git a/src/test/run-pass/issue-18083.rs b/src/test/run-pass/issue-18083.rs
index 8c3dc67c313..ff26e186db3 100644
--- a/src/test/run-pass/issue-18083.rs
+++ b/src/test/run-pass/issue-18083.rs
@@ -8,6 +8,9 @@
 // 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;
diff --git a/src/test/run-pass/issue-4865-1.rs b/src/test/run-pass/issue-4865-1.rs
index a025273073d..3c477795130 100644
--- a/src/test/run-pass/issue-4865-1.rs
+++ b/src/test/run-pass/issue-4865-1.rs
@@ -8,6 +8,11 @@
 // 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::*;