From 118c93ba5e3be48c91fff1840fdc7bbe942c5f00 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Thu, 14 Jan 2016 01:42:45 +0000
Subject: [PATCH 1/4] Refactor away NameBindings, NsDef,
 ImportResolutionPerNamespace, DuplicateCheckingMode, and NamespaceDefinition.

---
 src/librustc_resolve/build_reduced_graph.rs | 331 ++++------
 src/librustc_resolve/lib.rs                 | 300 +++------
 src/librustc_resolve/resolve_imports.rs     | 650 +++++++++-----------
 3 files changed, 460 insertions(+), 821 deletions(-)

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 22c18c9d6a4..f6d63246117 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);
+    /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined.
+    fn try_define<T>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T)
+        where T: ToNameBinding<'b>
+    {
+        parent.try_define_child(name, ns, def.to_name_binding());
+    }
 
-        // 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));
-
-                if let Some(sp) = child[ns].span() {
-                    let note = format!("first definition of {} `{}` here", ns_str, name);
-                    err.span_note(sp, &note);
-                }
-                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<T: ToNameBinding<'b>>(&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, &note);
             }
+            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()
@@ -406,25 +361,21 @@ 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)
+                let ctor_id = if struct_def.is_struct() {
+                    None
                 } else {
-                    (ForbidDuplicateTypesAndValues, Some(struct_def.id()))
+                    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);
+                    let def = Def::Struct(self.ast_map.local_def_id(cid));
+                    self.define(parent, name, ValueNS, (def, sp, modifiers));
                 }
 
                 // Record the def ID and fields of this struct.
@@ -447,48 +398,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);
+                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),
+                    };
 
-                    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);
-                        }
-                    }
+                    let modifiers = DefModifiers::PUBLIC; // NB: not DefModifiers::IMPORTABLE
+                    self.define(&module_parent, item.name, ns, (def, item.span, modifiers));
 
-                    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);
+                    self.trait_item_map.insert((item.name, def_id), item_def_id);
                 }
 
                 parent
@@ -512,13 +442,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 +460,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 +469,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 +492,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 +499,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,
@@ -592,37 +522,33 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
             false
         };
 
+        // Define a module if necessary.
         match def {
             Def::Mod(_) |
             Def::ForeignMod(_) |
-            Def::Struct(..) |
+            Def::Trait(..) |
             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);
-                }
+                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::Mod(_) | Def::ForeignMod(_) | Def::Enum(..) | Def::TyAlias(..) => {}
             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 +561,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 {}",
@@ -669,28 +585,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                         self.external_exports.insert(trait_item_def.def_id());
                     }
                 }
-
-                // 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);
             }
-            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
@@ -699,10 +598,10 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 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.
@@ -737,14 +636,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 +720,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;
+                for &ns in [TypeNS, ValueNS].iter() {
+                    let mut resolution = import_resolutions.entry((target, ns)).or_insert(
+                        ImportResolution::new(id, is_public)
+                    );
 
-                        // 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 => {}
+                    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 4b62e65bb0f..90a913f32d5 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<T> {
     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<Option<Def>>,
     is_public: bool,
 
-    children: RefCell<HashMap<Name, NameBindings<'a>>>,
+    children: RefCell<HashMap<(Name, Namespace), NameBinding<'a>>>,
     imports: RefCell<Vec<ImportDirective>>,
 
     // The external module children of this node that were declared with
@@ -859,7 +826,7 @@ pub struct ModuleS<'a> {
     anonymous_children: RefCell<NodeMap<Module<'a>>>,
 
     // The status of resolving each import in this module.
-    import_resolutions: RefCell<HashMap<Name, ImportResolutionPerNamespace<'a>>>,
+    import_resolutions: RefCell<HashMap<(Name, Namespace), ImportResolution<'a>>>,
 
     // The number of unresolved globs that this module exports.
     glob_count: Cell<usize>,
@@ -900,6 +867,17 @@ impl<'a> ModuleS<'a> {
         }
     }
 
+    fn get_child(&self, name: Name, ns: Namespace) -> Option<NameBinding<'a>> {
+        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<DefId> {
         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<Span>,
 }
 
-#[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<Span>) -> 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<Span>) -> 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<Module<'a>> {
@@ -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<RefCell<Option<NsDef<'a>>>>);
-
-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<Option<NsDef<'a>>> {
-        self.0.borrow()
-    }
-
-    // Lifted versions of the NsDef methods and fields
-    fn def(&self) -> Option<Def> {
-        self.borrow().as_ref().and_then(NsDef::def)
-    }
-    fn module(&self) -> Option<Module<'a>> {
-        self.borrow().as_ref().and_then(NsDef::module)
-    }
-    fn span(&self) -> Option<Span> {
-        self.borrow().as_ref().and_then(|def| def.span)
-    }
-    fn modifiers(&self) -> Option<DefModifiers> {
-        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<Namespace> 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<Name, PrimTy>,
@@ -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));
                     }
@@ -3719,8 +3580,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             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() {
+                for (_, name_binding) in search_module.children.borrow().iter() {
+                    let def = match name_binding.def() {
                         Some(def) => def,
                         None => continue,
                     };
@@ -3735,10 +3596,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             }
 
             // Look for imports.
-            for (_, import) in search_module.import_resolutions.borrow().iter() {
-                let target = match import.type_ns.target {
-                    None => continue,
-                    Some(ref target) => target,
+            for (&(_, ns), import) in search_module.import_resolutions.borrow().iter() {
+                let target = match (ns, &import.target) {
+                    (TypeNS, &Some(ref target)) => target.clone(),
+                    _ => continue,
                 };
                 let did = match target.binding.def() {
                     Some(Def::Trait(trait_def_id)) => trait_def_id,
@@ -3746,7 +3607,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);
@@ -3811,36 +3672,19 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
         debug!("Children:");
         build_reduced_graph::populate_module_if_necessary(self, &module_);
-        for (&name, _) in module_.children.borrow().iter() {
-            debug!("* {}", name);
+        for (&(name, ns), _) in module_.children.borrow().iter() {
+            if ns == TypeNS || module_.get_child(name, TypeNS).is_none() {
+                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);
+        for (&(name, ns), import_resolution) in import_resolutions.iter() {
+            let ns_str = match ns { ValueNS => "value", TypeNS => "type" };
+            // FIXME #4954
+            let repr = match import_resolution.target { None => "", Some(_) => "?" };
+            debug!("* {} {}: {}", ns_str, name, repr);
         }
     }
 }
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index 364218d8413..d4981ea4180 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<Namespace> 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<Namespace> 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,55 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         return resolution_result;
     }
 
+    fn resolve_imported_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>)> {
+        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),
+
+            // 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);
+                }
+
+                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))
+                } else {
+                    Failed(None)
+                }
+            }
+
+            // 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 the
+            // check_for_conflicts_between_imports_and_items call below handle the conflict
+            Some(_) => match (importing_module.def_id(), module.def_id()) {
+                (Some(id1), Some(id2)) if id1 == id2 => Failed(None),
+                _ => Indeterminate,
+            },
+        }
+    }
+
     fn resolve_single_import(&mut self,
                              module_: Module<'b>,
                              target_module: Module<'b>,
@@ -420,211 +451,99 @@ 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 value_result = Indeterminate;
+        let mut type_result = Indeterminate;
         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);
-                }
+        // pub_err makes sure we don't give the same error twice.
+        let mut pub_err = false;
+
+        if let Some(name_binding) = target_module.get_child(source, ValueNS) {
+            debug!("(resolving single import) found value binding");
+            value_result = Success((target_module, name_binding.clone()));
+            if 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, &note_msg)
+                    .emit();
+                pub_err = true;
             }
-            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, &note_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, &note_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);
-                    }
+        }
+
+        if let Some(name_binding) = target_module.get_child(source, TypeNS) {
+            debug!("(resolving single import) found type binding");
+            type_result = Success((target_module, name_binding.clone()));
+            if !pub_err && 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, &note_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);
                 }
             }
         }
 
-        // Unless we managed to find a result in both namespaces (unlikely),
-        // search imports as well.
+        if let (&Indeterminate, &Indeterminate) = (&value_result, &type_result) {
+            let names = target_module.children.borrow();
+            let names = names.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);
+            }
+        }
+
+        match (&value_result, &type_result) {
+            // 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.
+            (&Indeterminate, _) | (_, &Indeterminate) if target_module.pub_glob_count.get() > 0 =>
+                return Indeterminate,
+            _ => ()
+        }
+
         let mut value_used_reexport = false;
+        if let Indeterminate = value_result {
+            value_result =
+                self.resolve_imported_name_in_module(&target_module, source, ValueNS, module_);
+            value_used_reexport = match value_result { Success(_) => true, _ => 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 let Indeterminate = type_result {
+            type_result =
+                self.resolve_imported_name_in_module(&target_module, source, TypeNS, module_);
+            type_used_reexport = match type_result { Success(_) => true, _ => false };
+        }
 
-                if target_module.pub_glob_count.get() > 0 {
-                    debug!("(resolving single import) unresolved pub glob; bailing out");
-                    return ResolveResult::Indeterminate;
-                }
-
-                // 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;
-                            }
-                        }
+        match (&value_result, &type_result) {
+            (&Indeterminate, _) | (_, &Indeterminate) => return Indeterminate,
+            (&Failed(_), &Failed(_)) => {
+                if lev_suggestion.is_empty() {  // skip if we already have a suggestion
+                    let names = target_module.import_resolutions.borrow();
+                    let names = names.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;
@@ -633,9 +552,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         // If we didn't find a result in the type namespace, search the
         // external modules.
         match type_result {
-            BoundResult(..) => {}
+            Success(..) => {}
             _ => {
-                match target_module.external_module_children.borrow_mut().get(&source) {
+                match target_module.external_module_children.borrow_mut().get(&source).cloned() {
                     None => {} // Continue.
                     Some(module) => {
                         debug!("(resolving single import) found external module");
@@ -646,8 +565,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                             }
                             _ => {}
                         }
-                        let name_binding = NameBinding::create_from_module(module);
-                        type_result = BoundResult(target_module, name_binding);
+                        let name_binding = NameBinding::create_from_module(module, None);
+                        type_result = Success((target_module, name_binding));
                         type_used_public = true;
                     }
                 }
@@ -656,17 +575,19 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
         // 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,35 +600,34 @@ 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);
@@ -716,30 +636,40 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         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;
+        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.
-        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())
+            // 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();
+                (def,
+                 if value_used_public {
+                    lp
+                } else {
+                    DependsOn(def.def_id())
+                })
             })
-        });
-        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();
+                (def,
+                if type_used_public {
+                    lp
+                } else {
+                    DependsOn(def.def_id())
+                })
             })
-        });
+        };
 
         let import_lp = LastImport {
             value_priv: value_def_and_priv.map(|(_, p)| p),
@@ -806,44 +736,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)
-            });
+            let mut dest_import_resolution =
+                import_resolutions.entry((name, ns))
+                                  .or_insert_with(|| ImportResolution::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]);
-                    }
-                    _ => {}
+            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 +791,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 +807,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;
 
-                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 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);
+        }
+
+        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 +864,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 +885,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 +918,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 let TypeNS = ns {
+            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 let ValueNS = ns {
+            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 +977,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 => {}
         }
     }
 }

From e13a0450d34572bc564b3ee1d275a01f403b4186 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Fri, 22 Jan 2016 03:00:29 +0000
Subject: [PATCH 2/4] Clean up resolve_single_import

---
 src/librustc_resolve/resolve_imports.rs | 174 +++++++++---------------
 1 file changed, 67 insertions(+), 107 deletions(-)

diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index d4981ea4180..4694f521884 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -375,22 +375,47 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         return resolution_result;
     }
 
-    fn resolve_imported_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>)> {
+    /// 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 let TypeNS = ns {
+            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),
+            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);
+                    return (Failed(None), false);
                 }
 
                 let target = resolution.target.clone();
@@ -401,9 +426,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     if let Some(DefId { krate, .. }) = target_module.def_id() {
                         self.resolver.used_crates.insert(krate);
                     }
-                    Success((target_module, binding))
+                    (Success((target_module, binding)), true)
                 } else {
-                    Failed(None)
+                    (Failed(None), false)
                 }
             }
 
@@ -415,11 +440,11 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             // 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
+            // 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),
-                _ => Indeterminate,
+                (Some(id1), Some(id2)) if id1 == id2 => (Failed(None), false),
+                _ => (Indeterminate, false)
             },
         }
     }
@@ -451,34 +476,25 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         };
 
         // We need to resolve both namespaces for this to succeed.
-        let mut value_result = Indeterminate;
-        let mut type_result = Indeterminate;
-        let mut lev_suggestion = "".to_owned();
+        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_);
 
-        // Search for direct children of the containing module.
-        build_reduced_graph::populate_module_if_necessary(self.resolver, &target_module);
-
-        // pub_err makes sure we don't give the same error twice.
-        let mut pub_err = false;
-
-        if let Some(name_binding) = target_module.get_child(source, ValueNS) {
-            debug!("(resolving single import) found value binding");
-            value_result = Success((target_module, name_binding.clone()));
-            if directive.is_public && !name_binding.is_public() {
+        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, &note_msg)
                     .emit();
-                pub_err = true;
             }
-        }
 
-        if let Some(name_binding) = target_module.get_child(source, TypeNS) {
-            debug!("(resolving single import) found type binding");
-            type_result = Success((target_module, name_binding.clone()));
-            if !pub_err && directive.is_public {
+            (_, &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 =
@@ -496,50 +512,26 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                                    msg);
                 }
             }
+
+            _ => {}
         }
 
-        if let (&Indeterminate, &Indeterminate) = (&value_result, &type_result) {
-            let names = target_module.children.borrow();
-            let names = names.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);
-            }
-        }
-
-        match (&value_result, &type_result) {
-            // 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.
-            (&Indeterminate, _) | (_, &Indeterminate) if target_module.pub_glob_count.get() > 0 =>
-                return Indeterminate,
-            _ => ()
-        }
-
-        let mut value_used_reexport = false;
-        if let Indeterminate = value_result {
-            value_result =
-                self.resolve_imported_name_in_module(&target_module, source, ValueNS, module_);
-            value_used_reexport = match value_result { Success(_) => true, _ => false };
-        }
-
-        let mut type_used_reexport = false;
-        if let Indeterminate = type_result {
-            type_result =
-                self.resolve_imported_name_in_module(&target_module, source, TypeNS, module_);
-            type_used_reexport = match type_result { Success(_) => true, _ => false };
-        }
-
+        let mut lev_suggestion = "".to_owned();
         match (&value_result, &type_result) {
             (&Indeterminate, _) | (_, &Indeterminate) => return Indeterminate,
             (&Failed(_), &Failed(_)) => {
-                if lev_suggestion.is_empty() {  // skip if we already have a suggestion
-                    let names = target_module.import_resolutions.borrow();
-                    let names = names.keys().map(|&(ref name, _)| name);
+                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);
+                            format!(". Did you mean to use the re-exported import `{}`?", name);
                     }
                 }
             }
@@ -549,30 +541,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         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 {
-            Success(..) => {}
-            _ => {
-                match target_module.external_module_children.borrow_mut().get(&source).cloned() {
-                    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, None);
-                        type_result = Success((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();
 
@@ -621,7 +589,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                                                    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);
@@ -631,8 +598,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             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;
 
@@ -646,12 +614,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             // 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();
-                (def,
-                 if value_used_public {
-                    lp
-                } else {
-                    DependsOn(def.def_id())
-                })
+                let last_private = if value_used_public { lp } else { DependsOn(def.def_id()) };
+                (def, last_private)
             })
         };
 
@@ -662,12 +626,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
             import_resolution_type.target.as_ref().map(|target| {
                 let def = target.binding.def().unwrap();
-                (def,
-                if type_used_public {
-                    lp
-                } else {
-                    DependsOn(def.def_id())
-                })
+                let last_private = if type_used_public { lp } else { DependsOn(def.def_id()) };
+                (def, last_private)
             })
         };
 
@@ -696,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

From 1ca9f03eadf99446f3c208dd1b4cb891f9f90ca7 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Thu, 28 Jan 2016 03:03:40 +0000
Subject: [PATCH 3/4] Nits and other local improvements in resolve

---
 src/librustc_resolve/build_reduced_graph.rs | 39 ++++++---------------
 src/librustc_resolve/lib.rs                 | 28 +++++++--------
 src/librustc_resolve/resolve_imports.rs     |  6 ++--
 3 files changed, 25 insertions(+), 48 deletions(-)

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index f6d63246117..a1d866fc48b 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -360,21 +360,14 @@ 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 ctor_id = if struct_def.is_struct() {
-                    None
-                } else {
-                    Some(struct_def.id())
-                };
-
                 // Define a name in the type namespace.
                 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 {
-                    let def = Def::Struct(self.ast_map.local_def_id(cid));
+                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));
                 }
 
@@ -516,19 +509,9 @@ 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
-        };
 
-        // Define a module if necessary.
         match def {
-            Def::Mod(_) |
-            Def::ForeignMod(_) |
-            Def::Trait(..) |
-            Def::Enum(..) |
-            Def::TyAlias(..) if !is_struct_ctor => {
+            Def::Mod(_) | Def::ForeignMod(_) | Def::Enum(..) | Def::TyAlias(..) => {
                 debug!("(building reduced graph for external crate) building module {} {}",
                        final_ident,
                        is_public);
@@ -536,11 +519,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 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::Enum(..) | Def::TyAlias(..) => {}
             Def::Variant(_, variant_id) => {
                 debug!("(building reduced graph for external crate) building variant {}",
                        final_ident);
@@ -585,16 +563,18 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                         self.external_exports.insert(trait_item_def.def_id());
                     }
                 }
+
+                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));
             }
             Def::AssociatedTy(..) => {
                 debug!("(building reduced graph for external crate) building type {}",
                        final_ident);
                 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);
@@ -608,6 +588,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 let fields = self.session.cstore.struct_field_names(def_id);
                 self.structs.insert(def_id, fields);
             }
+            Def::Struct(..) => {}
             Def::Local(..) |
             Def::PrimTy(..) |
             Def::TyParam(..) |
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 90a913f32d5..3a6bc678dfc 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -3579,27 +3579,23 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             // Look for trait children.
             build_reduced_graph::populate_module_if_necessary(self, &search_module);
 
-            {
-                for (_, name_binding) in search_module.children.borrow().iter() {
-                    let def = match name_binding.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 (&(_, ns), import) in search_module.import_resolutions.borrow().iter() {
-                let target = match (ns, &import.target) {
-                    (TypeNS, &Some(ref target)) => target.clone(),
-                    _ => continue,
+                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,
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index 4694f521884..07f6a0f9549 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -389,7 +389,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             return (Success((module, name_binding)), false);
         }
 
-        if let TypeNS = ns {
+        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() {
@@ -882,7 +882,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                                      import_span: Span,
                                                      (name, ns): (Name, Namespace)) {
         // First, check for conflicts between imports and `extern crate`s.
-        if let TypeNS = ns {
+        if ns == TypeNS {
             if module.external_module_children.borrow().contains_key(&name) {
                 match import.target {
                     Some(ref target) if target.shadowable != Shadowable::Always => {
@@ -905,7 +905,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             Some(name_binding) => name_binding,
         };
 
-        if let ValueNS = ns {
+        if ns == ValueNS {
             match import.target {
                 Some(ref target) if target.shadowable != Shadowable::Always => {
                     let mut err = struct_span_err!(self.resolver.session,

From afd42d21442db92492799f65e22e024e1e2b1011 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Thu, 28 Jan 2016 03:04:01 +0000
Subject: [PATCH 4/4] Remove resolve::dump_module

---
 src/librustc_resolve/lib.rs | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 3a6bc678dfc..6c78f98c0cb 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -3654,35 +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, ns), _) in module_.children.borrow().iter() {
-            if ns == TypeNS || module_.get_child(name, TypeNS).is_none() {
-                debug!("* {}", name);
-            }
-        }
-
-        debug!("Import resolutions:");
-        let import_resolutions = module_.import_resolutions.borrow();
-        for (&(name, ns), import_resolution) in import_resolutions.iter() {
-            let ns_str = match ns { ValueNS => "value", TypeNS => "type" };
-            // FIXME #4954
-            let repr = match import_resolution.target { None => "", Some(_) => "?" };
-            debug!("* {} {}: {}", ns_str, name, repr);
-        }
-    }
 }