Use `DefId`s instead of `NodeId`s for `pub(restricted)` visibilities.

4 files changed, 70 insertions, 76 deletions

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 09f438953ec..758b93aed67 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -229,7 +229,7 @@ impl<'a> Resolver<'a> {
                     ViewPathGlob(_) => {
                         let subclass = GlobImport {
                             is_prelude: is_prelude,
-                            max_vis: Cell::new(ty::Visibility::PrivateExternal),
+                            max_vis: Cell::new(ty::Visibility::Invisible),
                         };
                         self.add_import_directive(
                             module_path, subclass, view_path.span, item.id, vis, expansion,
@@ -265,16 +265,16 @@ impl<'a> Resolver<'a> {
             ItemKind::Mod(..) if item.ident == keywords::Invalid.ident() => {} // Crate root
 
             ItemKind::Mod(..) => {
-                let def = Def::Mod(self.definitions.local_def_id(item.id));
+                let def_id = self.definitions.local_def_id(item.id);
+                let module_kind = ModuleKind::Def(Def::Mod(def_id), ident.name);
                 let module = self.arenas.alloc_module(ModuleData {
                     no_implicit_prelude: parent.no_implicit_prelude || {
                         attr::contains_name(&item.attrs, "no_implicit_prelude")
                     },
-                    normal_ancestor_id: Some(item.id),
-                    ..ModuleData::new(Some(parent), ModuleKind::Def(def, ident.name))
+                    ..ModuleData::new(Some(parent), module_kind, def_id)
                 });
                 self.define(parent, ident, TypeNS, (module, vis, sp, expansion));
-                self.module_map.insert(item.id, module);
+                self.module_map.insert(def_id, module);
 
                 // Descend into the module.
                 self.current_module = module;
@@ -305,7 +305,8 @@ impl<'a> Resolver<'a> {
 
             ItemKind::Enum(ref enum_definition, _) => {
                 let def = Def::Enum(self.definitions.local_def_id(item.id));
-                let module = self.new_module(parent, ModuleKind::Def(def, ident.name), true);
+                let module_kind = ModuleKind::Def(def, ident.name);
+                let module = self.new_module(parent, module_kind, parent.normal_ancestor_id);
                 self.define(parent, ident, TypeNS, (module, vis, sp, expansion));
 
                 for variant in &(*enum_definition).variants {
@@ -355,8 +356,8 @@ impl<'a> Resolver<'a> {
                 let def_id = self.definitions.local_def_id(item.id);
 
                 // Add all the items within to a new module.
-                let module =
-                    self.new_module(parent, ModuleKind::Def(Def::Trait(def_id), ident.name), true);
+                let module_kind = ModuleKind::Def(Def::Trait(def_id), ident.name);
+                let module = self.new_module(parent, module_kind, parent.normal_ancestor_id);
                 self.define(parent, ident, TypeNS, (module, vis, sp, expansion));
                 self.current_module = module;
             }
@@ -404,15 +405,10 @@ impl<'a> Resolver<'a> {
     fn build_reduced_graph_for_block(&mut self, block: &Block) {
         let parent = self.current_module;
         if self.block_needs_anonymous_module(block) {
-            let block_id = block.id;
-
-            debug!("(building reduced graph for block) creating a new anonymous module for block \
-                    {}",
-                   block_id);
-
-            let new_module = self.new_module(parent, ModuleKind::Block(block_id), true);
-            self.module_map.insert(block_id, new_module);
-            self.current_module = new_module; // Descend into the block.
+            let module =
+                self.new_module(parent, ModuleKind::Block(block.id), parent.normal_ancestor_id);
+            self.block_map.insert(block.id, module);
+            self.current_module = module; // Descend into the block.
         }
     }
 
@@ -429,7 +425,7 @@ impl<'a> Resolver<'a> {
 
         match def {
             Def::Mod(..) | Def::Enum(..) => {
-                let module = self.new_module(parent, ModuleKind::Def(def, ident.name), false);
+                let module = self.new_module(parent, ModuleKind::Def(def, ident.name), def_id);
                 self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, Mark::root()));
             }
             Def::Variant(..) => {
@@ -446,7 +442,8 @@ impl<'a> Resolver<'a> {
                 self.define(parent, ident, ValueNS, (def, vis, DUMMY_SP, Mark::root()));
             }
             Def::Trait(..) => {
-                let module = self.new_module(parent, ModuleKind::Def(def, ident.name), false);
+                let module_kind = ModuleKind::Def(def, ident.name);
+                let module = self.new_module(parent, module_kind, parent.normal_ancestor_id);
                 self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, Mark::root()));
 
                 // If this is a trait, add all the trait item names to the trait info.
@@ -497,12 +494,10 @@ impl<'a> Resolver<'a> {
         let def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX };
         let name = self.session.cstore.crate_name(cnum);
         let macros_only = self.session.cstore.dep_kind(cnum).macros_only();
+        let module_kind = ModuleKind::Def(Def::Mod(def_id), name);
         let arenas = self.arenas;
         *self.extern_crate_roots.entry((cnum, macros_only)).or_insert_with(|| {
-            arenas.alloc_module(ModuleData {
-                populated: Cell::new(false),
-                ..ModuleData::new(None, ModuleKind::Def(Def::Mod(def_id), name))
-            })
+            arenas.alloc_module(ModuleData::new(None, module_kind, def_id))
         })
     }
 
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index dbeb8fe63b6..d7f237ea095 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -43,14 +43,13 @@ use rustc::middle::cstore::CrateLoader;
 use rustc::session::Session;
 use rustc::lint;
 use rustc::hir::def::*;
-use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefId};
+use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, LOCAL_CRATE, DefId};
 use rustc::ty;
 use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
 use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet};
 
 use syntax::ext::hygiene::{Mark, SyntaxContext};
-use syntax::ast::{self, FloatTy};
-use syntax::ast::{CRATE_NODE_ID, Name, NodeId, Ident, SpannedIdent, IntTy, UintTy};
+use syntax::ast::{self, Name, NodeId, Ident, SpannedIdent, FloatTy, IntTy, UintTy};
 use syntax::ext::base::SyntaxExtension;
 use syntax::ext::base::Determinacy::{Determined, Undetermined};
 use syntax::symbol::{Symbol, keywords};
@@ -771,8 +770,8 @@ pub struct ModuleData<'a> {
     parent: Option<Module<'a>>,
     kind: ModuleKind,
 
-    // The node id of the closest normal module (`mod`) ancestor (including this module).
-    normal_ancestor_id: Option<NodeId>,
+    // The def id of the closest normal module (`mod`) ancestor (including this module).
+    normal_ancestor_id: DefId,
 
     resolutions: RefCell<FxHashMap<(Ident, Namespace), &'a RefCell<NameResolution<'a>>>>,
     legacy_macro_resolutions: RefCell<Vec<(Mark, Ident, Span)>>,
@@ -798,11 +797,11 @@ pub struct ModuleData<'a> {
 pub type Module<'a> = &'a ModuleData<'a>;
 
 impl<'a> ModuleData<'a> {
-    fn new(parent: Option<Module<'a>>, kind: ModuleKind) -> Self {
+    fn new(parent: Option<Module<'a>>, kind: ModuleKind, normal_ancestor_id: DefId) -> Self {
         ModuleData {
             parent: parent,
             kind: kind,
-            normal_ancestor_id: None,
+            normal_ancestor_id: normal_ancestor_id,
             resolutions: RefCell::new(FxHashMap()),
             legacy_macro_resolutions: RefCell::new(Vec::new()),
             macro_resolutions: RefCell::new(Vec::new()),
@@ -811,7 +810,7 @@ impl<'a> ModuleData<'a> {
             glob_importers: RefCell::new(Vec::new()),
             globs: RefCell::new((Vec::new())),
             traits: RefCell::new(None),
-            populated: Cell::new(true),
+            populated: Cell::new(normal_ancestor_id.is_local()),
         }
     }
 
@@ -848,7 +847,7 @@ impl<'a> ModuleData<'a> {
     }
 
     fn is_local(&self) -> bool {
-        self.normal_ancestor_id.is_some()
+        self.normal_ancestor_id.is_local()
     }
 }
 
@@ -1063,7 +1062,7 @@ pub struct Resolver<'a> {
     pub export_map: ExportMap,
     pub trait_map: TraitMap,
 
-    // A map from nodes to modules, both normal (`mod`) modules and anonymous modules.
+    // A map from nodes to anonymous modules.
     // Anonymous modules are pseudo-modules that are implicitly created around items
     // contained within blocks.
     //
@@ -1077,7 +1076,8 @@ pub struct Resolver<'a> {
     //
     // There will be an anonymous module created around `g` with the ID of the
     // entry block for `f`.
-    module_map: NodeMap<Module<'a>>,
+    block_map: NodeMap<Module<'a>>,
+    module_map: FxHashMap<DefId, Module<'a>>,
     extern_crate_roots: FxHashMap<(CrateNum, bool /* MacrosOnly? */), Module<'a>>,
 
     pub make_glob_map: bool,
@@ -1153,15 +1153,12 @@ impl<'a> ResolverArenas<'a> {
     }
 }
 
-impl<'a> ty::NodeIdTree for Resolver<'a> {
-    fn is_descendant_of(&self, mut node: NodeId, ancestor: NodeId) -> bool {
-        while node != ancestor {
-            node = match self.module_map[&node].parent {
-                Some(parent) => parent.normal_ancestor_id.unwrap(),
-                None => return false,
-            }
-        }
-        true
+impl<'a, 'b: 'a> ty::DefIdTree for &'a Resolver<'b> {
+    fn parent(self, id: DefId) -> Option<DefId> {
+        match id.krate {
+            LOCAL_CRATE => self.definitions.def_key(id.index).parent,
+            _ => self.session.cstore.def_key(id).parent,
+        }.map(|index| DefId { index: index, ..id })
     }
 }
 
@@ -1202,14 +1199,14 @@ impl<'a> Resolver<'a> {
                crate_loader: &'a mut CrateLoader,
                arenas: &'a ResolverArenas<'a>)
                -> Resolver<'a> {
-        let root_def = Def::Mod(DefId::local(CRATE_DEF_INDEX));
+        let root_def_id = DefId::local(CRATE_DEF_INDEX);
+        let root_module_kind = ModuleKind::Def(Def::Mod(root_def_id), keywords::Invalid.name());
         let graph_root = arenas.alloc_module(ModuleData {
-            normal_ancestor_id: Some(CRATE_NODE_ID),
             no_implicit_prelude: attr::contains_name(&krate.attrs, "no_implicit_prelude"),
-            ..ModuleData::new(None, ModuleKind::Def(root_def, keywords::Invalid.name()))
+            ..ModuleData::new(None, root_module_kind, root_def_id)
         });
-        let mut module_map = NodeMap();
-        module_map.insert(CRATE_NODE_ID, graph_root);
+        let mut module_map = FxHashMap();
+        module_map.insert(DefId::local(CRATE_DEF_INDEX), graph_root);
 
         let mut definitions = Definitions::new();
         DefCollector::new(&mut definitions).collect_root();
@@ -1254,6 +1251,7 @@ impl<'a> Resolver<'a> {
             export_map: NodeMap(),
             trait_map: NodeMap(),
             module_map: module_map,
+            block_map: NodeMap(),
             extern_crate_roots: FxHashMap(),
 
             make_glob_map: make_glob_map == MakeGlobMap::Yes,
@@ -1324,12 +1322,9 @@ impl<'a> Resolver<'a> {
         self.crate_loader.postprocess(krate);
     }
 
-    fn new_module(&self, parent: Module<'a>, kind: ModuleKind, local: bool) -> Module<'a> {
-        self.arenas.alloc_module(ModuleData {
-            normal_ancestor_id: if local { self.current_module.normal_ancestor_id } else { None },
-            populated: Cell::new(local),
-            ..ModuleData::new(Some(parent), kind)
-        })
+    fn new_module(&self, parent: Module<'a>, kind: ModuleKind, normal_ancestor_id: DefId)
+                  -> Module<'a> {
+        self.arenas.alloc_module(ModuleData::new(Some(parent), kind, normal_ancestor_id))
     }
 
     fn record_use(&mut self, ident: Ident, ns: Namespace, binding: &'a NameBinding<'a>, span: Span)
@@ -1462,6 +1457,7 @@ impl<'a> Resolver<'a> {
     fn with_scope<F>(&mut self, id: NodeId, f: F)
         where F: FnOnce(&mut Resolver)
     {
+        let id = self.definitions.local_def_id(id);
         let module = self.module_map.get(&id).cloned(); // clones a reference
         if let Some(module) = module {
             // Move down in the graph.
@@ -1958,7 +1954,7 @@ impl<'a> Resolver<'a> {
         debug!("(resolving block) entering block");
         // Move down in the graph, if there's an anonymous module rooted here.
         let orig_module = self.current_module;
-        let anonymous_module = self.module_map.get(&block.id).cloned(); // clones a reference
+        let anonymous_module = self.block_map.get(&block.id).cloned(); // clones a reference
 
         let mut num_macro_definition_ribs = 0;
         if let Some(anonymous_module) = anonymous_module {
@@ -2334,13 +2330,13 @@ impl<'a> Resolver<'a> {
             let ns = if is_last { opt_ns.unwrap_or(TypeNS) } else { TypeNS };
 
             if i == 0 && ns == TypeNS && ident.name == keywords::SelfValue.name() {
-                module = Some(self.module_map[&self.current_module.normal_ancestor_id.unwrap()]);
+                module = Some(self.module_map[&self.current_module.normal_ancestor_id]);
                 continue
             } else if allow_super && ns == TypeNS && ident.name == keywords::Super.name() {
                 let current_module = if i == 0 { self.current_module } else { module.unwrap() };
-                let self_module = self.module_map[&current_module.normal_ancestor_id.unwrap()];
+                let self_module = self.module_map[&current_module.normal_ancestor_id];
                 if let Some(parent) = self_module.parent {
-                    module = Some(self.module_map[&parent.normal_ancestor_id.unwrap()]);
+                    module = Some(self.module_map[&parent.normal_ancestor_id]);
                     continue
                 } else {
                     let msg = "There are too many initial `super`s.".to_string();
@@ -3000,10 +2996,12 @@ impl<'a> Resolver<'a> {
     fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility {
         let (segments, span, id) = match *vis {
             ast::Visibility::Public => return ty::Visibility::Public,
-            ast::Visibility::Crate(_) => return ty::Visibility::Restricted(ast::CRATE_NODE_ID),
+            ast::Visibility::Crate(_) => {
+                return ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX));
+            }
             ast::Visibility::Restricted { ref path, id } => (&path.segments, path.span, id),
             ast::Visibility::Inherited => {
-                return ty::Visibility::Restricted(self.current_module.normal_ancestor_id.unwrap());
+                return ty::Visibility::Restricted(self.current_module.normal_ancestor_id);
             }
         };
 
@@ -3012,7 +3010,7 @@ impl<'a> Resolver<'a> {
         let vis = match self.resolve_path(&path, None, Some(span)) {
             PathResult::Module(module) => {
                 path_resolution = PathResolution::new(module.def().unwrap());
-                ty::Visibility::Restricted(module.normal_ancestor_id.unwrap())
+                ty::Visibility::Restricted(module.normal_ancestor_id)
             }
             PathResult::Failed(msg, _) => {
                 self.session.span_err(span, &format!("failed to resolve module path. {}", msg));
@@ -3029,11 +3027,11 @@ impl<'a> Resolver<'a> {
     }
 
     fn is_accessible(&self, vis: ty::Visibility) -> bool {
-        vis.is_accessible_from(self.current_module.normal_ancestor_id.unwrap(), self)
+        vis.is_accessible_from(self.current_module.normal_ancestor_id, self)
     }
 
     fn is_accessible_from(&self, vis: ty::Visibility, module: Module<'a>) -> bool {
-        vis.is_accessible_from(module.normal_ancestor_id.unwrap(), self)
+        vis.is_accessible_from(module.normal_ancestor_id, self)
     }
 
     fn report_errors(&mut self) {
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
index b0f72d9f6e8..44cc580ad12 100644
--- a/src/librustc_resolve/macros.rs
+++ b/src/librustc_resolve/macros.rs
@@ -88,7 +88,7 @@ impl<'a> base::Resolver for Resolver<'a> {
 
     fn get_module_scope(&mut self, id: ast::NodeId) -> Mark {
         let mark = Mark::fresh();
-        let module = self.module_map[&id];
+        let module = self.module_map[&self.definitions.local_def_id(id)];
         self.invocations.insert(mark, self.arenas.alloc_invocation_data(InvocationData {
             module: Cell::new(module),
             def_index: module.def_id().unwrap().index,
@@ -154,7 +154,7 @@ impl<'a> base::Resolver for Resolver<'a> {
         let binding = self.arenas.alloc_name_binding(NameBinding {
             kind: NameBindingKind::Def(Def::Macro(def_id)),
             span: DUMMY_SP,
-            vis: ty::Visibility::PrivateExternal,
+            vis: ty::Visibility::Invisible,
             expansion: Mark::root(),
         });
         self.builtin_macros.insert(ident.name, binding);
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index f62974b30d1..62dcb7e9e2f 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -19,6 +19,7 @@ use {resolve_error, ResolutionError};
 
 use rustc::ty;
 use rustc::lint::builtin::PRIVATE_IN_PUBLIC;
+use rustc::hir::def_id::DefId;
 use rustc::hir::def::*;
 
 use syntax::ast::{Ident, NodeId};
@@ -274,7 +275,7 @@ impl<'a> Resolver<'a> {
 
     // Given a binding and an import directive that resolves to it,
     // return the corresponding binding defined by the import directive.
-    pub fn import(&mut self, binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>)
+    pub fn import(&self, binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>)
                   -> &'a NameBinding<'a> {
         let vis = if binding.pseudo_vis().is_at_least(directive.vis.get(), self) ||
                      !directive.is_glob() && binding.is_extern_crate() { // c.f. `PRIVATE_IN_PUBLIC`
@@ -317,7 +318,7 @@ impl<'a> Resolver<'a> {
                         resolution.shadows_glob = Some(binding);
                     } else if binding.def() != old_binding.def() {
                         resolution.binding = Some(this.ambiguity(old_binding, binding));
-                    } else if !old_binding.vis.is_at_least(binding.vis, this) {
+                    } else if !old_binding.vis.is_at_least(binding.vis, &*this) {
                         // We are glob-importing the same item but with greater visibility.
                         resolution.binding = Some(binding);
                     }
@@ -340,7 +341,7 @@ impl<'a> Resolver<'a> {
         })
     }
 
-    pub fn ambiguity(&mut self, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>)
+    pub fn ambiguity(&self, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>)
                      -> &'a NameBinding<'a> {
         self.arenas.alloc_name_binding(NameBinding {
             kind: NameBindingKind::Ambiguity { b1: b1, b2: b2, legacy: false },
@@ -415,9 +416,9 @@ impl<'a, 'b: 'a> ::std::ops::DerefMut for ImportResolver<'a, 'b> {
     }
 }
 
-impl<'a, 'b: 'a> ty::NodeIdTree for ImportResolver<'a, 'b> {
-    fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
-        self.resolver.is_descendant_of(node, ancestor)
+impl<'a, 'b: 'a> ty::DefIdTree for &'a ImportResolver<'a, 'b> {
+    fn parent(self, id: DefId) -> Option<DefId> {
+        self.resolver.parent(id)
     }
 }
 
@@ -490,7 +491,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
             let vis = directive.vis.get();
             // For better failure detection, pretend that the import will not define any names
             // while resolving its module path.
-            directive.vis.set(ty::Visibility::PrivateExternal);
+            directive.vis.set(ty::Visibility::Invisible);
             let result = self.resolve_path(&directive.module_path, None, None);
             directive.vis.set(vis);
 
@@ -580,8 +581,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
             }
             GlobImport { is_prelude, ref max_vis } => {
                 if !is_prelude &&
-                   max_vis.get() != ty::Visibility::PrivateExternal && // Allow empty globs.
-                   !max_vis.get().is_at_least(directive.vis.get(), self) {
+                   max_vis.get() != ty::Visibility::Invisible && // Allow empty globs.
+                   !max_vis.get().is_at_least(directive.vis.get(), &*self) {
                     let msg = "A non-empty glob must import something with the glob's visibility";
                     self.session.span_err(directive.span, msg);
                 }
@@ -644,7 +645,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
         self.per_ns(|this, ns| {
             if let Ok(binding) = result[ns].get() {
                 let vis = directive.vis.get();
-                if !binding.pseudo_vis().is_at_least(vis, this) {
+                if !binding.pseudo_vis().is_at_least(vis, &*this) {
                     reexport_error = Some((ns, binding));
                 } else {
                     any_successful_reexport = true;
@@ -752,7 +753,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
             match binding.kind {
                 NameBindingKind::Import { binding: orig_binding, directive, .. } => {
                     if ns == TypeNS && orig_binding.is_variant() &&
-                       !orig_binding.vis.is_at_least(binding.vis, self) {
+                       !orig_binding.vis.is_at_least(binding.vis, &*self) {
                         let msg = format!("variant `{}` is private, and cannot be reexported \
                                            (error E0364), consider declaring its enum as `pub`",
                                           ident);