Use the node id from the Restricted variant when checking accessibility

in `typeck` and in `privacy::PrivacyVisitor`.

Author: jseyfried

src/librustc/hir/map/mod.rs

@@ -440,18 +440,6 @@ impl<'ast> Map<'ast> {
         }
     }
 
-    pub fn private_item_is_visible_from(&self, item: NodeId, block: NodeId) -> bool {
-        // A private item is visible from everything in its nearest module parent.
-        let visibility = self.get_module_parent(item);
-        let mut block_ancestor = self.get_module_parent(block);
-        loop {
-            if block_ancestor == visibility { return true }
-            let block_ancestor_parent = self.get_module_parent(block_ancestor);
-            if block_ancestor_parent == block_ancestor { return false }
-            block_ancestor = block_ancestor_parent;
-        }
-    }
-
     /// Returns the nearest enclosing scope. A scope is an item or block.
     /// FIXME it is not clear to me that all items qualify as scopes - statics
     /// and associated types probably shouldn't, for example. Behaviour in this

src/librustc/ty/mod.rs

@@ -290,6 +290,26 @@ impl Visibility {
             hir::Inherited => Visibility::Restricted(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(self, block: NodeId, map: &ast_map::Map) -> 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,
+            // Restricted items are visible in an arbitrary local module.
+            Visibility::Restricted(module) => module,
+        };
+
+        let mut block_ancestor = map.get_module_parent(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;
+        }
+    }
 }
 
 #[derive(Clone, Debug)]

src/librustc_privacy/lib.rs

@@ -382,26 +382,18 @@ struct PrivacyVisitor<'a, 'tcx: 'a> {
 }
 
 impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
-    fn item_is_visible(&self, did: DefId) -> bool {
-        let is_public = match self.tcx.map.as_local_node_id(did) {
-            Some(node_id) => self.tcx.map.expect_item(node_id).vis == hir::Public,
-            None => self.tcx.sess.cstore.visibility(did) == ty::Visibility::Public,
-        };
-        is_public || self.private_accessible(did)
-    }
-
-    /// True if `did` is private-accessible
-    fn private_accessible(&self, did: DefId) -> bool {
+    fn item_is_accessible(&self, did: DefId) -> bool {
         match self.tcx.map.as_local_node_id(did) {
-            Some(node_id) => self.tcx.map.private_item_is_visible_from(node_id, self.curitem),
-            None => false,
-        }
+            Some(node_id) =>
+                ty::Visibility::from_hir(&self.tcx.map.expect_item(node_id).vis, node_id, self.tcx),
+            None => self.tcx.sess.cstore.visibility(did),
+        }.is_accessible_from(self.curitem, &self.tcx.map)
     }
 
     // Checks that a field is in scope.
     fn check_field(&mut self, span: Span, def: ty::AdtDef<'tcx>, field: ty::FieldDef<'tcx>) {
         if def.adt_kind() == ty::AdtKind::Struct &&
-                field.vis != ty::Visibility::Public && !self.private_accessible(def.did) {
+           !field.vis.is_accessible_from(self.curitem, &self.tcx.map) {
             span_err!(self.tcx.sess, span, E0451, "field `{}` of struct `{}` is private",
                       field.name, self.tcx.item_path_str(def.did));
         }
@@ -412,7 +404,7 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
         match self.tcx.impl_or_trait_item(method_def_id).container() {
             // Trait methods are always all public. The only controlling factor
             // is whether the trait itself is accessible or not.
-            ty::TraitContainer(trait_def_id) if !self.item_is_visible(trait_def_id) => {
+            ty::TraitContainer(trait_def_id) if !self.item_is_accessible(trait_def_id) => {
                 let msg = format!("source trait `{}` is private",
                                   self.tcx.item_path_str(trait_def_id));
                 self.tcx.sess.span_err(span, &msg);
@@ -464,7 +456,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {
                         _ => expr_ty
                     }.ty_adt_def().unwrap();
                     let any_priv = def.struct_variant().fields.iter().any(|f| {
-                        f.vis != ty::Visibility::Public && !self.private_accessible(def.did)
+                        !f.vis.is_accessible_from(self.curitem, &self.tcx.map)
                     });
                     if any_priv {
                         span_err!(self.tcx.sess, expr.span, E0450,

src/librustc_typeck/check/method/mod.rs

@@ -16,7 +16,7 @@ use hir::def::Def;
 use hir::def_id::DefId;
 use rustc::ty::subst;
 use rustc::traits;
-use rustc::ty::{self, TyCtxt, ToPredicate, ToPolyTraitRef, TraitRef, TypeFoldable, Visibility};
+use rustc::ty::{self, TyCtxt, ToPredicate, ToPolyTraitRef, TraitRef, TypeFoldable};
 use rustc::ty::adjustment::{AdjustDerefRef, AutoDerefRef, AutoPtr};
 use rustc::infer;
 
@@ -343,7 +343,7 @@ pub fn resolve_ufcs<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
     let def = pick.item.def();
 
     if let probe::InherentImplPick = pick.kind {
-        if pick.item.vis() != Visibility::Public && !fcx.private_item_is_visible(def.def_id()) {
+        if !pick.item.vis().is_accessible_from(fcx.body_id, &fcx.tcx().map) {
             let msg = format!("{} `{}` is private", def.kind_name(), &method_name.as_str());
             fcx.tcx().sess.span_err(span, &msg);
         }

src/librustc_typeck/check/method/probe.rs

@@ -20,7 +20,7 @@ use hir::def::Def;
 use rustc::ty::subst;
 use rustc::ty::subst::Subst;
 use rustc::traits;
-use rustc::ty::{self, NoPreference, Ty, TyCtxt, ToPolyTraitRef, TraitRef, TypeFoldable, Visibility};
+use rustc::ty::{self, NoPreference, Ty, TyCtxt, ToPolyTraitRef, TraitRef, TypeFoldable};
 use rustc::infer::{self, InferCtxt, InferOk, TypeOrigin};
 use syntax::ast;
 use syntax::codemap::{Span, DUMMY_SP};
@@ -412,7 +412,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
             return self.record_static_candidate(ImplSource(impl_def_id));
         }
 
-        if item.vis() != Visibility::Public && !self.fcx.private_item_is_visible(item.def_id()) {
+        if !item.vis().is_accessible_from(self.fcx.body_id, &self.tcx().map) {
             self.private_candidate = Some(item.def());
             return
         }

src/librustc_typeck/check/mod.rs

@@ -2054,13 +2054,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             Err(errors) => { report_fulfillment_errors(self.infcx(), &errors); }
         }
     }
-
-    fn private_item_is_visible(&self, def_id: DefId) -> bool {
-        match self.tcx().map.as_local_node_id(def_id) {
-            Some(node_id) => self.tcx().map.private_item_is_visible_from(node_id, self.body_id),
-            None => false, // Private items from other crates are never visible
-        }
-    }
 }
 
 impl<'a, 'tcx> RegionScope for FnCtxt<'a, 'tcx> {
@@ -2966,8 +2959,7 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                     debug!("struct named {:?}",  base_t);
                     if let Some(field) = base_def.struct_variant().find_field_named(field.node) {
                         let field_ty = fcx.field_ty(expr.span, field, substs);
-                        if field.vis == Visibility::Public ||
-                           fcx.private_item_is_visible(base_def.did) {
+                        if field.vis.is_accessible_from(fcx.body_id, &fcx.tcx().map) {
                             return Some(field_ty);
                         }
                         private_candidate = Some((base_def.did, field_ty));
@@ -3079,7 +3071,7 @@ fn check_expr_with_expectation_and_lvalue_pref<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                 debug!("tuple struct named {:?}",  base_t);
                 if let Some(field) = base_def.struct_variant().fields.get(idx.node) {
                     let field_ty = fcx.field_ty(expr.span, field, substs);
-                    if field.vis == Visibility::Public || fcx.private_item_is_visible(base_def.did) {
+                    if field.vis.is_accessible_from(fcx.body_id, &fcx.tcx().map) {
                         return Some(field_ty);
                     }
                     private_candidate = Some((base_def.did, field_ty));