From 13f5fca0f2c72b8af256f052b51bd636b6932c8a Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Fri, 26 Feb 2016 19:20:53 +0000 Subject: [PATCH] privacy: change def_privacy so that it checks for visiblity instead of nameability --- src/librustc_privacy/lib.rs | 104 +++++++++++++----------------------- 1 file changed, 37 insertions(+), 67 deletions(-) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 782ac593f44..226539b8e80 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -492,11 +492,6 @@ enum FieldName { } impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { - // used when debugging - fn nodestr(&self, id: ast::NodeId) -> String { - self.tcx.map.node_to_string(id).to_string() - } - // Determines whether the given definition is public from the point of view // of the current item. fn def_privacy(&self, did: DefId) -> PrivacyResult { @@ -604,75 +599,50 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { return Allowable; } - // We now know that there is at least one private member between the - // destination and the root. - let mut closest_private_id = node_id; - loop { - debug!("privacy - examining {}", self.nodestr(closest_private_id)); - let vis = match self.tcx.map.find(closest_private_id) { - // If this item is a method, then we know for sure that it's an - // actual method and not a static method. The reason for this is - // that these cases are only hit in the ExprMethodCall - // expression, and ExprCall will have its path checked later - // (the path of the trait/impl) if it's a static method. - // - // With this information, then we can completely ignore all - // trait methods. The privacy violation would be if the trait - // couldn't get imported, not if the method couldn't be used - // (all trait methods are public). - // - // However, if this is an impl method, then we dictate this - // decision solely based on the privacy of the method - // invocation. - // FIXME(#10573) is this the right behavior? Why not consider - // where the method was defined? - Some(ast_map::NodeImplItem(ii)) => { - match ii.node { - hir::ImplItemKind::Const(..) | - hir::ImplItemKind::Method(..) => { - let imp = self.tcx.map - .get_parent_did(closest_private_id); - match self.tcx.impl_trait_ref(imp) { - Some(..) => return Allowable, - _ if ii.vis == hir::Public => { - return Allowable - } - _ => ii.vis - } + let vis = match self.tcx.map.find(node_id) { + // If this item is a method, then we know for sure that it's an + // actual method and not a static method. The reason for this is + // that these cases are only hit in the ExprMethodCall + // expression, and ExprCall will have its path checked later + // (the path of the trait/impl) if it's a static method. + // + // With this information, then we can completely ignore all + // trait methods. The privacy violation would be if the trait + // couldn't get imported, not if the method couldn't be used + // (all trait methods are public). + // + // However, if this is an impl method, then we dictate this + // decision solely based on the privacy of the method + // invocation. + Some(ast_map::NodeImplItem(ii)) => { + match ii.node { + hir::ImplItemKind::Const(..) | + hir::ImplItemKind::Method(..) => { + let imp = self.tcx.map.get_parent_did(node_id); + match self.tcx.impl_trait_ref(imp) { + Some(..) => hir::Public, + _ => ii.vis } - hir::ImplItemKind::Type(_) => return Allowable, } + hir::ImplItemKind::Type(_) => hir::Public, } - Some(ast_map::NodeTraitItem(_)) => { - return Allowable; - } + } + Some(ast_map::NodeTraitItem(_)) => hir::Public, - // This is not a method call, extract the visibility as one - // would normally look at it - Some(ast_map::NodeItem(it)) => it.vis, - Some(ast_map::NodeForeignItem(_)) => { - self.tcx.map.get_foreign_vis(closest_private_id) - } - Some(ast_map::NodeVariant(..)) => { - hir::Public // need to move up a level (to the enum) - } - _ => hir::Public, - }; - if vis != hir::Public { break } - // if we've reached the root, then everything was allowable and this - // access is public. - if closest_private_id == ast::CRATE_NODE_ID { return Allowable } - closest_private_id = *self.parents.get(&closest_private_id).unwrap(); + // This is not a method call, extract the visibility as one + // would normally look at it + Some(ast_map::NodeItem(it)) => it.vis, + Some(ast_map::NodeForeignItem(_)) => { + self.tcx.map.get_foreign_vis(node_id) + } + _ => hir::Public, + }; + if vis == hir::Public { return Allowable } - // If we reached the top, then we were public all the way down and - // we can allow this access. - if closest_private_id == ast::DUMMY_NODE_ID { return Allowable } - } - debug!("privacy - closest priv {}", self.nodestr(closest_private_id)); - if self.private_accessible(closest_private_id) { + if self.private_accessible(node_id) { Allowable } else { - DisallowedBy(closest_private_id) + DisallowedBy(node_id) } }