Avoid using the hir map when visibility checking in resolve

Refactor `ty::Visibility` methods to use a new trait `NodeIdTree` instead of the ast map.

Author: jseyfried

src/librustc/ty/mod.rs

@@ -283,6 +283,22 @@ pub enum Visibility {
     PrivateExternal,
 }
 
+pub trait NodeIdTree {
+    fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool;
+}
+
+impl<'a> NodeIdTree for ast_map::Map<'a> {
+    fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
+        let mut node_ancestor = node;
+        loop {
+            if node_ancestor == ancestor { return true }
+            let node_ancestor_parent = self.get_module_parent(node_ancestor);
+            if node_ancestor_parent == node_ancestor { return false }
+            node_ancestor = node_ancestor_parent;
+        }
+    }
+}
+
 impl Visibility {
     pub fn from_hir(visibility: &hir::Visibility, id: NodeId, tcx: &TyCtxt) -> Self {
         match *visibility {
@@ -301,7 +317,7 @@ impl Visibility {
     }
 
     /// Returns true if an item with this visibility is accessible from the given block.
-    pub fn is_accessible_from(self, block: NodeId, map: &ast_map::Map) -> bool {
+    pub fn is_accessible_from<T: NodeIdTree>(self, block: NodeId, tree: &T) -> bool {
         let restriction = match self {
             // Public items are visible everywhere.
             Visibility::Public => return true,
@@ -311,24 +327,18 @@ impl Visibility {
             Visibility::Restricted(module) => module,
         };
 
-        let mut block_ancestor = block;
-        loop {
-            if block_ancestor == restriction { return true }
-            let block_ancestor_parent = map.get_module_parent(block_ancestor);
-            if block_ancestor_parent == block_ancestor { return false }
-            block_ancestor = block_ancestor_parent;
-        }
+        tree.is_descendant_of(block, restriction)
     }
 
     /// Returns true if this visibility is at least as accessible as the given visibility
-    pub fn is_at_least(self, vis: Visibility, map: &ast_map::Map) -> bool {
+    pub fn is_at_least<T: NodeIdTree>(self, vis: Visibility, tree: &T) -> bool {
         let vis_restriction = match vis {
             Visibility::Public => return self == Visibility::Public,
             Visibility::PrivateExternal => return true,
             Visibility::Restricted(module) => module,
         };
 
-        self.is_accessible_from(vis_restriction, map)
+        self.is_accessible_from(vis_restriction, tree)
     }
 }
 

src/librustc_resolve/lib.rs

@@ -1121,6 +1121,21 @@ impl<'a> ResolverArenas<'a> {
     }
 }
 
+impl<'a, 'tcx> ty::NodeIdTree for Resolver<'a, 'tcx> {
+    fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
+        let ancestor = self.ast_map.local_def_id(ancestor);
+        let mut module = *self.module_map.get(&node).unwrap();
+        loop {
+            if module.def_id() == Some(ancestor) { return true; }
+            let module_parent = match self.get_nearest_normal_module_parent(module) {
+                Some(parent) => parent,
+                None => return false,
+            };
+            module = module_parent;
+        }
+    }
+}
+
 impl<'a, 'tcx> Resolver<'a, 'tcx> {
     fn new(session: &'a Session,
            ast_map: &'a hir_map::Map<'tcx>,
@@ -1131,6 +1146,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         let graph_root =
             ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false, arenas);
         let graph_root = arenas.alloc_module(graph_root);
+        let mut module_map = NodeMap();
+        module_map.insert(CRATE_NODE_ID, graph_root);
 
         Resolver {
             session: session,
@@ -1161,7 +1178,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             freevars_seen: NodeMap(),
             export_map: NodeMap(),
             trait_map: NodeMap(),
-            module_map: NodeMap(),
+            module_map: module_map,
             used_imports: HashSet::new(),
             used_crates: HashSet::new(),
 
@@ -3343,7 +3360,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     fn is_accessible(&self, vis: ty::Visibility) -> bool {
         let current_module = self.get_nearest_normal_module_parent_or_self(self.current_module);
         let node_id = self.ast_map.as_local_node_id(current_module.def_id().unwrap()).unwrap();
-        vis.is_accessible_from(node_id, &self.ast_map)
+        vis.is_accessible_from(node_id, self)
     }
 
     fn check_privacy(&mut self, name: Name, binding: &'a NameBinding<'a>, span: Span) {

src/librustc_resolve/resolve_imports.rs

@@ -552,9 +552,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             _ => (),
         }
 
-        let ast_map = self.resolver.ast_map;
         match (&value_result, &type_result) {
-            (&Success(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, ast_map) &&
+            (&Success(binding), _) if !binding.pseudo_vis()
+                                              .is_at_least(directive.vis, self.resolver) &&
                                       self.resolver.is_accessible(binding.vis) => {
                 let msg = format!("`{}` is private, and cannot be reexported", source);
                 let note_msg = format!("consider marking `{}` as `pub` in the imported module",
@@ -564,7 +564,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     .emit();
             }
 
-            (_, &Success(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, ast_map) &&
+            (_, &Success(binding)) if !binding.pseudo_vis()
+                                              .is_at_least(directive.vis, self.resolver) &&
                                       self.resolver.is_accessible(binding.vis) => {
                 if binding.is_extern_crate() {
                     let msg = format!("extern crate `{}` is private, and cannot be reexported \
@@ -691,7 +692,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
             if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind {
                 if ns == TypeNS && orig_binding.is_variant() &&
-                   !orig_binding.vis.is_at_least(binding.vis, &self.resolver.ast_map) {
+                   !orig_binding.vis.is_at_least(binding.vis, self.resolver) {
                     let msg = format!("variant `{}` is private, and cannot be reexported \
                                        (error E0364), consider declaring its enum as `pub`",
                                       name);