Lay the groundwork for privacy checking in typeck

6 files changed, 72 insertions, 50 deletions

diff --git a/src/librustc/front/map/mod.rs b/src/librustc/front/map/mod.rs
index adf14d0d89c..44f588c2e9c 100644
--- a/src/librustc/front/map/mod.rs
+++ b/src/librustc/front/map/mod.rs
@@ -495,6 +495,30 @@ impl<'ast> Map<'ast> {
         }
     }
 
+    /// Returns the NodeId of `id`'s nearest module parent, or `id` itself if no
+    /// module parent is in this map.
+    fn get_module_parent(&self, id: NodeId) -> NodeId {
+        match self.walk_parent_nodes(id, |node| match *node {
+            NodeItem(&Item { node: Item_::ItemMod(_), .. }) => true,
+            _ => false,
+        }) {
+            Ok(id) => id,
+            Err(id) => id,
+        }
+    }
+
+    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
diff --git a/src/librustc/middle/def.rs b/src/librustc/middle/def.rs
index aee8fb10c2a..c0a5d27a506 100644
--- a/src/librustc/middle/def.rs
+++ b/src/librustc/middle/def.rs
@@ -152,4 +152,29 @@ impl Def {
             _ => None
         }
     }
+
+    pub fn kind_name(&self) -> &'static str {
+        match *self {
+            Def::Fn(..) => "function",
+            Def::Mod(..) => "module",
+            Def::ForeignMod(..) => "foreign module",
+            Def::Static(..) => "static",
+            Def::Variant(..) => "variant",
+            Def::Enum(..) => "enum",
+            Def::TyAlias(..) => "type",
+            Def::AssociatedTy(..) => "associated type",
+            Def::Struct(..) => "struct",
+            Def::Trait(..) => "trait",
+            Def::Method(..) => "method",
+            Def::Const(..) => "const",
+            Def::AssociatedConst(..) => "associated const",
+            Def::TyParam(..) => "type parameter",
+            Def::PrimTy(..) => "builtin type",
+            Def::Local(..) => "local variable",
+            Def::Upvar(..) => "closure capture",
+            Def::Label(..) => "label",
+            Def::SelfTy(..) => "self type",
+            Def::Err => "unresolved item",
+        }
+    }
 }
diff --git a/src/librustc/middle/ty/mod.rs b/src/librustc/middle/ty/mod.rs
index 00a011c6b5d..b6e8a670182 100644
--- a/src/librustc/middle/ty/mod.rs
+++ b/src/librustc/middle/ty/mod.rs
@@ -173,6 +173,14 @@ impl<'tcx> ImplOrTraitItem<'tcx> {
         }
     }
 
+    pub fn def(&self) -> Def {
+        match *self {
+            ConstTraitItem(ref associated_const) => Def::AssociatedConst(associated_const.def_id),
+            MethodTraitItem(ref method) => Def::Method(method.def_id),
+            TypeTraitItem(ref ty) => Def::AssociatedTy(ty.container.id(), ty.def_id),
+        }
+    }
+
     pub fn def_id(&self) -> DefId {
         match *self {
             ConstTraitItem(ref associated_const) => associated_const.def_id,
diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs
index 8908dac7a36..9f0afb7b98a 100644
--- a/src/librustc_privacy/lib.rs
+++ b/src/librustc_privacy/lib.rs
@@ -692,32 +692,7 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
     /// whether the node is accessible by the current module that iteration is
     /// inside.
     fn private_accessible(&self, id: ast::NodeId) -> bool {
-        let parent = *self.parents.get(&id).unwrap();
-        debug!("privacy - accessible parent {}", self.nodestr(parent));
-
-        // After finding `did`'s closest private member, we roll ourselves back
-        // to see if this private member's parent is anywhere in our ancestry.
-        // By the privacy rules, we can access all of our ancestor's private
-        // members, so that's why we test the parent, and not the did itself.
-        let mut cur = self.curitem;
-        loop {
-            debug!("privacy - questioning {}, {}", self.nodestr(cur), cur);
-            match cur {
-                // If the relevant parent is in our history, then we're allowed
-                // to look inside any of our ancestor's immediate private items,
-                // so this access is valid.
-                x if x == parent => return true,
-
-                // If we've reached the root, then we couldn't access this item
-                // in the first place
-                ast::DUMMY_NODE_ID => return false,
-
-                // Keep going up
-                _ => {}
-            }
-
-            cur = *self.parents.get(&cur).unwrap();
-        }
+        self.tcx.map.private_item_is_visible_from(id, self.curitem)
     }
 
     fn report_error(&self, result: CheckResult) -> bool {
@@ -835,7 +810,7 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
             }
             UnnamedField(idx) => &v.fields[idx]
         };
-        if field.vis == hir::Public || self.local_private_accessible(field.did) {
+        if field.vis == hir::Public || self.local_private_accessible(def.did) {
             return;
         }
 
@@ -945,19 +920,9 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
         // def map is not. Therefore the names we work out below will not always
         // be accurate and we can get slightly wonky error messages (but type
         // checking is always correct).
-        match path_res.full_def() {
-            Def::Fn(..) => ck("function"),
-            Def::Static(..) => ck("static"),
-            Def::Const(..) => ck("const"),
-            Def::AssociatedConst(..) => ck("associated const"),
-            Def::Variant(..) => ck("variant"),
-            Def::TyAlias(..) => ck("type"),
-            Def::Enum(..) => ck("enum"),
-            Def::Trait(..) => ck("trait"),
-            Def::Struct(..) => ck("struct"),
-            Def::Method(..) => ck("method"),
-            Def::Mod(..) => ck("module"),
-            _ => {}
+        let def = path_res.full_def();
+        if def != Def::Err {
+            ck(def.kind_name());
         }
     }
 
@@ -1036,7 +1001,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 != hir::Public && !self.local_private_accessible(f.did)
+                        f.vis != hir::Public && !self.local_private_accessible(def.did)
                     });
                     if any_priv {
                         span_err!(self.tcx.sess, expr.span, E0450,
diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs
index fc2dd4475e3..f680607eb99 100644
--- a/src/librustc_typeck/check/method/mod.rs
+++ b/src/librustc_typeck/check/method/mod.rs
@@ -338,20 +338,13 @@ pub fn resolve_ufcs<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
 {
     let mode = probe::Mode::Path;
     let pick = try!(probe::probe(fcx, span, mode, method_name, self_ty, expr_id));
-    let def_id = pick.item.def_id();
+    let def_result = pick.item.def();
     let mut lp = LastMod(AllPublic);
     if let probe::InherentImplPick = pick.kind {
         if pick.item.vis() != hir::Public {
-            lp = LastMod(DependsOn(def_id));
+            lp = LastMod(DependsOn(def_result.def_id()));
         }
     }
-    let def_result = match pick.item {
-        ty::ImplOrTraitItem::MethodTraitItem(..) => Def::Method(def_id),
-        ty::ImplOrTraitItem::ConstTraitItem(..) => Def::AssociatedConst(def_id),
-        ty::ImplOrTraitItem::TypeTraitItem(..) => {
-            fcx.tcx().sess.span_bug(span, "resolve_ufcs: probe picked associated type");
-        }
-    };
     Ok((def_result, lp))
 }
 
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
index 7ab4975c8b8..9170c00a261 100644
--- a/src/librustc_typeck/check/mod.rs
+++ b/src/librustc_typeck/check/mod.rs
@@ -2022,6 +2022,13 @@ 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> {