From 4f5616e3c43f866f4758a21f67d98da52b89ee20 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 22 Aug 2016 07:12:13 +0000 Subject: [PATCH] Avoid cascading name resolution errors caused by an ambiguous module. --- src/librustc_resolve/lib.rs | 68 +++++++++++++--------- src/librustc_resolve/resolve_imports.rs | 2 +- src/test/compile-fail/imports/duplicate.rs | 5 ++ 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index a881feaa4d35..0fe7f9ed2154 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -749,10 +749,6 @@ impl<'a> LexicalScopeBinding<'a> { _ => None, } } - - fn module(self) -> Option> { - self.item().and_then(NameBinding::module) - } } /// The link from a module up to its nearest parent node. @@ -884,12 +880,13 @@ enum NameBindingKind<'a> { struct PrivacyError<'a>(Span, Name, &'a NameBinding<'a>); impl<'a> NameBinding<'a> { - fn module(&self) -> Option> { + fn module(&self) -> Result, bool /* true if an error has already been reported */> { match self.kind { - NameBindingKind::Module(module) => Some(module), - NameBindingKind::Def(_) => None, + NameBindingKind::Module(module) => Ok(module), NameBindingKind::Import { binding, .. } => binding.module(), - NameBindingKind::Ambiguity { .. } => None, + NameBindingKind::Def(Def::Err) => Err(true), + NameBindingKind::Def(_) => Err(false), + NameBindingKind::Ambiguity { .. } => Err(false), } } @@ -915,7 +912,7 @@ impl<'a> NameBinding<'a> { } fn is_extern_crate(&self) -> bool { - self.module().and_then(|module| module.extern_crate_id).is_some() + self.module().ok().and_then(|module| module.extern_crate_id).is_some() } fn is_import(&self) -> bool { @@ -1269,7 +1266,7 @@ impl<'a> Resolver<'a> { fn record_use(&mut self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>, span: Span) -> bool /* true if an error was reported */ { // track extern crates for unused_extern_crate lint - if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) { + if let Some(DefId { krate, .. }) = binding.module().ok().and_then(ModuleS::def_id) { self.used_crates.insert(krate); } @@ -1292,6 +1289,18 @@ impl<'a> Resolver<'a> { } } + fn expect_module(&mut self, name: Name, binding: &'a NameBinding<'a>, span: Option) + -> ResolveResult> { + match binding.module() { + Ok(module) => Success(module), + Err(true) => Failed(None), + Err(false) => { + let msg = format!("Not a module `{}`", name); + Failed(span.map(|span| (span, msg))) + } + } + } + /// Resolves the given module path from the given root `search_module`. fn resolve_module_path_from_root(&mut self, mut search_module: Module<'a>, @@ -1357,11 +1366,9 @@ impl<'a> Resolver<'a> { Success(binding) => { // Check to see whether there are type bindings, and, if // so, whether there is a module within. - if let Some(module_def) = binding.module() { - search_module = module_def; - } else { - let msg = format!("Not a module `{}`", name); - return Failed(span.map(|span| (span, msg))); + match self.expect_module(name, binding, span) { + Success(module) => search_module = module, + result @ _ => return result, } } } @@ -1414,17 +1421,20 @@ impl<'a> Resolver<'a> { // first component of the path in the current lexical // scope and then proceed to resolve below that. let ident = ast::Ident::with_empty_ctxt(module_path[0]); - match self.resolve_ident_in_lexical_scope(ident, TypeNS, span) - .and_then(LexicalScopeBinding::module) { - None => { - let msg = - format!("Use of undeclared type or module `{}`", ident.name); - return Failed(span.map(|span| (span, msg))); - } - Some(containing_module) => { - search_module = containing_module; - start_index = 1; + let lexical_binding = + self.resolve_ident_in_lexical_scope(ident, TypeNS, span); + if let Some(binding) = lexical_binding.and_then(LexicalScopeBinding::item) { + match self.expect_module(ident.name, binding, span) { + Success(containing_module) => { + search_module = containing_module; + start_index = 1; + } + result @ _ => return result, } + } else { + let msg = + format!("Use of undeclared type or module `{}`", ident.name); + return Failed(span.map(|span| (span, msg))); } } } @@ -3202,7 +3212,7 @@ impl<'a> Resolver<'a> { } // collect submodules to explore - if let Some(module) = name_binding.module() { + if let Ok(module) = name_binding.module() { // form the path let path_segments = match module.parent_link { NoParentLink => path_segments.clone(), @@ -3341,9 +3351,9 @@ impl<'a> Resolver<'a> { let msg = { let kind = match (ns, old_binding.module()) { (ValueNS, _) => "a value", - (TypeNS, Some(module)) if module.extern_crate_id.is_some() => "an extern crate", - (TypeNS, Some(module)) if module.is_normal() => "a module", - (TypeNS, Some(module)) if module.is_trait() => "a trait", + (TypeNS, Ok(module)) if module.extern_crate_id.is_some() => "an extern crate", + (TypeNS, Ok(module)) if module.is_normal() => "a module", + (TypeNS, Ok(module)) if module.is_trait() => "a trait", (TypeNS, _) => "a type", }; format!("{} named `{}` has already been {} in this {}", diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index cb89231fc055..c8982d95d4e0 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -460,7 +460,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { errors = true; let (span, help) = match err { Some((span, msg)) => (span, msg), - None => (import.span, String::new()), + None => continue, }; // If the error is a single failed import then create a "fake" import diff --git a/src/test/compile-fail/imports/duplicate.rs b/src/test/compile-fail/imports/duplicate.rs index 70936b254464..fb61bb8e489b 100644 --- a/src/test/compile-fail/imports/duplicate.rs +++ b/src/test/compile-fail/imports/duplicate.rs @@ -56,7 +56,12 @@ mod ambiguous_module_errors { pub mod m2 { pub use super::m2 as foo; } use self::m1::*; //~ NOTE + //~| NOTE use self::m2::*; //~ NOTE + //~| NOTE + + use self::foo::bar; //~ ERROR `foo` is ambiguous + //~| NOTE fn f() { foo::bar(); //~ ERROR `foo` is ambiguous