diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index a1d866fc48b..8f4913f0420 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -107,20 +107,37 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined; /// otherwise, reports an error. fn define>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T) { - let name_binding = def.to_name_binding(); - let span = name_binding.span.unwrap_or(DUMMY_SP); - self.check_for_conflicts_between_external_crates_and_items(&parent, name, span); - if !parent.try_define_child(name, ns, name_binding) { + let binding = def.to_name_binding(); + let old_binding = match parent.try_define_child(name, ns, binding.clone()) { + Some(old_binding) => old_binding, + None => return, + }; + + let span = binding.span.unwrap_or(DUMMY_SP); + if !old_binding.is_extern_crate() && !binding.is_extern_crate() { // Record an error here by looking up the namespace that had the duplicate let ns_str = match ns { TypeNS => "type or module", ValueNS => "value" }; let resolution_error = ResolutionError::DuplicateDefinition(ns_str, name); let mut err = resolve_struct_error(self, span, resolution_error); - if let Some(sp) = parent.children.borrow().get(&(name, ns)).unwrap().span { + if let Some(sp) = old_binding.span { let note = format!("first definition of {} `{}` here", ns_str, name); err.span_note(sp, ¬e); } err.emit(); + } else if old_binding.is_extern_crate() && binding.is_extern_crate() { + span_err!(self.session, + span, + E0259, + "an external crate named `{}` has already been imported into this module", + name); + } else { + span_err!(self.session, + span, + E0260, + "the name `{}` conflicts with an external crate \ + that has been imported into this module", + name); } } @@ -289,14 +306,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { self.external_exports.insert(def_id); let parent_link = ModuleParentLink(parent, name); let def = Def::Mod(def_id); - let external_module = self.new_module(parent_link, Some(def), false, true); + let external_module = self.new_extern_crate_module(parent_link, def); + self.define(parent, name, TypeNS, (external_module, sp)); - debug!("(build reduced graph for item) found extern `{}`", - module_to_string(&*external_module)); - self.check_for_conflicts_for_external_crate(parent, name, sp); - parent.external_module_children - .borrow_mut() - .insert(name, external_module); self.build_reduced_graph_for_external_crate(&external_module); } parent diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 6c78f98c0cb..6f35d10c994 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -120,8 +120,6 @@ enum SuggestionType { } pub enum ResolutionError<'a> { - /// error E0260: name conflicts with an extern crate - NameConflictsWithExternCrate(Name), /// error E0401: can't use type parameters from outer function TypeParametersFromOuterFunction, /// error E0402: cannot use an outer type parameter in this context @@ -228,14 +226,6 @@ fn resolve_struct_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>, } match resolution_error { - ResolutionError::NameConflictsWithExternCrate(name) => { - struct_span_err!(resolver.session, - span, - E0260, - "the name `{}` conflicts with an external crate \ - that has been imported into this module", - name) - } ResolutionError::TypeParametersFromOuterFunction => { struct_span_err!(resolver.session, span, @@ -801,14 +791,11 @@ pub struct ModuleS<'a> { parent_link: ParentLink<'a>, def: Cell>, is_public: bool, + is_extern_crate: bool, children: RefCell>>, imports: RefCell>, - // The external module children of this node that were declared with - // `extern crate`. - external_module_children: RefCell>>, - // The anonymous children of this node. Anonymous children are pseudo- // modules that are implicitly created around items contained within // blocks. @@ -854,9 +841,9 @@ impl<'a> ModuleS<'a> { parent_link: parent_link, def: Cell::new(def), is_public: is_public, + is_extern_crate: false, children: RefCell::new(HashMap::new()), imports: RefCell::new(Vec::new()), - external_module_children: RefCell::new(HashMap::new()), anonymous_children: RefCell::new(NodeMap()), import_resolutions: RefCell::new(HashMap::new()), glob_count: Cell::new(0), @@ -871,10 +858,21 @@ impl<'a> ModuleS<'a> { self.children.borrow().get(&(name, ns)).cloned() } - fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>) -> bool { + // If the name is not yet defined, define the name and return None. + // Otherwise, return the existing definition. + fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>) + -> Option> { match self.children.borrow_mut().entry((name, ns)) { - hash_map::Entry::Vacant(entry) => { entry.insert(binding); true } - hash_map::Entry::Occupied(_) => false, + hash_map::Entry::Vacant(entry) => { entry.insert(binding); None } + hash_map::Entry::Occupied(entry) => { Some(entry.get().clone()) }, + } + } + + fn for_each_local_child)>(&self, mut f: F) { + for (&(name, ns), name_binding) in self.children.borrow().iter() { + if !name_binding.is_extern_crate() { + f(name, ns, name_binding) + } } } @@ -1005,6 +1003,10 @@ impl<'a> NameBinding<'a> { let def = self.def().unwrap(); (def, LastMod(if self.is_public() { AllPublic } else { DependsOn(def.def_id()) })) } + + fn is_extern_crate(&self) -> bool { + self.module().map(|module| module.is_extern_crate).unwrap_or(false) + } } /// Interns the names of the primitive types. @@ -1184,6 +1186,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.arenas.modules.alloc(ModuleS::new(parent_link, def, external, is_public)) } + fn new_extern_crate_module(&self, parent_link: ParentLink<'a>, def: Def) -> Module<'a> { + let mut module = ModuleS::new(parent_link, Some(def), false, true); + module.is_extern_crate = true; + self.arenas.modules.alloc(module) + } + fn get_ribs<'b>(&'b mut self, ns: Namespace) -> &'b mut Vec> { match ns { ValueNS => &mut self.value_ribs, TypeNS => &mut self.type_ribs } } @@ -1211,32 +1219,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } - /// Check that an external crate doesn't collide with items or other external crates. - fn check_for_conflicts_for_external_crate(&self, module: Module<'a>, name: Name, span: Span) { - if module.external_module_children.borrow().contains_key(&name) { - span_err!(self.session, - span, - E0259, - "an external crate named `{}` has already been imported into this module", - name); - } - if let Some(name_binding) = module.get_child(name, TypeNS) { - resolve_error(self, - name_binding.span.unwrap_or(codemap::DUMMY_SP), - ResolutionError::NameConflictsWithExternCrate(name)); - } - } - - /// Checks that the names of items don't collide with external crates. - fn check_for_conflicts_between_external_crates_and_items(&self, - module: Module<'a>, - name: Name, - span: Span) { - if module.external_module_children.borrow().contains_key(&name) { - resolve_error(self, span, ResolutionError::NameConflictsWithExternCrate(name)); - } - } - /// Resolves the given module path from the given root `module_`. fn resolve_module_path_from_root(&mut self, module_: Module<'a>, @@ -1245,11 +1227,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { span: Span, lp: LastPrivate) -> ResolveResult<(Module<'a>, LastPrivate)> { - fn search_parent_externals<'a>(needle: Name, module: Module<'a>) - -> Option> { - match module.external_module_children.borrow().get(&needle) { - Some(_) => Some(module), - None => match module.parent_link { + fn search_parent_externals<'a>(needle: Name, module: Module<'a>) -> Option> { + match module.get_child(needle, TypeNS) { + Some(ref binding) if binding.is_extern_crate() => Some(module), + _ => match module.parent_link { ModuleParentLink(ref parent, _) => { search_parent_externals(needle, parent) } @@ -1480,17 +1461,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } - // Search for external modules. - if namespace == TypeNS { - let children = module_.external_module_children.borrow(); - if let Some(module) = children.get(&name) { - let name_binding = NameBinding::create_from_module(module, None); - debug!("lower name bindings succeeded"); - return Success((Target::new(module_, name_binding, Shadowable::Never), - false)); - } - } - // Finally, proceed up the scope chain looking for parent modules. let mut search_module = module_; loop { @@ -1684,16 +1654,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Some(..) | None => {} // Continue. } - // Finally, search through external children. - if namespace == TypeNS { - let children = module_.external_module_children.borrow(); - if let Some(module) = children.get(&name) { - let name_binding = NameBinding::create_from_module(module, None); - return Success((Target::new(module_, name_binding, Shadowable::Never), - false)); - } - } - // We're out of luck. debug!("(resolving name in module) failed to resolve `{}`", name); return Failed(None); @@ -1712,7 +1672,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // Descend into children and anonymous children. build_reduced_graph::populate_module_if_necessary(self, &module_); - for (_, child_node) in module_.children.borrow().iter() { + module_.for_each_local_child(|_, _, child_node| { match child_node.module() { None => { // Continue. @@ -1721,7 +1681,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.report_unresolved_imports(child_module); } } - } + }); for (_, module_) in module_.anonymous_children.borrow().iter() { self.report_unresolved_imports(module_); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 07f6a0f9549..53f25cbb304 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -213,7 +213,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { self.resolver.current_module = orig_module; build_reduced_graph::populate_module_if_necessary(self.resolver, &module_); - for (_, child_node) in module_.children.borrow().iter() { + module_.for_each_local_child(|_, _, child_node| { match child_node.module() { None => { // Nothing to do. @@ -222,7 +222,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { errors.extend(self.resolve_imports_for_module_subtree(child_module)); } } - } + }); for (_, child_module) in module_.anonymous_children.borrow().iter() { errors.extend(self.resolve_imports_for_module_subtree(child_module)); @@ -386,18 +386,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { -> (ResolveResult<(Module<'b>, NameBinding<'b>)>, bool) { build_reduced_graph::populate_module_if_necessary(self.resolver, module); if let Some(name_binding) = module.get_child(name, ns) { - return (Success((module, name_binding)), false); - } - - if ns == TypeNS { - if let Some(extern_crate) = module.external_module_children.borrow().get(&name) { + if name_binding.is_extern_crate() { // track the extern crate as used. - if let Some(DefId{ krate: kid, .. }) = extern_crate.def_id() { - self.resolver.used_crates.insert(kid); + if let Some(DefId { krate, .. }) = name_binding.module().unwrap().def_id() { + self.resolver.used_crates.insert(krate); } - let name_binding = NameBinding::create_from_module(extern_crate, None); - return (Success((module, name_binding)), false); } + return (Success((module, name_binding)), false) } // If there is an unresolved glob at this point in the containing module, bail out. @@ -725,13 +720,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { // Add all children from the containing module. build_reduced_graph::populate_module_if_necessary(self.resolver, &target_module); - for (&name, name_binding) in target_module.children.borrow().iter() { + target_module.for_each_local_child(|name, ns, name_binding| { self.merge_import_resolution(module_, target_module, import_directive, - name, + (name, ns), name_binding.clone()); - } + }); // Record the destination of this import if let Some(did) = target_module.def_id() { @@ -881,21 +876,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { import: &ImportResolution<'b>, import_span: Span, (name, ns): (Name, Namespace)) { - // First, check for conflicts between imports and `extern crate`s. - if ns == TypeNS { - if module.external_module_children.borrow().contains_key(&name) { - match import.target { - Some(ref target) if target.shadowable != Shadowable::Always => { - let msg = format!("import `{0}` conflicts with imported crate \ - in this module (maybe you meant `use {0}::*`?)", - name); - span_err!(self.resolver.session, import_span, E0254, "{}", &msg[..]); - } - Some(_) | None => {} - } - } - } - // Check for item conflicts. let name_binding = match module.get_child(name, ns) { None => { @@ -924,6 +904,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { } else { match import.target { Some(ref target) if target.shadowable != Shadowable::Always => { + if name_binding.is_extern_crate() { + let msg = format!("import `{0}` conflicts with imported crate \ + in this module (maybe you meant `use {0}::*`?)", + name); + span_err!(self.resolver.session, import_span, E0254, "{}", &msg[..]); + return; + } + let (what, note) = match name_binding.module() { Some(ref module) if module.is_normal() => ("existing submodule", "note conflicting module here"), diff --git a/src/test/compile-fail/resolve-conflict-item-vs-extern-crate.rs b/src/test/compile-fail/resolve-conflict-item-vs-extern-crate.rs index e685353592f..07f80cf03d1 100644 --- a/src/test/compile-fail/resolve-conflict-item-vs-extern-crate.rs +++ b/src/test/compile-fail/resolve-conflict-item-vs-extern-crate.rs @@ -8,7 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -fn std() {} //~ ERROR the name `std` conflicts with an external crate +fn std() {} +mod std {} //~ ERROR the name `std` conflicts with an external crate fn main() { }