diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 22c18c9d6a43c..a1d866fc48be4 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -16,18 +16,16 @@ use DefModifiers; use resolve_imports::ImportDirective; use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport}; -use resolve_imports::{ImportResolution, ImportResolutionPerNamespace}; +use resolve_imports::ImportResolution; use Module; -use Namespace::{TypeNS, ValueNS}; -use NameBindings; +use Namespace::{self, TypeNS, ValueNS}; +use {NameBinding, DefOrModule}; use {names_to_string, module_to_string}; use ParentLink::{ModuleParentLink, BlockParentLink}; use Resolver; use resolve_imports::Shadowable; use {resolve_error, resolve_struct_error, ResolutionError}; -use self::DuplicateCheckingMode::*; - use rustc::middle::cstore::{CrateStore, ChildItem, DlDef, DlField, DlImpl}; use rustc::middle::def::*; use rustc::middle::def_id::{CRATE_DEF_INDEX, DefId}; @@ -54,16 +52,6 @@ use rustc_front::intravisit::{self, Visitor}; use std::mem::replace; use std::ops::{Deref, DerefMut}; -// Specifies how duplicates should be handled when adding a child item if -// another item exists with the same name in some namespace. -#[derive(Copy, Clone, PartialEq)] -enum DuplicateCheckingMode { - ForbidDuplicateTypes, - ForbidDuplicateValues, - ForbidDuplicateTypesAndValues, - OverwriteDuplicates, -} - struct GraphBuilder<'a, 'b: 'a, 'tcx: 'b> { resolver: &'a mut Resolver<'b, 'tcx>, } @@ -82,6 +70,23 @@ impl<'a, 'b:'a, 'tcx:'b> DerefMut for GraphBuilder<'a, 'b, 'tcx> { } } +trait ToNameBinding<'a> { + fn to_name_binding(self) -> NameBinding<'a>; +} + +impl<'a> ToNameBinding<'a> for (Module<'a>, Span) { + fn to_name_binding(self) -> NameBinding<'a> { + NameBinding::create_from_module(self.0, Some(self.1)) + } +} + +impl<'a> ToNameBinding<'a> for (Def, Span, DefModifiers) { + fn to_name_binding(self) -> NameBinding<'a> { + let def = DefOrModule::Def(self.0); + NameBinding { modifiers: self.2, def_or_module: def, span: Some(self.1) } + } +} + impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { /// Constructs the reduced graph for the entire crate. fn build_reduced_graph(self, krate: &hir::Crate) { @@ -92,63 +97,30 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { intravisit::walk_crate(&mut visitor, krate); } - /// Adds a new child item to the module definition of the parent node, - /// or if there is already a child, does duplicate checking on the child. - /// Returns the child's corresponding name bindings. - fn add_child(&self, - name: Name, - parent: Module<'b>, - duplicate_checking_mode: DuplicateCheckingMode, - // For printing errors - sp: Span) - -> NameBindings<'b> { - self.check_for_conflicts_between_external_crates_and_items(parent, name, sp); - - // Add or reuse the child. - let child = parent.children.borrow().get(&name).cloned(); - match child { - None => { - let child = NameBindings::new(); - parent.children.borrow_mut().insert(name, child.clone()); - child - } - Some(child) => { - // Enforce the duplicate checking mode: - // - // * If we're requesting duplicate type checking, check that - // the name isn't defined in the type namespace. - // - // * If we're requesting duplicate value checking, check that - // the name isn't defined in the value namespace. - // - // * If we're requesting duplicate type and value checking, - // check that the name isn't defined in either namespace. - // - // * If no duplicate checking was requested at all, do - // nothing. - - let ns = match duplicate_checking_mode { - ForbidDuplicateTypes if child.type_ns.defined() => TypeNS, - ForbidDuplicateValues if child.value_ns.defined() => ValueNS, - ForbidDuplicateTypesAndValues if child.type_ns.defined() => TypeNS, - ForbidDuplicateTypesAndValues if child.value_ns.defined() => ValueNS, - _ => return child, - }; - - // 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 mut err = resolve_struct_error(self, - sp, - ResolutionError::DuplicateDefinition(ns_str, - name)); + /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined. + fn try_define(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T) + where T: ToNameBinding<'b> + { + parent.try_define_child(name, ns, def.to_name_binding()); + } - if let Some(sp) = child[ns].span() { - let note = format!("first definition of {} `{}` here", ns_str, name); - err.span_note(sp, ¬e); - } - err.emit(); - child + /// 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) { + // 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 { + let note = format!("first definition of {} `{}` here", ns_str, name); + err.span_note(sp, ¬e); } + err.emit(); } } @@ -331,12 +303,10 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { } ItemMod(..) => { - let name_bindings = self.add_child(name, parent, ForbidDuplicateTypes, sp); - let parent_link = ModuleParentLink(parent, name); let def = Def::Mod(self.ast_map.local_def_id(item.id)); let module = self.new_module(parent_link, Some(def), false, is_public); - name_bindings.define_module(module.clone(), sp); + self.define(parent, name, TypeNS, (module, sp)); module } @@ -344,51 +314,36 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { // These items live in the value namespace. ItemStatic(_, m, _) => { - let name_bindings = self.add_child(name, parent, ForbidDuplicateValues, sp); let mutbl = m == hir::MutMutable; - - name_bindings.define_value(Def::Static(self.ast_map.local_def_id(item.id), mutbl), - sp, - modifiers); + let def = Def::Static(self.ast_map.local_def_id(item.id), mutbl); + self.define(parent, name, ValueNS, (def, sp, modifiers)); parent } ItemConst(_, _) => { - self.add_child(name, parent, ForbidDuplicateValues, sp) - .define_value(Def::Const(self.ast_map.local_def_id(item.id)), sp, modifiers); + let def = Def::Const(self.ast_map.local_def_id(item.id)); + self.define(parent, name, ValueNS, (def, sp, modifiers)); parent } ItemFn(_, _, _, _, _, _) => { - let name_bindings = self.add_child(name, parent, ForbidDuplicateValues, sp); - let def = Def::Fn(self.ast_map.local_def_id(item.id)); - name_bindings.define_value(def, sp, modifiers); + self.define(parent, name, ValueNS, (def, sp, modifiers)); parent } // These items live in the type namespace. ItemTy(..) => { - let name_bindings = self.add_child(name, - parent, - ForbidDuplicateTypes, - sp); - let parent_link = ModuleParentLink(parent, name); let def = Def::TyAlias(self.ast_map.local_def_id(item.id)); let module = self.new_module(parent_link, Some(def), false, is_public); - name_bindings.define_module(module, sp); + self.define(parent, name, TypeNS, (module, sp)); parent } ItemEnum(ref enum_definition, _) => { - let name_bindings = self.add_child(name, - parent, - ForbidDuplicateTypes, - sp); - let parent_link = ModuleParentLink(parent, name); let def = Def::Enum(self.ast_map.local_def_id(item.id)); let module = self.new_module(parent_link, Some(def), false, is_public); - name_bindings.define_module(module.clone(), sp); + self.define(parent, name, TypeNS, (module, sp)); let variant_modifiers = if is_public { DefModifiers::empty() @@ -405,26 +360,15 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { // These items live in both the type and value namespaces. ItemStruct(ref struct_def, _) => { - // Adding to both Type and Value namespaces or just Type? - let (forbid, ctor_id) = if struct_def.is_struct() { - (ForbidDuplicateTypes, None) - } else { - (ForbidDuplicateTypesAndValues, Some(struct_def.id())) - }; - - let name_bindings = self.add_child(name, parent, forbid, sp); - // Define a name in the type namespace. - name_bindings.define_type(Def::Struct(self.ast_map.local_def_id(item.id)), - sp, - modifiers); + let def = Def::Struct(self.ast_map.local_def_id(item.id)); + self.define(parent, name, TypeNS, (def, sp, modifiers)); // If this is a newtype or unit-like struct, define a name // in the value namespace as well - if let Some(cid) = ctor_id { - name_bindings.define_value(Def::Struct(self.ast_map.local_def_id(cid)), - sp, - modifiers); + if !struct_def.is_struct() { + let def = Def::Struct(self.ast_map.local_def_id(struct_def.id())); + self.define(parent, name, ValueNS, (def, sp, modifiers)); } // Record the def ID and fields of this struct. @@ -447,48 +391,27 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { ItemImpl(..) => parent, ItemTrait(_, _, _, ref items) => { - let name_bindings = self.add_child(name, - parent, - ForbidDuplicateTypes, - sp); - let def_id = self.ast_map.local_def_id(item.id); // Add all the items within to a new module. let parent_link = ModuleParentLink(parent, name); let def = Def::Trait(def_id); let module_parent = self.new_module(parent_link, Some(def), false, is_public); - name_bindings.define_module(module_parent.clone(), sp); + self.define(parent, name, TypeNS, (module_parent, sp)); // Add the names of all the items to the trait info. - for trait_item in items { - let name_bindings = self.add_child(trait_item.name, - &module_parent, - ForbidDuplicateTypesAndValues, - trait_item.span); - - match trait_item.node { - hir::ConstTraitItem(..) => { - let def = Def::AssociatedConst(self.ast_map. - local_def_id(trait_item.id)); - // NB: not DefModifiers::IMPORTABLE - name_bindings.define_value(def, trait_item.span, DefModifiers::PUBLIC); - } - hir::MethodTraitItem(..) => { - let def = Def::Method(self.ast_map.local_def_id(trait_item.id)); - // NB: not DefModifiers::IMPORTABLE - name_bindings.define_value(def, trait_item.span, DefModifiers::PUBLIC); - } - hir::TypeTraitItem(..) => { - let def = Def::AssociatedTy(self.ast_map.local_def_id(item.id), - self.ast_map.local_def_id(trait_item.id)); - // NB: not DefModifiers::IMPORTABLE - name_bindings.define_type(def, trait_item.span, DefModifiers::PUBLIC); - } - } + for item in items { + let item_def_id = self.ast_map.local_def_id(item.id); + let (def, ns) = match item.node { + hir::ConstTraitItem(..) => (Def::AssociatedConst(item_def_id), ValueNS), + hir::MethodTraitItem(..) => (Def::Method(item_def_id), ValueNS), + hir::TypeTraitItem(..) => (Def::AssociatedTy(def_id, item_def_id), TypeNS), + }; - let trait_item_def_id = self.ast_map.local_def_id(trait_item.id); - self.trait_item_map.insert((trait_item.name, def_id), trait_item_def_id); + let modifiers = DefModifiers::PUBLIC; // NB: not DefModifiers::IMPORTABLE + self.define(&module_parent, item.name, ns, (def, item.span, modifiers)); + + self.trait_item_map.insert((item.name, def_id), item_def_id); } parent @@ -512,13 +435,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { // Variants are always treated as importable to allow them to be glob used. // All variants are defined in both type and value namespaces as future-proofing. - let child = self.add_child(name, parent, ForbidDuplicateTypesAndValues, variant.span); - child.define_value(Def::Variant(item_id, self.ast_map.local_def_id(variant.node.data.id())), - variant.span, - DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers); - child.define_type(Def::Variant(item_id, self.ast_map.local_def_id(variant.node.data.id())), - variant.span, - DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers); + let modifiers = DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers; + let def = Def::Variant(item_id, self.ast_map.local_def_id(variant.node.data.id())); + + self.define(parent, name, ValueNS, (def, variant.span, modifiers)); + self.define(parent, name, TypeNS, (def, variant.span, modifiers)); } /// Constructs the reduced graph for one foreign item. @@ -532,7 +453,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { } else { DefModifiers::empty() } | DefModifiers::IMPORTABLE; - let name_bindings = self.add_child(name, parent, ForbidDuplicateValues, foreign_item.span); let def = match foreign_item.node { ForeignItemFn(..) => { @@ -542,7 +462,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { Def::Static(self.ast_map.local_def_id(foreign_item.id), m) } }; - name_bindings.define_value(def, foreign_item.span, modifiers); + self.define(parent, name, ValueNS, (def, foreign_item.span, modifiers)); } fn build_reduced_graph_for_block(&mut self, block: &Block, parent: Module<'b>) -> Module<'b> { @@ -565,7 +485,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { fn handle_external_def(&mut self, def: Def, vis: Visibility, - child_name_bindings: &NameBindings<'b>, final_ident: &str, name: Name, new_parent: Module<'b>) { @@ -573,11 +492,15 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { final_ident, vis); let is_public = vis == hir::Public; - let modifiers = if is_public { - DefModifiers::PUBLIC - } else { - DefModifiers::empty() - } | DefModifiers::IMPORTABLE; + + let mut modifiers = DefModifiers::empty(); + if is_public { + modifiers = modifiers | DefModifiers::PUBLIC; + } + if new_parent.is_normal() { + modifiers = modifiers | DefModifiers::IMPORTABLE; + } + let is_exported = is_public && match new_parent.def_id() { None => true, @@ -586,43 +509,24 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { if is_exported { self.external_exports.insert(def.def_id()); } - let is_struct_ctor = if let Def::Struct(def_id) = def { - self.session.cstore.tuple_struct_definition_if_ctor(def_id).is_some() - } else { - false - }; match def { - Def::Mod(_) | - Def::ForeignMod(_) | - Def::Struct(..) | - Def::Enum(..) | - Def::TyAlias(..) if !is_struct_ctor => { - if let Some(module_def) = child_name_bindings.type_ns.module() { - debug!("(building reduced graph for external crate) already created module"); - module_def.def.set(Some(def)); - } else { - debug!("(building reduced graph for external crate) building module {} {}", - final_ident, - is_public); - let parent_link = ModuleParentLink(new_parent, name); - let module = self.new_module(parent_link, Some(def), true, is_public); - child_name_bindings.define_module(module, DUMMY_SP); - } + Def::Mod(_) | Def::ForeignMod(_) | Def::Enum(..) | Def::TyAlias(..) => { + debug!("(building reduced graph for external crate) building module {} {}", + final_ident, + is_public); + let parent_link = ModuleParentLink(new_parent, name); + let module = self.new_module(parent_link, Some(def), true, is_public); + self.try_define(new_parent, name, TypeNS, (module, DUMMY_SP)); } - _ => {} - } - - match def { - Def::Mod(_) | Def::ForeignMod(_) => {} Def::Variant(_, variant_id) => { debug!("(building reduced graph for external crate) building variant {}", final_ident); // Variants are always treated as importable to allow them to be glob used. // All variants are defined in both type and value namespaces as future-proofing. let modifiers = DefModifiers::PUBLIC | DefModifiers::IMPORTABLE; - child_name_bindings.define_type(def, DUMMY_SP, modifiers); - child_name_bindings.define_value(def, DUMMY_SP, modifiers); + self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers)); + self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers)); if self.session.cstore.variant_kind(variant_id) == Some(VariantKind::Struct) { // Not adding fields for variants as they are not accessed with a self receiver self.structs.insert(variant_id, Vec::new()); @@ -635,17 +539,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { Def::Method(..) => { debug!("(building reduced graph for external crate) building value (fn/static) {}", final_ident); - // impl methods have already been defined with the correct importability - // modifier - let mut modifiers = match *child_name_bindings.value_ns.borrow() { - Some(ref def) => (modifiers & !DefModifiers::IMPORTABLE) | - (def.modifiers & DefModifiers::IMPORTABLE), - None => modifiers, - }; - if !new_parent.is_normal() { - modifiers = modifiers & !DefModifiers::IMPORTABLE; - } - child_name_bindings.define_value(def, DUMMY_SP, modifiers); + self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers)); } Def::Trait(def_id) => { debug!("(building reduced graph for external crate) building type {}", @@ -670,45 +564,31 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { } } - // Define a module if necessary. let parent_link = ModuleParentLink(new_parent, name); let module = self.new_module(parent_link, Some(def), true, is_public); - child_name_bindings.define_module(module, DUMMY_SP); + self.try_define(new_parent, name, TypeNS, (module, DUMMY_SP)); } - Def::Enum(..) | Def::TyAlias(..) | Def::AssociatedTy(..) => { + Def::AssociatedTy(..) => { debug!("(building reduced graph for external crate) building type {}", final_ident); - - let modifiers = match new_parent.is_normal() { - true => modifiers, - _ => modifiers & !DefModifiers::IMPORTABLE, - }; - - if let Def::Enum(..) = def { - child_name_bindings.type_ns.set_modifiers(modifiers); - } else if let Def::TyAlias(..) = def { - child_name_bindings.type_ns.set_modifiers(modifiers); - } else { - child_name_bindings.define_type(def, DUMMY_SP, modifiers); - } + self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers)); } - Def::Struct(..) if is_struct_ctor => { - // Do nothing - } - Def::Struct(def_id) => { + Def::Struct(def_id) + if self.session.cstore.tuple_struct_definition_if_ctor(def_id).is_none() => { debug!("(building reduced graph for external crate) building type and value for \ {}", final_ident); - - child_name_bindings.define_type(def, DUMMY_SP, modifiers); + self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers)); if let Some(ctor_def_id) = self.session.cstore.struct_ctor_def_id(def_id) { - child_name_bindings.define_value(Def::Struct(ctor_def_id), DUMMY_SP, modifiers); + let def = Def::Struct(ctor_def_id); + self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers)); } // Record the def ID and fields of this struct. let fields = self.session.cstore.struct_field_names(def_id); self.structs.insert(def_id, fields); } + Def::Struct(..) => {} Def::Local(..) | Def::PrimTy(..) | Def::TyParam(..) | @@ -737,14 +617,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { } } _ => { - let child_name_bindings = self.add_child(xcdef.name, - root, - OverwriteDuplicates, - DUMMY_SP); - self.handle_external_def(def, xcdef.vis, - &child_name_bindings, &xcdef.name.as_str(), xcdef.name, root); @@ -827,24 +701,16 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { target); let mut import_resolutions = module_.import_resolutions.borrow_mut(); - match import_resolutions.get_mut(&target) { - Some(resolution_per_ns) => { - debug!("(building import directive) bumping reference"); - resolution_per_ns.outstanding_references += 1; - - // the source of this name is different now - let resolution = - ImportResolution { id: id, is_public: is_public, target: None }; - resolution_per_ns[TypeNS] = resolution.clone(); - resolution_per_ns[ValueNS] = resolution; - return; - } - None => {} + for &ns in [TypeNS, ValueNS].iter() { + let mut resolution = import_resolutions.entry((target, ns)).or_insert( + ImportResolution::new(id, is_public) + ); + + resolution.outstanding_references += 1; + // the source of this name is different now + resolution.id = id; + resolution.is_public = is_public; } - debug!("(building import directive) creating new"); - let mut import_resolution_per_ns = ImportResolutionPerNamespace::new(id, is_public); - import_resolution_per_ns.outstanding_references = 1; - import_resolutions.insert(target, import_resolution_per_ns); } GlobImport => { // Set the glob flag. This tells us that we don't know the diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 4b62e65bb0f13..6c78f98c0cb0d 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -36,7 +36,6 @@ extern crate rustc; use self::PatternBindingMode::*; use self::Namespace::*; -use self::NamespaceResult::*; use self::ResolveResult::*; use self::FallbackSuggestion::*; use self::TypeParameters::*; @@ -87,13 +86,12 @@ use rustc_front::hir::{TraitRef, Ty, TyBool, TyChar, TyFloat, TyInt}; use rustc_front::hir::{TyRptr, TyStr, TyUint, TyPath, TyPtr}; use rustc_front::util::walk_pat; -use std::collections::{HashMap, HashSet}; +use std::collections::{hash_map, HashMap, HashSet}; use std::cell::{Cell, RefCell}; use std::fmt; use std::mem::replace; -use std::rc::Rc; -use resolve_imports::{Target, ImportDirective, ImportResolutionPerNamespace}; +use resolve_imports::{Target, ImportDirective, ImportResolution}; use resolve_imports::Shadowable; // NB: This module needs to be declared first so diagnostics are @@ -357,8 +355,8 @@ fn resolve_struct_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>, if let Some(directive) = resolver.current_module .import_resolutions .borrow() - .get(&name) { - let item = resolver.ast_map.expect_item(directive.value_ns.id); + .get(&(name, ValueNS)) { + let item = resolver.ast_map.expect_item(directive.id); err.span_note(item.span, "constant imported here"); } err @@ -572,38 +570,6 @@ pub enum Namespace { ValueNS, } -/// A NamespaceResult represents the result of resolving an import in -/// a particular namespace. The result is either definitely-resolved, -/// definitely- unresolved, or unknown. -#[derive(Clone)] -enum NamespaceResult<'a> { - /// Means that resolve hasn't gathered enough information yet to determine - /// whether the name is bound in this namespace. (That is, it hasn't - /// resolved all `use` directives yet.) - UnknownResult, - /// Means that resolve has determined that the name is definitely - /// not bound in the namespace. - UnboundResult, - /// Means that resolve has determined that the name is bound in the Module - /// argument, and specified by the NameBinding argument. - BoundResult(Module<'a>, NameBinding<'a>), -} - -impl<'a> NamespaceResult<'a> { - fn is_unknown(&self) -> bool { - match *self { - UnknownResult => true, - _ => false, - } - } - fn is_unbound(&self) -> bool { - match *self { - UnboundResult => true, - _ => false, - } - } -} - impl<'a, 'v, 'tcx> Visitor<'v> for Resolver<'a, 'tcx> { fn visit_nested_item(&mut self, item: hir::ItemId) { self.visit_item(self.ast_map.expect_item(item.id)) @@ -698,6 +664,7 @@ impl<'a, 'v, 'tcx> Visitor<'v> for Resolver<'a, 'tcx> { type ErrorMessage = Option<(Span, String)>; +#[derive(Clone, PartialEq, Eq)] enum ResolveResult { Failed(ErrorMessage), // Failed to resolve the name, optional helpful error message. Indeterminate, // Couldn't determine due to unresolved globs. @@ -835,7 +802,7 @@ pub struct ModuleS<'a> { def: Cell>, is_public: bool, - children: RefCell>>, + children: RefCell>>, imports: RefCell>, // The external module children of this node that were declared with @@ -859,7 +826,7 @@ pub struct ModuleS<'a> { anonymous_children: RefCell>>, // The status of resolving each import in this module. - import_resolutions: RefCell>>, + import_resolutions: RefCell>>, // The number of unresolved globs that this module exports. glob_count: Cell, @@ -900,6 +867,17 @@ impl<'a> ModuleS<'a> { } } + fn get_child(&self, name: Name, ns: Namespace) -> Option> { + self.children.borrow().get(&(name, ns)).cloned() + } + + fn try_define_child(&self, name: Name, ns: Namespace, binding: NameBinding<'a>) -> bool { + match self.children.borrow_mut().entry((name, ns)) { + hash_map::Entry::Vacant(entry) => { entry.insert(binding); true } + hash_map::Entry::Occupied(_) => false, + } + } + fn def_id(&self) -> Option { self.def.get().as_ref().map(Def::def_id) } @@ -977,20 +955,20 @@ bitflags! { } // Records a possibly-private value, type, or module definition. -#[derive(Debug)] -struct NsDef<'a> { - modifiers: DefModifiers, // see note in ImportResolutionPerNamespace about how to use this +#[derive(Clone, Debug)] +pub struct NameBinding<'a> { + modifiers: DefModifiers, // see note in ImportResolution about how to use this def_or_module: DefOrModule<'a>, span: Option, } -#[derive(Debug)] +#[derive(Clone, Debug)] enum DefOrModule<'a> { Def(Def), Module(Module<'a>), } -impl<'a> NsDef<'a> { +impl<'a> NameBinding<'a> { fn create_from_module(module: Module<'a>, span: Option) -> Self { let modifiers = if module.is_public { DefModifiers::PUBLIC @@ -998,11 +976,7 @@ impl<'a> NsDef<'a> { DefModifiers::empty() } | DefModifiers::IMPORTABLE; - NsDef { modifiers: modifiers, def_or_module: DefOrModule::Module(module), span: span } - } - - fn create_from_def(def: Def, modifiers: DefModifiers, span: Option) -> Self { - NsDef { modifiers: modifiers, def_or_module: DefOrModule::Def(def), span: span } + NameBinding { modifiers: modifiers, def_or_module: DefOrModule::Module(module), span: span } } fn module(&self) -> Option> { @@ -1018,55 +992,9 @@ impl<'a> NsDef<'a> { DefOrModule::Module(ref module) => module.def.get(), } } -} - -// Records at most one definition that a name in a namespace is bound to -#[derive(Clone,Debug)] -pub struct NameBinding<'a>(Rc>>>); - -impl<'a> NameBinding<'a> { - fn new() -> Self { - NameBinding(Rc::new(RefCell::new(None))) - } - - fn create_from_module(module: Module<'a>) -> Self { - NameBinding(Rc::new(RefCell::new(Some(NsDef::create_from_module(module, None))))) - } - - fn set(&self, ns_def: NsDef<'a>) { - *self.0.borrow_mut() = Some(ns_def); - } - - fn set_modifiers(&self, modifiers: DefModifiers) { - if let Some(ref mut ns_def) = *self.0.borrow_mut() { - ns_def.modifiers = modifiers - } - } - - fn borrow(&self) -> ::std::cell::Ref>> { - self.0.borrow() - } - - // Lifted versions of the NsDef methods and fields - fn def(&self) -> Option { - self.borrow().as_ref().and_then(NsDef::def) - } - fn module(&self) -> Option> { - self.borrow().as_ref().and_then(NsDef::module) - } - fn span(&self) -> Option { - self.borrow().as_ref().and_then(|def| def.span) - } - fn modifiers(&self) -> Option { - self.borrow().as_ref().and_then(|def| Some(def.modifiers)) - } - - fn defined(&self) -> bool { - self.borrow().is_some() - } fn defined_with(&self, modifiers: DefModifiers) -> bool { - self.modifiers().map(|m| m.contains(modifiers)).unwrap_or(false) + self.modifiers.contains(modifiers) } fn is_public(&self) -> bool { @@ -1079,47 +1007,6 @@ impl<'a> NameBinding<'a> { } } -// Records the definitions (at most one for each namespace) that a name is -// bound to. -#[derive(Clone,Debug)] -pub struct NameBindings<'a> { - type_ns: NameBinding<'a>, // < Meaning in type namespace. - value_ns: NameBinding<'a>, // < Meaning in value namespace. -} - -impl<'a> ::std::ops::Index for NameBindings<'a> { - type Output = NameBinding<'a>; - fn index(&self, namespace: Namespace) -> &NameBinding<'a> { - match namespace { TypeNS => &self.type_ns, ValueNS => &self.value_ns } - } -} - -impl<'a> NameBindings<'a> { - fn new() -> Self { - NameBindings { - type_ns: NameBinding::new(), - value_ns: NameBinding::new(), - } - } - - /// Creates a new module in this set of name bindings. - fn define_module(&self, module: Module<'a>, sp: Span) { - self.type_ns.set(NsDef::create_from_module(module, Some(sp))); - } - - /// Records a type definition. - fn define_type(&self, def: Def, sp: Span, modifiers: DefModifiers) { - debug!("defining type for def {:?} with modifiers {:?}", def, modifiers); - self.type_ns.set(NsDef::create_from_def(def, modifiers, Some(sp))); - } - - /// Records a value definition. - fn define_value(&self, def: Def, sp: Span, modifiers: DefModifiers) { - debug!("defining value for def {:?} with modifiers {:?}", def, modifiers); - self.value_ns.set(NsDef::create_from_def(def, modifiers, Some(sp))); - } -} - /// Interns the names of the primitive types. struct PrimitiveTypeTable { primitive_types: HashMap, @@ -1333,13 +1220,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { "an external crate named `{}` has already been imported into this module", name); } - match module.children.borrow().get(&name) { - Some(name_bindings) if name_bindings.type_ns.defined() => { - resolve_error(self, - name_bindings.type_ns.span().unwrap_or(codemap::DUMMY_SP), - ResolutionError::NameConflictsWithExternCrate(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)); } } @@ -1562,25 +1446,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // its immediate children. build_reduced_graph::populate_module_if_necessary(self, &module_); - match module_.children.borrow().get(&name) { - Some(name_bindings) if name_bindings[namespace].defined() => { - debug!("top name bindings succeeded"); - return Success((Target::new(module_, - name_bindings[namespace].clone(), - Shadowable::Never), - false)); - } - Some(_) | None => { - // Not found; continue. - } + if let Some(binding) = module_.get_child(name, namespace) { + debug!("top name bindings succeeded"); + return Success((Target::new(module_, binding, Shadowable::Never), false)); } // Now check for its import directives. We don't have to have resolved // all its imports in the usual way; this is because chains of // adjacent import statements are processed as though they mutated the // current scope. - if let Some(import_resolution) = module_.import_resolutions.borrow().get(&name) { - match import_resolution[namespace].target.clone() { + if let Some(import_resolution) = + module_.import_resolutions.borrow().get(&(name, namespace)) { + match import_resolution.target.clone() { None => { // Not found; continue. debug!("(resolving item in lexical scope) found import resolution, but not \ @@ -1590,7 +1467,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Some(target) => { debug!("(resolving item in lexical scope) using import resolution"); // track used imports and extern crates as well - let id = import_resolution[namespace].id; + let id = import_resolution.id; if record_used { self.used_imports.insert((id, namespace)); self.record_import_use(id, name); @@ -1607,7 +1484,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { 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); + let name_binding = NameBinding::create_from_module(module, None); debug!("lower name bindings succeeded"); return Success((Target::new(module_, name_binding, Shadowable::Never), false)); @@ -1774,32 +1651,19 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // First, check the direct children of the module. build_reduced_graph::populate_module_if_necessary(self, &module_); - let children = module_.children.borrow(); - match children.get(&name) { - Some(name_bindings) if name_bindings[namespace].defined() => { - debug!("(resolving name in module) found node as child"); - return Success((Target::new(module_, - name_bindings[namespace].clone(), - Shadowable::Never), - false)); - } - Some(_) | None => { - // Continue. - } + if let Some(binding) = module_.get_child(name, namespace) { + debug!("(resolving name in module) found node as child"); + return Success((Target::new(module_, binding, Shadowable::Never), false)); } // Check the list of resolved imports. - let children = module_.import_resolutions.borrow(); - match children.get(&name) { - Some(import_resolution) if allow_private_imports || - import_resolution[namespace].is_public => { - - if import_resolution[namespace].is_public && - import_resolution.outstanding_references != 0 { + match module_.import_resolutions.borrow().get(&(name, namespace)) { + Some(import_resolution) if allow_private_imports || import_resolution.is_public => { + if import_resolution.is_public && import_resolution.outstanding_references != 0 { debug!("(resolving name in module) import unresolved; bailing out"); return Indeterminate; } - match import_resolution[namespace].target.clone() { + match import_resolution.target.clone() { None => { debug!("(resolving name in module) name found, but not in namespace {:?}", namespace); @@ -1807,7 +1671,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Some(target) => { debug!("(resolving name in module) resolved to import"); // track used imports and extern crates as well - let id = import_resolution[namespace].id; + let id = import_resolution.id; self.used_imports.insert((id, namespace)); self.record_import_use(id, name); if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() { @@ -1824,7 +1688,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { 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); + let name_binding = NameBinding::create_from_module(module, None); return Success((Target::new(module_, name_binding, Shadowable::Never), false)); } @@ -1849,7 +1713,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { build_reduced_graph::populate_module_if_necessary(self, &module_); for (_, child_node) in module_.children.borrow().iter() { - match child_node.type_ns.module() { + match child_node.module() { None => { // Continue. } @@ -1895,14 +1759,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Some(name) => { build_reduced_graph::populate_module_if_necessary(self, &orig_module); - match orig_module.children.borrow().get(&name) { + match orig_module.get_child(name, TypeNS) { None => { debug!("!!! (with scope) didn't find `{}` in `{}`", name, module_to_string(&*orig_module)); } - Some(name_bindings) => { - match name_bindings.type_ns.module() { + Some(name_binding) => { + match name_binding.module() { None => { debug!("!!! (with scope) didn't find module for `{}` in `{}`", name, @@ -2858,7 +2722,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Success((target, _)) => { debug!("(resolve bare identifier pattern) succeeded in finding {} at {:?}", name, - target.binding.borrow()); + &target.binding); match target.binding.def() { None => { panic!("resolved name in the value namespace to a set of name bindings \ @@ -3331,12 +3195,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { if name_path.len() == 1 { match this.primitive_type_table.primitive_types.get(last_name) { Some(_) => None, - None => { - match this.current_module.children.borrow().get(last_name) { - Some(child) => child.type_ns.module(), - None => None, - } - } + None => this.current_module.get_child(*last_name, TypeNS) + .as_ref() + .and_then(NameBinding::module) } } else { match this.resolve_module_path(root, &name_path, UseLexicalScope, span) { @@ -3395,8 +3256,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // Look for a method in the current self type's impl module. if let Some(module) = get_module(self, path.span, &name_path) { - if let Some(binding) = module.children.borrow().get(&name) { - if let Some(Def::Method(did)) = binding.value_ns.def() { + if let Some(binding) = module.get_child(name, ValueNS) { + if let Some(Def::Method(did)) = binding.def() { if is_static_method(self, did) { return StaticMethod(path_names_to_string(&path, 0)); } @@ -3718,27 +3579,23 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // Look for trait children. build_reduced_graph::populate_module_if_necessary(self, &search_module); - { - for (_, child_names) in search_module.children.borrow().iter() { - let def = match child_names.type_ns.def() { - Some(def) => def, - None => continue, - }; - let trait_def_id = match def { - Def::Trait(trait_def_id) => trait_def_id, - _ => continue, - }; - if self.trait_item_map.contains_key(&(name, trait_def_id)) { - add_trait_info(&mut found_traits, trait_def_id, name); - } + for (&(_, ns), name_binding) in search_module.children.borrow().iter() { + if ns != TypeNS { continue } + let trait_def_id = match name_binding.def() { + Some(Def::Trait(trait_def_id)) => trait_def_id, + Some(..) | None => continue, + }; + if self.trait_item_map.contains_key(&(name, trait_def_id)) { + add_trait_info(&mut found_traits, trait_def_id, name); } } // Look for imports. - for (_, import) in search_module.import_resolutions.borrow().iter() { - let target = match import.type_ns.target { - None => continue, + for (&(_, ns), import) in search_module.import_resolutions.borrow().iter() { + if ns != TypeNS { continue } + let target = match import.target { Some(ref target) => target, + None => continue, }; let did = match target.binding.def() { Some(Def::Trait(trait_def_id)) => trait_def_id, @@ -3746,7 +3603,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { }; if self.trait_item_map.contains_key(&(name, did)) { add_trait_info(&mut found_traits, did, name); - let id = import.type_ns.id; + let id = import.id; self.used_imports.insert((id, TypeNS)); let trait_name = self.get_trait_name(did); self.record_import_use(id, trait_name); @@ -3797,52 +3654,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } } - - // - // Diagnostics - // - // Diagnostics are not particularly efficient, because they're rarely - // hit. - // - - #[allow(dead_code)] // useful for debugging - fn dump_module(&mut self, module_: Module<'a>) { - debug!("Dump of module `{}`:", module_to_string(&*module_)); - - debug!("Children:"); - build_reduced_graph::populate_module_if_necessary(self, &module_); - for (&name, _) in module_.children.borrow().iter() { - debug!("* {}", name); - } - - debug!("Import resolutions:"); - let import_resolutions = module_.import_resolutions.borrow(); - for (&name, import_resolution) in import_resolutions.iter() { - let value_repr; - match import_resolution.value_ns.target { - None => { - value_repr = "".to_string(); - } - Some(_) => { - value_repr = " value:?".to_string(); - // FIXME #4954 - } - } - - let type_repr; - match import_resolution.type_ns.target { - None => { - type_repr = "".to_string(); - } - Some(_) => { - type_repr = " type:?".to_string(); - // FIXME #4954 - } - } - - debug!("* {}:{}{}", name, value_repr, type_repr); - } - } } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 364218d84137c..07f6a0f954990 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -13,10 +13,9 @@ use self::ImportDirectiveSubclass::*; use DefModifiers; use Module; use Namespace::{self, TypeNS, ValueNS}; -use {NameBindings, NameBinding}; -use NamespaceResult::{BoundResult, UnboundResult, UnknownResult}; -use NamespaceResult; +use NameBinding; use ResolveResult; +use ResolveResult::*; use Resolver; use UseLexicalScopeFlag; use {names_to_string, module_to_string}; @@ -100,26 +99,20 @@ impl<'a> Target<'a> { } #[derive(Debug)] -/// An ImportResolutionPerNamespace records what we know about an imported name. +/// An ImportResolution records what we know about an imported name in a given namespace. /// More specifically, it records the number of unresolved `use` directives that import the name, -/// and for each namespace, it records the `use` directive importing the name in the namespace -/// and the `Target` to which the name in the namespace resolves (if applicable). +/// the `use` directive importing the name in the namespace, and the `NameBinding` to which the +/// name in the namespace resolves (if applicable). /// Different `use` directives may import the same name in different namespaces. -pub struct ImportResolutionPerNamespace<'a> { +pub struct ImportResolution<'a> { // When outstanding_references reaches zero, outside modules can count on the targets being // correct. Before then, all bets are off; future `use` directives could override the name. // Since shadowing is forbidden, the only way outstanding_references > 1 in a legal program // is if the name is imported by exactly two `use` directives, one of which resolves to a // value and the other of which resolves to a type. pub outstanding_references: usize, - pub type_ns: ImportResolution<'a>, - pub value_ns: ImportResolution<'a>, -} -/// Records what we know about an imported name in a namespace (see `ImportResolutionPerNamespace`). -#[derive(Clone,Debug)] -pub struct ImportResolution<'a> { - /// Whether the name in the namespace was imported with a `use` or a `pub use`. + /// Whether this resolution came from a `use` or a `pub use`. pub is_public: bool, /// Resolution of the name in the namespace @@ -129,29 +122,18 @@ pub struct ImportResolution<'a> { pub id: NodeId, } -impl<'a> ::std::ops::Index for ImportResolutionPerNamespace<'a> { - type Output = ImportResolution<'a>; - fn index(&self, ns: Namespace) -> &ImportResolution<'a> { - match ns { TypeNS => &self.type_ns, ValueNS => &self.value_ns } - } -} - -impl<'a> ::std::ops::IndexMut for ImportResolutionPerNamespace<'a> { - fn index_mut(&mut self, ns: Namespace) -> &mut ImportResolution<'a> { - match ns { TypeNS => &mut self.type_ns, ValueNS => &mut self.value_ns } - } -} - -impl<'a> ImportResolutionPerNamespace<'a> { +impl<'a> ImportResolution<'a> { pub fn new(id: NodeId, is_public: bool) -> Self { - let resolution = ImportResolution { id: id, is_public: is_public, target: None }; - ImportResolutionPerNamespace { - outstanding_references: 0, type_ns: resolution.clone(), value_ns: resolution, + ImportResolution { + outstanding_references: 0, + id: id, + target: None, + is_public: is_public, } } - pub fn shadowable(&self, namespace: Namespace) -> Shadowable { - match self[namespace].target { + pub fn shadowable(&self) -> Shadowable { + match self.target { Some(ref target) => target.shadowable, None => Shadowable::Always, } @@ -232,7 +214,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { build_reduced_graph::populate_module_if_necessary(self.resolver, &module_); for (_, child_node) in module_.children.borrow().iter() { - match child_node.type_ns.module() { + match child_node.module() { None => { // Nothing to do. } @@ -393,6 +375,80 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { return resolution_result; } + /// Resolves the name in the namespace of the module because it is being imported by + /// importing_module. Returns the module in which the name was defined (as opposed to imported), + /// the name bindings defining the name, and whether or not the name was imported into `module`. + fn resolve_name_in_module(&mut self, + module: Module<'b>, // Module containing the name + name: Name, + ns: Namespace, + importing_module: Module<'b>) // Module importing the name + -> (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) { + // track the extern crate as used. + if let Some(DefId{ krate: kid, .. }) = extern_crate.def_id() { + self.resolver.used_crates.insert(kid); + } + let name_binding = NameBinding::create_from_module(extern_crate, None); + return (Success((module, name_binding)), false); + } + } + + // If there is an unresolved glob at this point in the containing module, bail out. + // We don't know enough to be able to resolve the name. + if module.pub_glob_count.get() > 0 { + return (Indeterminate, false); + } + + match module.import_resolutions.borrow().get(&(name, ns)) { + // The containing module definitely doesn't have an exported import with the + // name in question. We can therefore accurately report that names are unbound. + None => (Failed(None), false), + + // The name is an import which has been fully resolved, so we just follow it. + Some(resolution) if resolution.outstanding_references == 0 => { + // Import resolutions must be declared with "pub" in order to be exported. + if !resolution.is_public { + return (Failed(None), false); + } + + let target = resolution.target.clone(); + if let Some(Target { target_module, binding, shadowable: _ }) = target { + // track used imports and extern crates as well + self.resolver.used_imports.insert((resolution.id, ns)); + self.resolver.record_import_use(resolution.id, name); + if let Some(DefId { krate, .. }) = target_module.def_id() { + self.resolver.used_crates.insert(krate); + } + (Success((target_module, binding)), true) + } else { + (Failed(None), false) + } + } + + // If module is the same module whose import we are resolving and + // it has an unresolved import with the same name as `name`, then the user + // is actually trying to import an item that is declared in the same scope + // + // e.g + // use self::submodule; + // pub mod submodule; + // + // In this case we continue as if we resolved the import and let + // check_for_conflicts_between_imports_and_items handle the conflict + Some(_) => match (importing_module.def_id(), module.def_id()) { + (Some(id1), Some(id2)) if id1 == id2 => (Failed(None), false), + _ => (Indeterminate, false) + }, + } + } + fn resolve_single_import(&mut self, module_: Module<'b>, target_module: Module<'b>, @@ -420,253 +476,86 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { }; // We need to resolve both namespaces for this to succeed. - - let mut value_result = UnknownResult; - let mut type_result = UnknownResult; - let mut lev_suggestion = "".to_owned(); - - // Search for direct children of the containing module. - build_reduced_graph::populate_module_if_necessary(self.resolver, &target_module); - - match target_module.children.borrow().get(&source) { - None => { - let names = target_module.children.borrow(); - if let Some(name) = find_best_match_for_name(names.keys(), - &source.as_str(), - None) { - lev_suggestion = format!(". Did you mean to use `{}`?", name); - } + let (value_result, value_used_reexport) = + self.resolve_name_in_module(&target_module, source, ValueNS, module_); + let (type_result, type_used_reexport) = + self.resolve_name_in_module(&target_module, source, TypeNS, module_); + + match (&value_result, &type_result) { + (&Success((_, ref name_binding)), _) if !value_used_reexport && + directive.is_public && + !name_binding.is_public() => { + let msg = format!("`{}` is private, and cannot be reexported", source); + let note_msg = format!("Consider marking `{}` as `pub` in the imported module", + source); + struct_span_err!(self.resolver.session, directive.span, E0364, "{}", &msg) + .span_note(directive.span, ¬e_msg) + .emit(); } - Some(ref child_name_bindings) => { - // pub_err makes sure we don't give the same error twice. - let mut pub_err = false; - if child_name_bindings.value_ns.defined() { - debug!("(resolving single import) found value binding"); - value_result = BoundResult(target_module, - child_name_bindings.value_ns.clone()); - if directive.is_public && !child_name_bindings.value_ns.is_public() { - let msg = format!("`{}` is private, and cannot be reexported", source); - let note_msg = format!("Consider marking `{}` as `pub` in the imported \ - module", - source); - struct_span_err!(self.resolver.session, directive.span, E0364, "{}", &msg) - .span_note(directive.span, ¬e_msg) - .emit(); - pub_err = true; - } - if directive.is_public && child_name_bindings.value_ns. - defined_with(DefModifiers::PRIVATE_VARIANT) { - let msg = format!("variant `{}` is private, and cannot be reexported ( \ - error E0364), consider declaring its enum as `pub`", - source); - self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, - directive.id, - directive.span, - msg); - pub_err = true; - } - } - if child_name_bindings.type_ns.defined() { - debug!("(resolving single import) found type binding"); - type_result = BoundResult(target_module, - child_name_bindings.type_ns.clone()); - if !pub_err && directive.is_public && - !child_name_bindings.type_ns.is_public() { - let msg = format!("`{}` is private, and cannot be reexported", source); - let note_msg = format!("Consider declaring module `{}` as a `pub mod`", - source); - struct_span_err!(self.resolver.session, directive.span, E0365, "{}", &msg) - .span_note(directive.span, ¬e_msg) - .emit(); - } - if !pub_err && directive.is_public && child_name_bindings.type_ns. - defined_with(DefModifiers::PRIVATE_VARIANT) { - let msg = format!("variant `{}` is private, and cannot be reexported ( \ - error E0365), consider declaring its enum as `pub`", - source); - self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, - directive.id, - directive.span, - msg); - } - } - } - } - // Unless we managed to find a result in both namespaces (unlikely), - // search imports as well. - let mut value_used_reexport = false; - let mut type_used_reexport = false; - match (value_result.clone(), type_result.clone()) { - (BoundResult(..), BoundResult(..)) => {} // Continue. - _ => { - // If there is an unresolved glob at this point in the - // containing module, bail out. We don't know enough to be - // able to resolve this import. - - if target_module.pub_glob_count.get() > 0 { - debug!("(resolving single import) unresolved pub glob; bailing out"); - return ResolveResult::Indeterminate; + (_, &Success((_, ref name_binding))) if !type_used_reexport && + directive.is_public => { + if !name_binding.is_public() { + let msg = format!("`{}` is private, and cannot be reexported", source); + let note_msg = + format!("Consider declaring type or module `{}` with `pub`", source); + struct_span_err!(self.resolver.session, directive.span, E0365, "{}", &msg) + .span_note(directive.span, ¬e_msg) + .emit(); + } else if name_binding.defined_with(DefModifiers::PRIVATE_VARIANT) { + let msg = format!("variant `{}` is private, and cannot be reexported \ + (error E0364), consider declaring its enum as `pub`", + source); + self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, + directive.id, + directive.span, + msg); } + } - // Now search the exported imports within the containing module. - match target_module.import_resolutions.borrow().get(&source) { - None => { - debug!("(resolving single import) no import"); - // The containing module definitely doesn't have an - // exported import with the name in question. We can - // therefore accurately report that the names are - // unbound. - - if lev_suggestion.is_empty() { // skip if we already have a suggestion - let names = target_module.import_resolutions.borrow(); - if let Some(name) = find_best_match_for_name(names.keys(), - &source.as_str(), - None) { - lev_suggestion = - format!(". Did you mean to use the re-exported import `{}`?", - name); - } - } - - if value_result.is_unknown() { - value_result = UnboundResult; - } - if type_result.is_unknown() { - type_result = UnboundResult; - } - } - Some(import_resolution) if import_resolution.outstanding_references == 0 => { - - fn get_binding<'a>(this: &mut Resolver, - import_resolution: &ImportResolutionPerNamespace<'a>, - namespace: Namespace, - source: Name) - -> NamespaceResult<'a> { - - // Import resolutions must be declared with "pub" - // in order to be exported. - if !import_resolution[namespace].is_public { - return UnboundResult; - } - - match import_resolution[namespace].target.clone() { - None => { - return UnboundResult; - } - Some(Target { - target_module, - binding, - shadowable: _ - }) => { - debug!("(resolving single import) found import in ns {:?}", - namespace); - let id = import_resolution[namespace].id; - // track used imports and extern crates as well - this.used_imports.insert((id, namespace)); - this.record_import_use(id, source); - match target_module.def_id() { - Some(DefId{krate: kid, ..}) => { - this.used_crates.insert(kid); - } - _ => {} - } - return BoundResult(target_module, binding); - } - } - } - - // The name is an import which has been fully - // resolved. We can, therefore, just follow it. - if value_result.is_unknown() { - value_result = get_binding(self.resolver, - import_resolution, - ValueNS, - source); - value_used_reexport = import_resolution.value_ns.is_public; - } - if type_result.is_unknown() { - type_result = get_binding(self.resolver, - import_resolution, - TypeNS, - source); - type_used_reexport = import_resolution.type_ns.is_public; - } + _ => {} + } - } - Some(_) => { - // If target_module is the same module whose import we are resolving - // and there it has an unresolved import with the same name as `source`, - // then the user is actually trying to import an item that is declared - // in the same scope - // - // e.g - // use self::submodule; - // pub mod submodule; - // - // In this case we continue as if we resolved the import and let the - // check_for_conflicts_between_imports_and_items call below handle - // the conflict - match (module_.def_id(), target_module.def_id()) { - (Some(id1), Some(id2)) if id1 == id2 => { - if value_result.is_unknown() { - value_result = UnboundResult; - } - if type_result.is_unknown() { - type_result = UnboundResult; - } - } - _ => { - // The import is unresolved. Bail out. - debug!("(resolving single import) unresolved import; bailing out"); - return ResolveResult::Indeterminate; - } - } + let mut lev_suggestion = "".to_owned(); + match (&value_result, &type_result) { + (&Indeterminate, _) | (_, &Indeterminate) => return Indeterminate, + (&Failed(_), &Failed(_)) => { + let children = target_module.children.borrow(); + let names = children.keys().map(|&(ref name, _)| name); + if let Some(name) = find_best_match_for_name(names, &source.as_str(), None) { + lev_suggestion = format!(". Did you mean to use `{}`?", name); + } else { + let resolutions = target_module.import_resolutions.borrow(); + let names = resolutions.keys().map(|&(ref name, _)| name); + if let Some(name) = find_best_match_for_name(names, + &source.as_str(), + None) { + lev_suggestion = + format!(". Did you mean to use the re-exported import `{}`?", name); } } } + _ => (), } let mut value_used_public = false; let mut type_used_public = false; - // If we didn't find a result in the type namespace, search the - // external modules. - match type_result { - BoundResult(..) => {} - _ => { - match target_module.external_module_children.borrow_mut().get(&source) { - None => {} // Continue. - Some(module) => { - debug!("(resolving single import) found external module"); - // track the module as used. - match module.def_id() { - Some(DefId{krate: kid, ..}) => { - self.resolver.used_crates.insert(kid); - } - _ => {} - } - let name_binding = NameBinding::create_from_module(module); - type_result = BoundResult(target_module, name_binding); - type_used_public = true; - } - } - } - } - // We've successfully resolved the import. Write the results in. let mut import_resolutions = module_.import_resolutions.borrow_mut(); - let import_resolution = import_resolutions.get_mut(&target).unwrap(); { - let mut check_and_write_import = |namespace, result: &_, used_public: &mut bool| { + let mut check_and_write_import = |namespace, result, used_public: &mut bool| { + let result: &ResolveResult<(Module<'b>, NameBinding)> = result; + + let import_resolution = import_resolutions.get_mut(&(target, namespace)).unwrap(); let namespace_name = match namespace { TypeNS => "type", ValueNS => "value", }; match *result { - BoundResult(ref target_module, ref name_binding) => { + Success((ref target_module, ref name_binding)) => { debug!("(resolving single import) found {:?} target: {:?}", namespace_name, name_binding.def()); @@ -679,67 +568,68 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { directive.span, target); - import_resolution[namespace] = ImportResolution { - target: Some(Target::new(target_module, - name_binding.clone(), - directive.shadowable)), - id: directive.id, - is_public: directive.is_public - }; + import_resolution.target = Some(Target::new(target_module, + name_binding.clone(), + directive.shadowable)); + import_resolution.id = directive.id; + import_resolution.is_public = directive.is_public; - self.add_export(module_, target, &import_resolution[namespace]); + self.add_export(module_, target, &import_resolution); *used_public = name_binding.is_public(); } - UnboundResult => { + Failed(_) => { // Continue. } - UnknownResult => { + Indeterminate => { panic!("{:?} result should be known at this point", namespace_name); } } + + self.check_for_conflicts_between_imports_and_items(module_, + import_resolution, + directive.span, + (target, namespace)); }; check_and_write_import(ValueNS, &value_result, &mut value_used_public); check_and_write_import(TypeNS, &type_result, &mut type_used_public); } - self.check_for_conflicts_between_imports_and_items(module_, - import_resolution, - directive.span, - target); - - if value_result.is_unbound() && type_result.is_unbound() { + if let (&Failed(_), &Failed(_)) = (&value_result, &type_result) { let msg = format!("There is no `{}` in `{}`{}", source, module_to_string(&target_module), lev_suggestion); - return ResolveResult::Failed(Some((directive.span, msg))); + return Failed(Some((directive.span, msg))); } + let value_used_public = value_used_reexport || value_used_public; let type_used_public = type_used_reexport || type_used_public; - assert!(import_resolution.outstanding_references >= 1); - import_resolution.outstanding_references -= 1; - - // Record what this import resolves to for later uses in documentation, - // this may resolve to either a value or a type, but for documentation - // purposes it's good enough to just favor one over the other. - let value_def_and_priv = import_resolution.value_ns.target.as_ref().map(|target| { - let def = target.binding.def().unwrap(); - (def, - if value_used_public { - lp - } else { - DependsOn(def.def_id()) + let value_def_and_priv = { + let import_resolution_value = import_resolutions.get_mut(&(target, ValueNS)).unwrap(); + assert!(import_resolution_value.outstanding_references >= 1); + import_resolution_value.outstanding_references -= 1; + + // Record what this import resolves to for later uses in documentation, + // this may resolve to either a value or a type, but for documentation + // purposes it's good enough to just favor one over the other. + import_resolution_value.target.as_ref().map(|target| { + let def = target.binding.def().unwrap(); + let last_private = if value_used_public { lp } else { DependsOn(def.def_id()) }; + (def, last_private) }) - }); - let type_def_and_priv = import_resolution.type_ns.target.as_ref().map(|target| { - let def = target.binding.def().unwrap(); - (def, - if type_used_public { - lp - } else { - DependsOn(def.def_id()) + }; + + let type_def_and_priv = { + let import_resolution_type = import_resolutions.get_mut(&(target, TypeNS)).unwrap(); + assert!(import_resolution_type.outstanding_references >= 1); + import_resolution_type.outstanding_references -= 1; + + import_resolution_type.target.as_ref().map(|target| { + let def = target.binding.def().unwrap(); + let last_private = if type_used_public { lp } else { DependsOn(def.def_id()) }; + (def, last_private) }) - }); + }; let import_lp = LastImport { value_priv: value_def_and_priv.map(|(_, p)| p), @@ -766,7 +656,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { } debug!("(resolving single import) successfully resolved import"); - return ResolveResult::Success(()); + return Success(()); } // Resolves a glob import. Note that this function cannot fail; it either @@ -806,44 +696,41 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { "Cannot glob-import a module into itself.".into()))); } - for (name, target_import_resolution) in import_resolutions.iter() { + for (&(name, ns), target_import_resolution) in import_resolutions.iter() { debug!("(resolving glob import) writing module resolution {} into `{}`", - *name, + name, module_to_string(module_)); // Here we merge two import resolutions. let mut import_resolutions = module_.import_resolutions.borrow_mut(); - let mut dest_import_resolution = import_resolutions.entry(*name).or_insert_with(|| { - ImportResolutionPerNamespace::new(id, is_public) - }); - - for &ns in [TypeNS, ValueNS].iter() { - match target_import_resolution[ns].target { - Some(ref target) if target_import_resolution[ns].is_public => { - self.check_for_conflicting_import(&dest_import_resolution, - import_directive.span, - *name, - ns); - dest_import_resolution[ns] = ImportResolution { - id: id, is_public: is_public, target: Some(target.clone()) - }; - self.add_export(module_, *name, &dest_import_resolution[ns]); - } - _ => {} + let mut dest_import_resolution = + import_resolutions.entry((name, ns)) + .or_insert_with(|| ImportResolution::new(id, is_public)); + + match target_import_resolution.target { + Some(ref target) if target_import_resolution.is_public => { + self.check_for_conflicting_import(&dest_import_resolution, + import_directive.span, + name, + ns); + dest_import_resolution.id = id; + dest_import_resolution.is_public = is_public; + dest_import_resolution.target = Some(target.clone()); + self.add_export(module_, name, &dest_import_resolution); } + _ => {} } } // Add all children from the containing module. build_reduced_graph::populate_module_if_necessary(self.resolver, &target_module); - for (&name, name_bindings) in target_module.children.borrow().iter() { + for (&name, name_binding) in target_module.children.borrow().iter() { self.merge_import_resolution(module_, target_module, import_directive, name, - name_bindings.clone()); - + name_binding.clone()); } // Record the destination of this import @@ -864,14 +751,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { module_: Module<'b>, containing_module: Module<'b>, import_directive: &ImportDirective, - name: Name, - name_bindings: NameBindings<'b>) { + (name, ns): (Name, Namespace), + name_binding: NameBinding<'b>) { let id = import_directive.id; let is_public = import_directive.is_public; let mut import_resolutions = module_.import_resolutions.borrow_mut(); - let dest_import_resolution = import_resolutions.entry(name).or_insert_with(|| { - ImportResolutionPerNamespace::new(id, is_public) + let dest_import_resolution = import_resolutions.entry((name, ns)).or_insert_with(|| { + ImportResolution::new(id, is_public) }); debug!("(resolving glob import) writing resolution `{}` in `{}` to `{}`", @@ -880,61 +767,46 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { module_to_string(module_)); // Merge the child item into the import resolution. - // pub_err makes sure we don't give the same error twice. - let mut pub_err = false; - { - let mut merge_child_item = |namespace| { - if !pub_err && is_public && - name_bindings[namespace].defined_with(DefModifiers::PRIVATE_VARIANT) { - let msg = format!("variant `{}` is private, and cannot be reexported (error \ - E0364), consider declaring its enum as `pub`", name); - self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, - import_directive.id, - import_directive.span, - msg); - pub_err = true; - } + let modifier = DefModifiers::IMPORTABLE | DefModifiers::PUBLIC; + + if ns == TypeNS && is_public && name_binding.defined_with(DefModifiers::PRIVATE_VARIANT) { + let msg = format!("variant `{}` is private, and cannot be reexported (error \ + E0364), consider declaring its enum as `pub`", name); + self.resolver.session.add_lint(lint::builtin::PRIVATE_IN_PUBLIC, + import_directive.id, + import_directive.span, + msg); + } - let modifier = DefModifiers::IMPORTABLE | DefModifiers::PUBLIC; - if name_bindings[namespace].defined_with(modifier) { - let namespace_name = match namespace { - TypeNS => "type", - ValueNS => "value", - }; - debug!("(resolving glob import) ... for {} target", namespace_name); - if dest_import_resolution.shadowable(namespace) == Shadowable::Never { - let msg = format!("a {} named `{}` has already been imported in this \ - module", - namespace_name, - name); - span_err!(self.resolver.session, - import_directive.span, - E0251, - "{}", - msg); - } else { - dest_import_resolution[namespace] = ImportResolution { - target: Some(Target::new(containing_module, - name_bindings[namespace].clone(), - import_directive.shadowable)), - id: id, - is_public: is_public - }; - self.add_export(module_, name, &dest_import_resolution[namespace]); - } - } else { - // FIXME #30159: This is required for backwards compatability. - dest_import_resolution[namespace].is_public |= is_public; - } + if name_binding.defined_with(modifier) { + let namespace_name = match ns { + TypeNS => "type", + ValueNS => "value", }; - merge_child_item(ValueNS); - merge_child_item(TypeNS); + debug!("(resolving glob import) ... for {} target", namespace_name); + if dest_import_resolution.shadowable() == Shadowable::Never { + let msg = format!("a {} named `{}` has already been imported in this module", + namespace_name, + name); + span_err!(self.resolver.session, import_directive.span, E0251, "{}", msg); + } else { + let target = Target::new(containing_module, + name_binding.clone(), + import_directive.shadowable); + dest_import_resolution.target = Some(target); + dest_import_resolution.id = id; + dest_import_resolution.is_public = is_public; + self.add_export(module_, name, &dest_import_resolution); + } + } else { + // FIXME #30159: This is required for backwards compatability. + dest_import_resolution.is_public |= is_public; } self.check_for_conflicts_between_imports_and_items(module_, dest_import_resolution, import_directive.span, - name); + (name, ns)); } fn add_export(&mut self, module: Module<'b>, name: Name, resolution: &ImportResolution<'b>) { @@ -952,11 +824,11 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { /// Checks that imported names and items don't have the same name. fn check_for_conflicting_import(&mut self, - import_resolution: &ImportResolutionPerNamespace, + import_resolution: &ImportResolution, import_span: Span, name: Name, namespace: Namespace) { - let target = &import_resolution[namespace].target; + let target = &import_resolution.target; debug!("check_for_conflicting_import: {}; target exists: {}", name, target.is_some()); @@ -973,7 +845,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { } ValueNS => "value", }; - let use_id = import_resolution[namespace].id; + let use_id = import_resolution.id; let item = self.resolver.ast_map.expect_item(use_id); let mut err = struct_span_err!(self.resolver.session, import_span, @@ -1006,55 +878,53 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { /// Checks that imported names and items don't have the same name. fn check_for_conflicts_between_imports_and_items(&mut self, module: Module<'b>, - import: &ImportResolutionPerNamespace<'b>, + import: &ImportResolution<'b>, import_span: Span, - name: Name) { + (name, ns): (Name, Namespace)) { // First, check for conflicts between imports and `extern crate`s. - if module.external_module_children - .borrow() - .contains_key(&name) { - match import.type_ns.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[..]); + 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 => {} } - Some(_) | None => {} } } // Check for item conflicts. - let name_bindings = match module.children.borrow().get(&name) { + let name_binding = match module.get_child(name, ns) { None => { // There can't be any conflicts. return; } - Some(ref name_bindings) => (*name_bindings).clone(), + Some(name_binding) => name_binding, }; - match import.value_ns.target { - Some(ref target) if target.shadowable != Shadowable::Always => { - if let Some(ref value) = *name_bindings.value_ns.borrow() { + if ns == ValueNS { + match import.target { + Some(ref target) if target.shadowable != Shadowable::Always => { let mut err = struct_span_err!(self.resolver.session, import_span, E0255, "import `{}` conflicts with \ value in this module", name); - if let Some(span) = value.span { + if let Some(span) = name_binding.span { err.span_note(span, "conflicting value here"); } err.emit(); } + Some(_) | None => {} } - Some(_) | None => {} - } - - match import.type_ns.target { - Some(ref target) if target.shadowable != Shadowable::Always => { - if let Some(ref ty) = *name_bindings.type_ns.borrow() { - let (what, note) = match ty.module() { + } else { + match import.target { + Some(ref target) if target.shadowable != Shadowable::Always => { + let (what, note) = match name_binding.module() { Some(ref module) if module.is_normal() => ("existing submodule", "note conflicting module here"), Some(ref module) if module.is_trait() => @@ -1067,13 +937,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { "import `{}` conflicts with {}", name, what); - if let Some(span) = ty.span { + if let Some(span) = name_binding.span { err.span_note(span, note); } err.emit(); } + Some(_) | None => {} } - Some(_) | None => {} } } }