Refactor fields def_id and kind of Module into a single field def.

Change build_reduced_graph.rs so the fields def and module of NsDef are never both Some unless the NsDef represents a duplicate definition (see issue 26421).

5 files changed, 126 insertions, 184 deletions

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 802f9a0d40f..b70349bfe94 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -18,7 +18,6 @@ use resolve_imports::ImportDirective;
 use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport};
 use resolve_imports::ImportResolution;
 use Module;
-use ModuleKind::*;
 use Namespace::{TypeNS, ValueNS};
 use NameBindings;
 use {names_to_string, module_to_string};
@@ -395,8 +394,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     self.external_exports.insert(def_id);
                     let parent_link = ModuleParentLink(Rc::downgrade(parent), name);
                     let external_module = Rc::new(Module::new(parent_link,
-                                                              Some(def_id),
-                                                              NormalModuleKind,
+                                                              Some(DefMod(def_id)),
                                                               false,
                                                               true));
                     debug!("(build reduced graph for item) found extern `{}`",
@@ -436,13 +434,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 let name_bindings = self.add_child(name, parent, ForbidDuplicateModules, sp);
 
                 let parent_link = self.get_parent_link(parent, name);
-                let def_id = self.ast_map.local_def_id(item.id);
-                name_bindings.define_module(parent_link,
-                                            Some(def_id),
-                                            NormalModuleKind,
-                                            false,
-                                            is_public,
-                                            sp);
+                let def = DefMod(self.ast_map.local_def_id(item.id));
+                name_bindings.define_module(parent_link, Some(def), false, is_public, sp);
 
                 name_bindings.get_module()
             }
@@ -479,17 +472,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                    ForbidDuplicateTypesAndModules,
                                                    sp);
 
-                name_bindings.define_type(DefTy(self.ast_map.local_def_id(item.id), false),
-                                          sp,
-                                          modifiers);
-
                 let parent_link = self.get_parent_link(parent, name);
-                name_bindings.set_module_kind(parent_link,
-                                              Some(self.ast_map.local_def_id(item.id)),
-                                              TypeModuleKind,
-                                              false,
-                                              is_public,
-                                              sp);
+                let def = DefTy(self.ast_map.local_def_id(item.id), false);
+                name_bindings.define_module(parent_link, Some(def), false, is_public, sp);
                 parent.clone()
             }
 
@@ -499,17 +484,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                    ForbidDuplicateTypesAndModules,
                                                    sp);
 
-                name_bindings.define_type(DefTy(self.ast_map.local_def_id(item.id), true),
-                                          sp,
-                                          modifiers);
-
                 let parent_link = self.get_parent_link(parent, name);
-                name_bindings.set_module_kind(parent_link,
-                                              Some(self.ast_map.local_def_id(item.id)),
-                                              EnumModuleKind,
-                                              false,
-                                              is_public,
-                                              sp);
+                let def = DefTy(self.ast_map.local_def_id(item.id), true);
+                name_bindings.define_module(parent_link, Some(def), false, is_public, sp);
 
                 let module = name_bindings.get_module();
 
@@ -592,18 +569,14 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                    ForbidDuplicateTypesAndModules,
                                                    sp);
 
+                let def_id = self.ast_map.local_def_id(item.id);
+
                 // Add all the items within to a new module.
                 let parent_link = self.get_parent_link(parent, name);
-                name_bindings.define_module(parent_link,
-                                            Some(self.ast_map.local_def_id(item.id)),
-                                            TraitModuleKind,
-                                            false,
-                                            is_public,
-                                            sp);
+                let def = DefTrait(def_id);
+                name_bindings.define_module(parent_link, Some(def), false, is_public, sp);
                 let module_parent = name_bindings.get_module();
 
-                let def_id = self.ast_map.local_def_id(item.id);
-
                 // 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,
@@ -634,7 +607,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     self.trait_item_map.insert((trait_item.name, def_id), trait_item_def_id);
                 }
 
-                name_bindings.define_type(DefTrait(def_id), sp, modifiers);
                 parent.clone()
             }
         }
@@ -705,7 +677,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
             let new_module = Rc::new(Module::new(BlockParentLink(Rc::downgrade(parent), block_id),
                                                  None,
-                                                 AnonymousModuleKind,
                                                  false,
                                                  false));
             parent.anonymous_children.borrow_mut().insert(block_id, new_module.clone());
@@ -732,7 +703,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
             DefModifiers::empty()
         } | DefModifiers::IMPORTABLE;
         let is_exported = is_public &&
-                          match new_parent.def_id.get() {
+                          match new_parent.def_id() {
             None => true,
             Some(did) => self.external_exports.contains(&did),
         };
@@ -740,20 +711,14 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
             self.external_exports.insert(def.def_id());
         }
 
-        let kind = match def {
-            DefTy(_, true) => EnumModuleKind,
-            DefTy(_, false) | DefStruct(..) => TypeModuleKind,
-            _ => NormalModuleKind,
-        };
-
         match def {
-            DefMod(def_id) |
-            DefForeignMod(def_id) |
-            DefStruct(def_id) |
-            DefTy(def_id, _) => {
+            DefMod(_) |
+            DefForeignMod(_) |
+            DefStruct(_) |
+            DefTy(..) => {
                 if let Some(module_def) = child_name_bindings.type_ns.module() {
                     debug!("(building reduced graph for external crate) already created module");
-                    module_def.def_id.set(Some(def_id));
+                    module_def.def.set(Some(def));
                 } else {
                     debug!("(building reduced graph for external crate) building module {} {}",
                            final_ident,
@@ -761,8 +726,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     let parent_link = self.get_parent_link(new_parent, name);
 
                     child_name_bindings.define_module(parent_link,
-                                                      Some(def_id),
-                                                      kind,
+                                                      Some(def),
                                                       true,
                                                       is_public,
                                                       DUMMY_SP);
@@ -806,7 +770,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                      (def.modifiers & DefModifiers::IMPORTABLE),
                     None => modifiers,
                 };
-                if new_parent.kind.get() != NormalModuleKind {
+                if !new_parent.is_normal() {
                     modifiers = modifiers & !DefModifiers::IMPORTABLE;
                 }
                 child_name_bindings.define_value(def, DUMMY_SP, modifiers);
@@ -835,33 +799,33 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                     }
                 }
 
-                child_name_bindings.define_type(def, DUMMY_SP, modifiers);
-
                 // Define a module if necessary.
                 let parent_link = self.get_parent_link(new_parent, name);
-                child_name_bindings.set_module_kind(parent_link,
-                                                    Some(def_id),
-                                                    TraitModuleKind,
-                                                    true,
-                                                    is_public,
-                                                    DUMMY_SP)
+                child_name_bindings.define_module(parent_link,
+                                                  Some(def),
+                                                  true,
+                                                  is_public,
+                                                  DUMMY_SP)
             }
             DefTy(..) | DefAssociatedTy(..) => {
                 debug!("(building reduced graph for external crate) building type {}",
                        final_ident);
 
-                let modifiers = match new_parent.kind.get() {
-                    NormalModuleKind => modifiers,
+                let modifiers = match new_parent.is_normal() {
+                    true => modifiers,
                     _ => modifiers & !DefModifiers::IMPORTABLE,
                 };
 
-                child_name_bindings.define_type(def, DUMMY_SP, modifiers);
+                if let DefTy(..) = def {
+                    child_name_bindings.type_ns.set_modifiers(modifiers);
+                } else {
+                    child_name_bindings.define_type(def, DUMMY_SP, modifiers);
+                }
             }
             DefStruct(def_id) => {
                 debug!("(building reduced graph for external crate) building type and value for \
                         {}",
                        final_ident);
-                child_name_bindings.define_type(def, DUMMY_SP, modifiers);
                 let fields = csearch::get_struct_field_names(&self.session.cstore, def_id);
 
                 if fields.is_empty() {
@@ -937,7 +901,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
         debug!("(populating external module) attempting to populate {}",
                module_to_string(&**module));
 
-        let def_id = match module.def_id.get() {
+        let def_id = match module.def_id() {
             None => {
                 debug!("(populating external module) ... no def ID!");
                 return;
@@ -971,8 +935,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     /// crate.
     fn build_reduced_graph_for_external_crate(&mut self, root: &Rc<Module>) {
         csearch::each_top_level_item_of_crate(&self.session.cstore,
-                                              root.def_id
-                                                  .get()
+                                              root.def_id()
                                                   .unwrap()
                                                   .krate,
                                               |def_like, name, visibility| {
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 3f6f66cad7a..18e2b66d3fb 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -49,7 +49,6 @@ use self::AssocItemResolveResult::*;
 use self::NameSearchType::*;
 use self::BareIdentifierPatternResolution::*;
 use self::ParentLink::*;
-use self::ModuleKind::*;
 use self::FallbackChecks::*;
 
 use rustc::front::map as hir_map;
@@ -759,21 +758,10 @@ enum ParentLink {
     BlockParentLink(Weak<Module>, NodeId),
 }
 
-/// The type of module this is.
-#[derive(Copy, Clone, PartialEq, Debug)]
-enum ModuleKind {
-    NormalModuleKind,
-    TraitModuleKind,
-    EnumModuleKind,
-    TypeModuleKind,
-    AnonymousModuleKind,
-}
-
 /// One node in the tree of modules.
 pub struct Module {
     parent_link: ParentLink,
-    def_id: Cell<Option<DefId>>,
-    kind: Cell<ModuleKind>,
+    def: Cell<Option<Def>>,
     is_public: bool,
 
     children: RefCell<HashMap<Name, NameBindings>>,
@@ -822,15 +810,13 @@ pub struct Module {
 
 impl Module {
     fn new(parent_link: ParentLink,
-           def_id: Option<DefId>,
-           kind: ModuleKind,
+           def: Option<Def>,
            external: bool,
            is_public: bool)
            -> Module {
         Module {
             parent_link: parent_link,
-            def_id: Cell::new(def_id),
-            kind: Cell::new(kind),
+            def: Cell::new(def),
             is_public: is_public,
             children: RefCell::new(HashMap::new()),
             imports: RefCell::new(Vec::new()),
@@ -845,6 +831,24 @@ impl Module {
         }
     }
 
+    fn def_id(&self) -> Option<DefId> {
+        self.def.get().as_ref().map(Def::def_id)
+    }
+
+    fn is_normal(&self) -> bool {
+        match self.def.get() {
+            Some(DefMod(_)) | Some(DefForeignMod(_)) => true,
+            _ => false,
+        }
+    }
+
+    fn is_trait(&self) -> bool {
+        match self.def.get() {
+            Some(DefTrait(_)) => true,
+            _ => false,
+        }
+    }
+
     fn all_imports_resolved(&self) -> bool {
         if self.imports.borrow_state() == ::std::cell::BorrowState::Writing {
             // it is currently being resolved ! so nope
@@ -882,9 +886,8 @@ impl Module {
 impl fmt::Debug for Module {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f,
-               "{:?}, kind: {:?}, {}",
-               self.def_id,
-               self.kind,
+               "{:?}, {}",
+               self.def,
                if self.is_public {
                    "public"
                } else {
@@ -902,7 +905,9 @@ bitflags! {
 }
 
 // Records a possibly-private definition.
-#[derive(Clone,Debug)]
+// FIXME once #21546 is resolved, the def and module fields will never both be Some,
+// so they can be refactored into something like Result<Def, Rc<Module>>.
+#[derive(Debug)]
 struct NsDef {
     modifiers: DefModifiers, // see note in ImportResolution about how to use this
     def: Option<Def>,
@@ -911,10 +916,20 @@ struct NsDef {
 }
 
 impl NsDef {
+    fn create_from_module(module: Rc<Module>, span: Option<Span>) -> Self {
+        let modifiers = if module.is_public {
+            DefModifiers::PUBLIC
+        } else {
+            DefModifiers::empty()
+        } | DefModifiers::IMPORTABLE;
+
+        NsDef { modifiers: modifiers, def: None, module: Some(module), span: span }
+    }
+
     fn def(&self) -> Option<Def> {
         match (self.def, &self.module) {
             (def @ Some(_), _) => def,
-            (_, &Some(ref module)) => module.def_id.get().map(|def_id| DefMod(def_id)),
+            (_, &Some(ref module)) => module.def.get(),
             _ => panic!("NsDef has neither a Def nor a Module"),
         }
     }
@@ -930,17 +945,17 @@ impl NameBinding {
     }
 
     fn create_from_module(module: Rc<Module>) -> Self {
-        NameBinding(Rc::new(RefCell::new(Some(NsDef {
-            modifiers: DefModifiers::IMPORTABLE,
-            def: None,
-            module: Some(module),
-            span: None,
-        }))))
+        NameBinding(Rc::new(RefCell::new(Some(NsDef::create_from_module(module, None)))))
+    }
+
+    fn set(&self, ns_def: NsDef) {
+        *self.0.borrow_mut() = Some(ns_def);
     }
 
-    fn set(&self, modifiers: DefModifiers, def: Option<Def>, mod_: Option<Rc<Module>>, sp: Span) {
-        *self.0.borrow_mut() =
-            Some(NsDef { modifiers: modifiers, def: def, module: mod_, span: Some(sp) });
+    fn set_modifiers(&self, modifiers: DefModifiers) {
+        if let Some(ref mut ns_def) = *self.0.borrow_mut() {
+            ns_def.modifiers = modifiers
+        }
     }
 
     fn and_then<T, F: Fn(&NsDef) -> Option<T>>(&self, f: F) -> Option<T> {
@@ -1004,35 +1019,12 @@ impl NameBindings {
     /// Creates a new module in this set of name bindings.
     fn define_module(&self,
                      parent_link: ParentLink,
-                     def_id: Option<DefId>,
-                     kind: ModuleKind,
+                     def: Option<Def>,
                      external: bool,
                      is_public: bool,
                      sp: Span) {
-        // Merges the module with the existing type def or creates a new one.
-        let modifiers = if is_public {
-            DefModifiers::PUBLIC
-        } else {
-            DefModifiers::empty()
-        } | DefModifiers::IMPORTABLE;
-
-        let module_ = Rc::new(Module::new(parent_link, def_id, kind, external, is_public));
-        self.type_ns.set(modifiers, self.type_ns.def(), Some(module_), sp);
-    }
-
-    /// Sets the kind of the module, creating a new one if necessary.
-    fn set_module_kind(&self,
-                       parent_link: ParentLink,
-                       def_id: Option<DefId>,
-                       kind: ModuleKind,
-                       external: bool,
-                       is_public: bool,
-                       _sp: Span) {
-        if let Some(module) = self.type_ns.module() {
-            module.kind.set(kind)
-        } else {
-            self.define_module(parent_link, def_id, kind, external, is_public, _sp)
-        }
+        let module = Module::new(parent_link, def, external, is_public);
+        self.type_ns.set(NsDef::create_from_module(Rc::new(module), Some(sp)));
     }
 
     /// Records a type definition.
@@ -1041,13 +1033,19 @@ impl NameBindings {
                def,
                modifiers);
         // Merges the type with the existing type def or creates a new one.
-        self.type_ns.set(modifiers, Some(def), self.type_ns.module(), sp);
+        self.type_ns.set(NsDef {
+            modifiers: modifiers, def: Some(def), module: self.type_ns.module(), span: 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(modifiers, Some(def), None, sp);
+        debug!("defining value for def {:?} with modifiers {:?}",
+               def,
+               modifiers);
+        self.value_ns.set(NsDef {
+            modifiers: modifiers, def: Some(def), module: None, span: Some(sp)
+        });
     }
 
     /// Returns the module node if applicable.
@@ -1178,8 +1176,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
         let root_def_id = ast_map.local_def_id(CRATE_NODE_ID);
         graph_root.define_module(NoParentLink,
-                                 Some(root_def_id),
-                                 NormalModuleKind,
+                                 Some(DefMod(root_def_id)),
                                  false,
                                  true,
                                  crate_span);
@@ -1358,7 +1355,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     // so, whether there is a module within.
                     if let Some(module_def) = target.binding.module() {
                         // track extern crates for unused_extern_crate lint
-                        if let Some(did) = module_def.def_id.get() {
+                        if let Some(did) = module_def.def_id() {
                             self.used_crates.insert(did.krate);
                         }
 
@@ -1367,7 +1364,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                         // Keep track of the closest private module used
                         // when resolving this import chain.
                         if !used_proxy && !search_module.is_public {
-                            if let Some(did) = search_module.def_id.get() {
+                            if let Some(did) = search_module.def_id() {
                                 closest_private = LastMod(DependsOn(did));
                             }
                         }
@@ -1466,8 +1463,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             Success(PrefixFound(ref containing_module, index)) => {
                 search_module = containing_module.clone();
                 start_index = index;
-                last_private = LastMod(DependsOn(containing_module.def_id
-                                                                  .get()
+                last_private = LastMod(DependsOn(containing_module.def_id()
                                                                   .unwrap()));
             }
         }
@@ -1527,8 +1523,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     let id = import_resolution.id(namespace);
                     self.used_imports.insert((id, namespace));
                     self.record_import_use(id, name);
-                    if let Some(DefId{krate: kid, ..}) = target.target_module.def_id.get() {
-                        self.used_crates.insert(kid);
+                    if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
+                         self.used_crates.insert(kid);
                     }
                     return Success((target, false));
                 }
@@ -1558,19 +1554,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     return Failed(None);
                 }
                 ModuleParentLink(parent_module_node, _) => {
-                    match search_module.kind.get() {
-                        NormalModuleKind => {
-                            // We stop the search here.
-                            debug!("(resolving item in lexical scope) unresolved module: not \
-                                    searching through module parents");
+                    if search_module.is_normal() {
+                        // We stop the search here.
+                        debug!("(resolving item in lexical scope) unresolved module: not \
+                                searching through module parents");
                             return Failed(None);
-                        }
-                        TraitModuleKind |
-                        EnumModuleKind |
-                        TypeModuleKind |
-                        AnonymousModuleKind => {
-                            search_module = parent_module_node.upgrade().unwrap();
-                        }
+                    } else {
+                        search_module = parent_module_node.upgrade().unwrap();
                     }
                 }
                 BlockParentLink(ref parent_module_node, _) => {
@@ -1642,13 +1632,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 ModuleParentLink(new_module, _) |
                 BlockParentLink(new_module, _) => {
                     let new_module = new_module.upgrade().unwrap();
-                    match new_module.kind.get() {
-                        NormalModuleKind => return Some(new_module),
-                        TraitModuleKind |
-                        EnumModuleKind |
-                        TypeModuleKind |
-                        AnonymousModuleKind => module_ = new_module,
+                    if new_module.is_normal() {
+                        return Some(new_module);
                     }
+                    module_ = new_module;
                 }
             }
         }
@@ -1657,17 +1644,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     /// Returns the nearest normal module parent of the given module, or the
     /// module itself if it is a normal module.
     fn get_nearest_normal_module_parent_or_self(&mut self, module_: Rc<Module>) -> Rc<Module> {
-        match module_.kind.get() {
-            NormalModuleKind => return module_,
-            TraitModuleKind |
-            EnumModuleKind |
-            TypeModuleKind |
-            AnonymousModuleKind => {
-                match self.get_nearest_normal_module_parent(module_.clone()) {
-                    None => module_,
-                    Some(new_module) => new_module,
-                }
-            }
+        if module_.is_normal() {
+            return module_;
+        }
+        match self.get_nearest_normal_module_parent(module_.clone()) {
+            None => module_,
+            Some(new_module) => new_module,
         }
     }
 
@@ -1766,7 +1748,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                         let id = import_resolution.id(namespace);
                         self.used_imports.insert((id, namespace));
                         self.record_import_use(id, name);
-                        if let Some(DefId{krate: kid, ..}) = target.target_module.def_id.get() {
+                        if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
                             self.used_crates.insert(kid);
                         }
                         return Success((target, true));
@@ -3109,7 +3091,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             }
             _ => return None,
         };
-        if let Some(DefId{krate: kid, ..}) = containing_module.def_id.get() {
+        if let Some(DefId{krate: kid, ..}) = containing_module.def_id() {
             self.used_crates.insert(kid);
         }
         return Some(def);
@@ -3696,7 +3678,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     self.used_imports.insert((id, TypeNS));
                     let trait_name = self.get_trait_name(did);
                     self.record_import_use(id, trait_name);
-                    if let Some(DefId{krate: kid, ..}) = target.target_module.def_id.get() {
+                    if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
                         self.used_crates.insert(kid);
                     }
                 }
diff --git a/src/librustc_resolve/record_exports.rs b/src/librustc_resolve/record_exports.rs
index 02ab3bd0295..6e8d2ac4ca5 100644
--- a/src/librustc_resolve/record_exports.rs
+++ b/src/librustc_resolve/record_exports.rs
@@ -54,7 +54,7 @@ impl<'a, 'b, 'tcx> ExportRecorder<'a, 'b, 'tcx> {
         // If this isn't a local krate, then bail out. We don't need to record
         // exports for nonlocal crates.
 
-        match module_.def_id.get() {
+        match module_.def_id() {
             Some(def_id) if def_id.is_local() => {
                 // OK. Continue.
                 debug!("(recording exports for module subtree) recording exports for local \
@@ -98,7 +98,7 @@ impl<'a, 'b, 'tcx> ExportRecorder<'a, 'b, 'tcx> {
         let mut exports = Vec::new();
 
         self.add_exports_for_module(&mut exports, module_);
-        match module_.def_id.get() {
+        match module_.def_id() {
             Some(def_id) => {
                 let node_id = self.ast_map.as_local_node_id(def_id).unwrap();
                 self.export_map.insert(node_id, exports);
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index 150c6ec9657..14f8287a5f8 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -12,7 +12,6 @@ use self::ImportDirectiveSubclass::*;
 
 use DefModifiers;
 use Module;
-use ModuleKind;
 use Namespace::{self, TypeNS, ValueNS};
 use {NameBindings, NameBinding};
 use NamespaceResult::{BoundResult, UnboundResult, UnknownResult};
@@ -550,7 +549,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                     // 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.get() {
+                                    match target_module.def_id() {
                                         Some(DefId{krate: kid, ..}) => {
                                             this.used_crates.insert(kid);
                                         }
@@ -592,7 +591,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                         // 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.get(), target_module.def_id.get()) {
+                        match (module_.def_id(), target_module.def_id()) {
                             (Some(id1), Some(id2)) if id1 == id2 => {
                                 if value_result.is_unknown() {
                                     value_result = UnboundResult;
@@ -625,7 +624,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     Some(module) => {
                         debug!("(resolving single import) found external module");
                         // track the module as used.
-                        match module.def_id.get() {
+                        match module.def_id() {
                             Some(DefId{krate: kid, ..}) => {
                                 self.resolver.used_crates.insert(kid);
                             }
@@ -864,7 +863,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         }
 
         // Record the destination of this import
-        if let Some(did) = target_module.def_id.get() {
+        if let Some(did) = target_module.def_id() {
             self.resolver.def_map.borrow_mut().insert(id,
                                                       PathResolution {
                                                           base_def: DefMod(did),
@@ -954,10 +953,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                 let ns_word = match namespace {
                     TypeNS => {
                         match target.binding.module() {
-                            Some(ref module) if module.kind.get() ==
-                                                ModuleKind::NormalModuleKind => "module",
-                            Some(ref module) if module.kind.get() ==
-                                                ModuleKind::TraitModuleKind => "trait",
+                            Some(ref module) if module.is_normal() => "module",
+                            Some(ref module) if module.is_trait() => "trait",
                             _ => "type",
                         }
                     }
@@ -1043,9 +1040,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             Some(ref target) if target.shadowable != Shadowable::Always => {
                 if let Some(ref ty) = *name_bindings.type_ns.borrow() {
                     let (what, note) = match ty.module.clone() {
-                        Some(ref module) if module.kind.get() == ModuleKind::NormalModuleKind =>
+                        Some(ref module) if module.is_normal() =>
                             ("existing submodule", "note conflicting module here"),
-                        Some(ref module) if module.kind.get() == ModuleKind::TraitModuleKind =>
+                        Some(ref module) if module.is_trait() =>
                             ("trait in this module", "note conflicting trait here"),
                         _ => ("type in this module", "note conflicting type here"),
                     };
diff --git a/src/test/compile-fail/enum-and-module-in-same-scope.rs b/src/test/compile-fail/enum-and-module-in-same-scope.rs
index f3d8fcf31d7..67969616ca3 100644
--- a/src/test/compile-fail/enum-and-module-in-same-scope.rs
+++ b/src/test/compile-fail/enum-and-module-in-same-scope.rs
@@ -13,7 +13,7 @@ mod Foo {
 }
 
 enum Foo {  //~ ERROR duplicate definition of type or module `Foo`
-    X //~ ERROR duplicate definition of value `X`
+    X
 }
 
 fn main() {}