Auto merge of #38490 - jseyfried:def_id_vis, r=nrc

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

This is groundwork for hygiene 2.0, specifically privacy checking hygienic intercrate name resolutions.
r? @nrc

src/librustc/ty/mod.rs

@@ -219,63 +219,65 @@ pub enum Visibility {
     /// Visible everywhere (including in other crates).
     Public,
     /// Visible only in the given crate-local module.
-    Restricted(NodeId),
+    Restricted(DefId),
     /// Not visible anywhere in the local crate. This is the visibility of private external items.
-    PrivateExternal,
+    Invisible,
 }
 
-pub trait NodeIdTree {
-    fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool;
+pub trait DefIdTree: Copy {
+    fn parent(self, id: DefId) -> Option<DefId>;
 }
 
-impl<'a> NodeIdTree for ast_map::Map<'a> {
-    fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
-        let mut node_ancestor = node;
-        while node_ancestor != ancestor {
-            let node_ancestor_parent = self.get_module_parent(node_ancestor);
-            if node_ancestor_parent == node_ancestor {
-                return false;
-            }
-            node_ancestor = node_ancestor_parent;
-        }
-        true
+impl<'a, 'gcx, 'tcx> DefIdTree for TyCtxt<'a, 'gcx, 'tcx> {
+    fn parent(self, id: DefId) -> Option<DefId> {
+        self.def_key(id).parent.map(|index| DefId { index: index, ..id })
     }
 }
 
 impl Visibility {
     pub fn from_hir(visibility: &hir::Visibility, id: NodeId, tcx: TyCtxt) -> Self {
         match *visibility {
             hir::Public => Visibility::Public,
-            hir::Visibility::Crate => Visibility::Restricted(ast::CRATE_NODE_ID),
+            hir::Visibility::Crate => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
             hir::Visibility::Restricted { ref path, .. } => match path.def {
                 // If there is no resolution, `resolve` will have already reported an error, so
                 // assume that the visibility is public to avoid reporting more privacy errors.
                 Def::Err => Visibility::Public,
-                def => Visibility::Restricted(tcx.map.as_local_node_id(def.def_id()).unwrap()),
+                def => Visibility::Restricted(def.def_id()),
             },
-            hir::Inherited => Visibility::Restricted(tcx.map.get_module_parent(id)),
+            hir::Inherited => {
+                Visibility::Restricted(tcx.map.local_def_id(tcx.map.get_module_parent(id)))
+            }
         }
     }
 
     /// Returns true if an item with this visibility is accessible from the given block.
-    pub fn is_accessible_from<T: NodeIdTree>(self, block: NodeId, tree: &T) -> bool {
+    pub fn is_accessible_from<T: DefIdTree>(self, mut module: DefId, tree: T) -> bool {
         let restriction = match self {
             // Public items are visible everywhere.
             Visibility::Public => return true,
             // Private items from other crates are visible nowhere.
-            Visibility::PrivateExternal => return false,
+            Visibility::Invisible => return false,
             // Restricted items are visible in an arbitrary local module.
+            Visibility::Restricted(other) if other.krate != module.krate => return false,
             Visibility::Restricted(module) => module,
         };
 
-        tree.is_descendant_of(block, restriction)
+        while module != restriction {
+            match tree.parent(module) {
+                Some(parent) => module = parent,
+                None => return false,
+            }
+        }
+
+        true
     }
 
     /// Returns true if this visibility is at least as accessible as the given visibility
-    pub fn is_at_least<T: NodeIdTree>(self, vis: Visibility, tree: &T) -> bool {
+    pub fn is_at_least<T: DefIdTree>(self, vis: Visibility, tree: T) -> bool {
         let vis_restriction = match vis {
             Visibility::Public => return self == Visibility::Public,
-            Visibility::PrivateExternal => return true,
+            Visibility::Invisible => return true,
             Visibility::Restricted(module) => module,
         };
 
@@ -1779,7 +1781,7 @@ impl<'a, 'gcx, 'tcx> FieldDef {
                                   block: Option<NodeId>,
                                   tcx: TyCtxt<'a, 'gcx, 'tcx>,
                                   substs: &'tcx Substs<'tcx>) -> bool {
-        block.map_or(true, |b| self.vis.is_accessible_from(b, &tcx.map)) &&
+        block.map_or(true, |b| tcx.vis_is_accessible_from(self.vis, b)) &&
         self.ty(tcx, substs).is_uninhabited_recurse(visited, block, tcx)
     }
 }
@@ -2266,6 +2268,10 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
+    pub fn vis_is_accessible_from(self, vis: Visibility, block: NodeId) -> bool {
+        vis.is_accessible_from(self.map.local_def_id(self.map.get_module_parent(block)), self)
+    }
+
     pub fn item_name(self, id: DefId) -> ast::Name {
         if let Some(id) = self.map.as_local_node_id(id) {
             self.map.name(id)

src/librustc_metadata/decoder.rs

@@ -588,7 +588,7 @@ impl<'a, 'tcx> CrateMetadata {
                 ty::FieldDef {
                     did: self.local_def_id(index),
                     name: self.item_name(index),
-                    vis: f.visibility
+                    vis: f.visibility.decode(self)
                 }
             }).collect(),
             disr_val: ConstInt::Infer(data.disr),
@@ -678,7 +678,7 @@ impl<'a, 'tcx> CrateMetadata {
     pub fn get_visibility(&self, id: DefIndex) -> ty::Visibility {
         match self.is_proc_macro(id) {
             true => ty::Visibility::Public,
-            false => self.entry(id).visibility,
+            false => self.entry(id).visibility.decode(self),
         }
     }
 
@@ -885,7 +885,7 @@ impl<'a, 'tcx> CrateMetadata {
                 ty::AssociatedItem {
                     name: name,
                     kind: ty::AssociatedKind::Const,
-                    vis: item.visibility,
+                    vis: item.visibility.decode(self),
                     defaultness: container.defaultness(),
                     def_id: self.local_def_id(id),
                     container: container.with_def_id(parent),
@@ -898,7 +898,7 @@ impl<'a, 'tcx> CrateMetadata {
                 ty::AssociatedItem {
                     name: name,
                     kind: ty::AssociatedKind::Method,
-                    vis: item.visibility,
+                    vis: item.visibility.decode(self),
                     defaultness: data.container.defaultness(),
                     def_id: self.local_def_id(id),
                     container: data.container.with_def_id(parent),
@@ -910,7 +910,7 @@ impl<'a, 'tcx> CrateMetadata {
                 ty::AssociatedItem {
                     name: name,
                     kind: ty::AssociatedKind::Type,
-                    vis: item.visibility,
+                    vis: item.visibility.decode(self),
                     defaultness: container.defaultness(),
                     def_id: self.local_def_id(id),
                     container: container.with_def_id(parent),

src/librustc_metadata/encoder.rs

@@ -268,7 +268,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: EntryKind::Variant(self.lazy(&data)),
-            visibility: enum_vis.simplify(),
+            visibility: self.lazy(&ty::Visibility::from_hir(enum_vis, enum_id, tcx)),
             span: self.lazy(&tcx.def_span(def_id)),
             attributes: self.encode_attributes(&tcx.get_attrs(def_id)),
             children: self.lazy_seq(variant.fields.iter().map(|f| {
@@ -306,7 +306,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: EntryKind::Mod(self.lazy(&data)),
-            visibility: vis.simplify(),
+            visibility: self.lazy(&ty::Visibility::from_hir(vis, id, tcx)),
             span: self.lazy(&md.inner),
             attributes: self.encode_attributes(attrs),
             children: self.lazy_seq(md.item_ids.iter().map(|item_id| {
@@ -327,30 +327,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
     }
 }
 
-trait Visibility {
-    fn simplify(&self) -> ty::Visibility;
-}
-
-impl Visibility for hir::Visibility {
-    fn simplify(&self) -> ty::Visibility {
-        if *self == hir::Public {
-            ty::Visibility::Public
-        } else {
-            ty::Visibility::PrivateExternal
-        }
-    }
-}
-
-impl Visibility for ty::Visibility {
-    fn simplify(&self) -> ty::Visibility {
-        if *self == ty::Visibility::Public {
-            ty::Visibility::Public
-        } else {
-            ty::Visibility::PrivateExternal
-        }
-    }
-}
-
 impl<'a, 'b, 'tcx> IndexBuilder<'a, 'b, 'tcx> {
     fn encode_fields(&mut self, adt_def_id: DefId) {
         let def = self.tcx.lookup_adt_def(adt_def_id);
@@ -386,7 +362,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: EntryKind::Field,
-            visibility: field.vis.simplify(),
+            visibility: self.lazy(&field.vis),
             span: self.lazy(&tcx.def_span(def_id)),
             attributes: self.encode_attributes(&variant_data.fields()[field_index].attrs),
             children: LazySeq::empty(),
@@ -419,7 +395,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: EntryKind::Struct(self.lazy(&data)),
-            visibility: struct_vis.simplify(),
+            visibility: self.lazy(&ty::Visibility::from_hir(struct_vis, struct_id, tcx)),
             span: self.lazy(&tcx.def_span(def_id)),
             attributes: LazySeq::empty(),
             children: LazySeq::empty(),
@@ -485,7 +461,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: kind,
-            visibility: trait_item.vis.simplify(),
+            visibility: self.lazy(&trait_item.vis),
             span: self.lazy(&ast_item.span),
             attributes: self.encode_attributes(&ast_item.attrs),
             children: LazySeq::empty(),
@@ -574,7 +550,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: kind,
-            visibility: impl_item.vis.simplify(),
+            visibility: self.lazy(&impl_item.vis),
             span: self.lazy(&ast_item.span),
             attributes: self.encode_attributes(&ast_item.attrs),
             children: LazySeq::empty(),
@@ -736,7 +712,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: kind,
-            visibility: item.vis.simplify(),
+            visibility: self.lazy(&ty::Visibility::from_hir(&item.vis, item.id, tcx)),
             span: self.lazy(&item.span),
             attributes: self.encode_attributes(&item.attrs),
             children: match item.node {
@@ -849,7 +825,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             kind: EntryKind::MacroDef(self.lazy(&MacroDef {
                 body: ::syntax::print::pprust::tts_to_string(&macro_def.body)
             })),
-            visibility: ty::Visibility::Public,
+            visibility: self.lazy(&ty::Visibility::Public),
             span: self.lazy(&macro_def.span),
 
             attributes: self.encode_attributes(&macro_def.attrs),
@@ -950,7 +926,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: kind,
-            visibility: nitem.vis.simplify(),
+            visibility: self.lazy(&ty::Visibility::from_hir(&nitem.vis, nitem.id, tcx)),
             span: self.lazy(&nitem.span),
             attributes: self.encode_attributes(&nitem.attrs),
             children: LazySeq::empty(),
@@ -1032,7 +1008,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         let tcx = self.tcx;
         Entry {
             kind: EntryKind::Type,
-            visibility: ty::Visibility::Public,
+            visibility: self.lazy(&ty::Visibility::Public),
             span: self.lazy(&tcx.def_span(def_id)),
             attributes: LazySeq::empty(),
             children: LazySeq::empty(),
@@ -1060,7 +1036,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         Entry {
             kind: EntryKind::Closure(self.lazy(&data)),
-            visibility: ty::Visibility::Public,
+            visibility: self.lazy(&ty::Visibility::Public),
             span: self.lazy(&tcx.def_span(def_id)),
             attributes: self.encode_attributes(&tcx.get_attrs(def_id)),
             children: LazySeq::empty(),

src/librustc_metadata/schema.rs

@@ -201,7 +201,7 @@ pub struct TraitImpls {
 #[derive(RustcEncodable, RustcDecodable)]
 pub struct Entry<'tcx> {
     pub kind: EntryKind<'tcx>,
-    pub visibility: ty::Visibility,
+    pub visibility: Lazy<ty::Visibility>,
     pub span: Lazy<Span>,
     pub attributes: LazySeq<ast::Attribute>,
     pub children: LazySeq<DefIndex>,