diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index fa9c7a2038c80..45dbf08071d19 100644 --- a/src/librustc_resolve/lib.rs +++ b/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 { } impl ResolveResult { - 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. diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 0a88a3c0aefbb..412643ba94546 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> @@ -208,7 +213,7 @@ 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"); @@ -216,7 +221,20 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { } 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) { + fn resolve_imports_for_module_subtree(&mut self, module_: Rc) + -> Vec { + 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) { + fn resolve_imports_for_module(&mut self, module: Rc) -> Vec { + 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); diff --git a/src/test/compile-fail/import-shadow-6.rs b/src/test/compile-fail/import-shadow-6.rs index fa3b75c70f0b6..0f3d54d5fe3d8 100644 --- a/src/test/compile-fail/import-shadow-6.rs +++ b/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; diff --git a/src/test/compile-fail/issue-25396.rs b/src/test/compile-fail/issue-25396.rs index 3ada57c999305..bf26a591b4b5d 100644 --- a/src/test/compile-fail/issue-25396.rs +++ b/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() {} diff --git a/src/test/run-pass/import-glob-1.rs b/src/test/run-pass/import-glob-1.rs new file mode 100644 index 0000000000000..a7949e7d6e3b1 --- /dev/null +++ b/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 or the MIT license +// , 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() {} diff --git a/src/test/run-pass/issue-18083.rs b/src/test/run-pass/issue-18083.rs new file mode 100644 index 0000000000000..ff26e186db393 --- /dev/null +++ b/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 or the MIT license +// , 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() {} diff --git a/src/test/run-pass/issue-4865-1.rs b/src/test/run-pass/issue-4865-1.rs new file mode 100644 index 0000000000000..3c4777951302e --- /dev/null +++ b/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 or the MIT license +// , 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() { +}