1
Fork 0

privacy: change def_privacy so that it checks for visiblity instead of nameability

This commit is contained in:
Jeffrey Seyfried 2016-02-26 19:20:53 +00:00
parent c97524bef9
commit 13f5fca0f2

View file

@ -492,11 +492,6 @@ enum FieldName {
} }
impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { 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 // Determines whether the given definition is public from the point of view
// of the current item. // of the current item.
fn def_privacy(&self, did: DefId) -> PrivacyResult { fn def_privacy(&self, did: DefId) -> PrivacyResult {
@ -604,75 +599,50 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
return Allowable; return Allowable;
} }
// We now know that there is at least one private member between the let vis = match self.tcx.map.find(node_id) {
// destination and the root. // If this item is a method, then we know for sure that it's an
let mut closest_private_id = node_id; // actual method and not a static method. The reason for this is
loop { // that these cases are only hit in the ExprMethodCall
debug!("privacy - examining {}", self.nodestr(closest_private_id)); // expression, and ExprCall will have its path checked later
let vis = match self.tcx.map.find(closest_private_id) { // (the path of the trait/impl) if it's a static method.
// 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 // With this information, then we can completely ignore all
// that these cases are only hit in the ExprMethodCall // trait methods. The privacy violation would be if the trait
// expression, and ExprCall will have its path checked later // couldn't get imported, not if the method couldn't be used
// (the path of the trait/impl) if it's a static method. // (all trait methods are public).
// //
// With this information, then we can completely ignore all // However, if this is an impl method, then we dictate this
// trait methods. The privacy violation would be if the trait // decision solely based on the privacy of the method
// couldn't get imported, not if the method couldn't be used // invocation.
// (all trait methods are public). Some(ast_map::NodeImplItem(ii)) => {
// match ii.node {
// However, if this is an impl method, then we dictate this hir::ImplItemKind::Const(..) |
// decision solely based on the privacy of the method hir::ImplItemKind::Method(..) => {
// invocation. let imp = self.tcx.map.get_parent_did(node_id);
// FIXME(#10573) is this the right behavior? Why not consider match self.tcx.impl_trait_ref(imp) {
// where the method was defined? Some(..) => hir::Public,
Some(ast_map::NodeImplItem(ii)) => { _ => ii.vis
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
}
} }
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 // This is not a method call, extract the visibility as one
// would normally look at it // would normally look at it
Some(ast_map::NodeItem(it)) => it.vis, Some(ast_map::NodeItem(it)) => it.vis,
Some(ast_map::NodeForeignItem(_)) => { Some(ast_map::NodeForeignItem(_)) => {
self.tcx.map.get_foreign_vis(closest_private_id) self.tcx.map.get_foreign_vis(node_id)
} }
Some(ast_map::NodeVariant(..)) => { _ => hir::Public,
hir::Public // need to move up a level (to the enum) };
} if vis == hir::Public { return Allowable }
_ => 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();
// If we reached the top, then we were public all the way down and if self.private_accessible(node_id) {
// 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) {
Allowable Allowable
} else { } else {
DisallowedBy(closest_private_id) DisallowedBy(node_id)
} }
} }