X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=src%2Flibrustc_privacy%2Flib.rs;h=226539b8e804a864a1a09b5fa1125d1510bf5bef;hb=13f5fca0f2c72b8af256f052b51bd636b6932c8a;hp=8908dac7a36dd72c9cd355cd6e94e3da09c9a13a;hpb=acea6fc1cb5edf5211ade6ad4a79b119879eab82;p=rust.git diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 8908dac7a36..226539b8e80 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -41,11 +41,8 @@ use rustc::middle::def::{self, Def}; use rustc::middle::def_id::DefId; use rustc::middle::privacy::{AccessLevel, AccessLevels}; -use rustc::middle::privacy::ImportUse::*; -use rustc::middle::privacy::LastPrivate::*; -use rustc::middle::privacy::PrivateDep::*; use rustc::middle::privacy::ExternalExports; -use rustc::middle::ty; +use rustc::middle::ty::{self, TyCtxt}; use rustc::util::nodemap::{NodeMap, NodeSet}; use rustc::front::map as ast_map; @@ -66,7 +63,7 @@ //////////////////////////////////////////////////////////////////////////////// struct ParentVisitor<'a, 'tcx:'a> { - tcx: &'a ty::ctxt<'tcx>, + tcx: &'a TyCtxt<'tcx>, parents: NodeMap, curparent: ast::NodeId, } @@ -147,7 +144,7 @@ fn visit_variant_data(&mut self, s: &hir::VariantData, _: ast::Name, // While we have the id of the struct definition, go ahead and parent // all the fields. for field in s.fields() { - self.parents.insert(field.node.id, self.curparent); + self.parents.insert(field.id, self.curparent); } intravisit::walk_struct_def(self, s) } @@ -158,7 +155,7 @@ fn visit_variant_data(&mut self, s: &hir::VariantData, _: ast::Name, //////////////////////////////////////////////////////////////////////////////// struct EmbargoVisitor<'a, 'tcx: 'a> { - tcx: &'a ty::ctxt<'tcx>, + tcx: &'a TyCtxt<'tcx>, export_map: &'a def::ExportMap, // Accessibility levels for reachable nodes @@ -262,7 +259,7 @@ fn visit_item(&mut self, item: &hir::Item) { for variant in &def.variants { let variant_level = self.update(variant.node.data.id(), item_level); for field in variant.node.data.fields() { - self.update(field.node.id, variant_level); + self.update(field.id, variant_level); } } } @@ -288,8 +285,8 @@ fn visit_item(&mut self, item: &hir::Item) { self.update(def.id(), item_level); } for field in def.fields() { - if field.node.kind.visibility() == hir::Public { - self.update(field.node.id, item_level); + if field.vis == hir::Public { + self.update(field.id, item_level); } } } @@ -347,7 +344,7 @@ fn visit_item(&mut self, item: &hir::Item) { if item_level.is_some() { self.reach().visit_generics(generics); for field in struct_def.fields() { - if self.get(field.node.id).is_some() { + if self.get(field.id).is_some() { self.reach().visit_struct_field(field); } } @@ -475,7 +472,7 @@ fn visit_pat(&mut self, _: &hir::Pat) {} //////////////////////////////////////////////////////////////////////////////// struct PrivacyVisitor<'a, 'tcx: 'a> { - tcx: &'a ty::ctxt<'tcx>, + tcx: &'a TyCtxt<'tcx>, curitem: ast::NodeId, in_foreign: bool, parents: NodeMap, @@ -495,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 { @@ -607,75 +599,50 @@ fn def_privacy(&self, did: DefId) -> PrivacyResult { 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(); - - // 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) { + // 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 self.private_accessible(node_id) { Allowable } else { - DisallowedBy(closest_private_id) + DisallowedBy(node_id) } } @@ -692,32 +659,7 @@ fn local_private_accessible(&self, did: DefId) -> bool { /// 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 { @@ -743,7 +685,6 @@ fn ensure_public(&self, source_did: Option, msg: &str) -> CheckResult { - use rustc_front::hir::Item_::ItemExternCrate; debug!("ensure_public(span={:?}, to_check={:?}, source_did={:?}, msg={:?})", span, to_check, source_did, msg); let def_privacy = self.def_privacy(to_check); @@ -765,20 +706,6 @@ fn ensure_public(&self, let def_id = source_did.unwrap_or(to_check); let node_id = self.tcx.map.as_local_node_id(def_id); - // Warn when using a inaccessible extern crate. - if let Some(node_id) = self.tcx.map.as_local_node_id(to_check) { - match self.tcx.map.get(node_id) { - ast_map::Node::NodeItem(&hir::Item { node: ItemExternCrate(_), name, .. }) => { - self.tcx.sess.add_lint(lint::builtin::INACCESSIBLE_EXTERN_CRATE, - node_id, - span, - format!("extern crate `{}` is private", name)); - return None; - } - _ => {} - } - } - let (err_span, err_msg) = if Some(id) == node_id { return Some((span, format!("{} is private", msg), None)); } else { @@ -835,7 +762,7 @@ fn check_field(&mut self, } 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; } @@ -867,100 +794,6 @@ fn check_static_method(&mut self, name))); } - // Checks that a path is in scope. - fn check_path(&mut self, span: Span, path_id: ast::NodeId, last: ast::Name) { - debug!("privacy - path {}", self.nodestr(path_id)); - let path_res = *self.tcx.def_map.borrow().get(&path_id).unwrap(); - let ck = |tyname: &str| { - let ck_public = |def: DefId| { - debug!("privacy - ck_public {:?}", def); - let origdid = path_res.def_id(); - self.ensure_public(span, - def, - Some(origdid), - &format!("{} `{}`", tyname, last)) - }; - - match path_res.last_private { - LastMod(AllPublic) => {}, - LastMod(DependsOn(def)) => { - self.report_error(ck_public(def)); - }, - LastImport { value_priv, - value_used: check_value, - type_priv, - type_used: check_type } => { - // This dance with found_error is because we don't want to - // report a privacy error twice for the same directive. - let found_error = match (type_priv, check_type) { - (Some(DependsOn(def)), Used) => { - !self.report_error(ck_public(def)) - }, - _ => false, - }; - if !found_error { - match (value_priv, check_value) { - (Some(DependsOn(def)), Used) => { - self.report_error(ck_public(def)); - }, - _ => {}, - } - } - // If an import is not used in either namespace, we still - // want to check that it could be legal. Therefore we check - // in both namespaces and only report an error if both would - // be illegal. We only report one error, even if it is - // illegal to import from both namespaces. - match (value_priv, check_value, type_priv, check_type) { - (Some(p), Unused, None, _) | - (None, _, Some(p), Unused) => { - let p = match p { - AllPublic => None, - DependsOn(def) => ck_public(def), - }; - if p.is_some() { - self.report_error(p); - } - }, - (Some(v), Unused, Some(t), Unused) => { - let v = match v { - AllPublic => None, - DependsOn(def) => ck_public(def), - }; - let t = match t { - AllPublic => None, - DependsOn(def) => ck_public(def), - }; - if let (Some(_), Some(t)) = (v, t) { - self.report_error(Some(t)); - } - }, - _ => {}, - } - }, - } - }; - // FIXME(#12334) Imports can refer to definitions in both the type and - // value namespaces. The privacy information is aware of this, but the - // 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"), - _ => {} - } - } - // Checks that a method is in scope. fn check_method(&mut self, span: Span, method_def_id: DefId, name: ast::Name) { @@ -1036,7 +869,7 @@ fn visit_expr(&mut self, expr: &hir::Expr) { _ => 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, @@ -1102,25 +935,6 @@ fn visit_foreign_item(&mut self, fi: &hir::ForeignItem) { intravisit::walk_foreign_item(self, fi); self.in_foreign = false; } - - fn visit_path(&mut self, path: &hir::Path, id: ast::NodeId) { - if !path.segments.is_empty() { - self.check_path(path.span, id, path.segments.last().unwrap().identifier.name); - intravisit::walk_path(self, path); - } - } - - fn visit_path_list_item(&mut self, prefix: &hir::Path, item: &hir::PathListItem) { - let name = if let hir::PathListIdent { name, .. } = item.node { - name - } else if !prefix.segments.is_empty() { - prefix.segments.last().unwrap().identifier.name - } else { - self.tcx.sess.bug("`self` import in an import list with empty prefix"); - }; - self.check_path(item.span, item.node.id(), name); - intravisit::walk_path_list_item(self, prefix, item); - } } //////////////////////////////////////////////////////////////////////////////// @@ -1128,44 +942,20 @@ fn visit_path_list_item(&mut self, prefix: &hir::Path, item: &hir::PathListItem) //////////////////////////////////////////////////////////////////////////////// struct SanePrivacyVisitor<'a, 'tcx: 'a> { - tcx: &'a ty::ctxt<'tcx>, - in_block: bool, + tcx: &'a TyCtxt<'tcx>, } impl<'a, 'tcx, 'v> Visitor<'v> for SanePrivacyVisitor<'a, 'tcx> { - /// We want to visit items in the context of their containing - /// module and so forth, so supply a crate for doing a deep walk. - fn visit_nested_item(&mut self, item: hir::ItemId) { - self.visit_item(self.tcx.map.expect_item(item.id)) - } - fn visit_item(&mut self, item: &hir::Item) { self.check_sane_privacy(item); - if self.in_block { - self.check_all_inherited(item); - } - - let orig_in_block = self.in_block; - - // Modules turn privacy back on, otherwise we inherit - self.in_block = if let hir::ItemMod(..) = item.node { false } else { orig_in_block }; - intravisit::walk_item(self, item); - self.in_block = orig_in_block; - } - - fn visit_block(&mut self, b: &'v hir::Block) { - let orig_in_block = replace(&mut self.in_block, true); - intravisit::walk_block(self, b); - self.in_block = orig_in_block; } } impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> { - /// Validates all of the visibility qualifiers placed on the item given. This - /// ensures that there are no extraneous qualifiers that don't actually do - /// anything. In theory these qualifiers wouldn't parse, but that may happen - /// later on down the road... + /// Validate that items that shouldn't have visibility qualifiers don't have them. + /// Such qualifiers can be set by syntax extensions even if the parser doesn't allow them, + /// so we check things like variant fields too. fn check_sane_privacy(&self, item: &hir::Item) { let check_inherited = |sp, vis, note: &str| { if vis != hir::Inherited { @@ -1179,13 +969,12 @@ fn check_sane_privacy(&self, item: &hir::Item) { }; match item.node { - // implementations of traits don't need visibility qualifiers because - // that's controlled by having the trait in scope. hir::ItemImpl(_, _, _, Some(..), _, ref impl_items) => { check_inherited(item.span, item.vis, "visibility qualifiers have no effect on trait impls"); for impl_item in impl_items { - check_inherited(impl_item.span, impl_item.vis, ""); + check_inherited(impl_item.span, impl_item.vis, + "visibility qualifiers have no effect on trait impl items"); } } hir::ItemImpl(_, _, _, None, _, _) => { @@ -1200,41 +989,15 @@ fn check_sane_privacy(&self, item: &hir::Item) { check_inherited(item.span, item.vis, "place qualifiers on individual functions instead"); } - hir::ItemStruct(..) | hir::ItemEnum(..) | hir::ItemTrait(..) | - hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) | - hir::ItemMod(..) | hir::ItemExternCrate(..) | - hir::ItemUse(..) | hir::ItemTy(..) => {} - } - } - - /// When inside of something like a function or a method, visibility has no - /// control over anything so this forbids any mention of any visibility - fn check_all_inherited(&self, item: &hir::Item) { - let check_inherited = |sp, vis| { - if vis != hir::Inherited { - span_err!(self.tcx.sess, sp, E0447, - "visibility has no effect inside functions or block expressions"); - } - }; - - check_inherited(item.span, item.vis); - match item.node { - hir::ItemImpl(_, _, _, _, _, ref impl_items) => { - for impl_item in impl_items { - check_inherited(impl_item.span, impl_item.vis); - } - } - hir::ItemForeignMod(ref fm) => { - for fi in &fm.items { - check_inherited(fi.span, fi.vis); - } - } - hir::ItemStruct(ref vdata, _) => { - for f in vdata.fields() { - check_inherited(f.span, f.node.kind.visibility()); + hir::ItemEnum(ref def, _) => { + for variant in &def.variants { + for field in variant.node.data.fields() { + check_inherited(field.span, field.vis, + "visibility qualifiers have no effect on variant fields"); + } } } - hir::ItemDefaultImpl(..) | hir::ItemEnum(..) | hir::ItemTrait(..) | + hir::ItemStruct(..) | hir::ItemTrait(..) | hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) | hir::ItemMod(..) | hir::ItemExternCrate(..) | hir::ItemUse(..) | hir::ItemTy(..) => {} @@ -1250,7 +1013,7 @@ fn check_all_inherited(&self, item: &hir::Item) { /////////////////////////////////////////////////////////////////////////////// struct ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx: 'a> { - tcx: &'a ty::ctxt<'tcx>, + tcx: &'a TyCtxt<'tcx>, access_levels: &'a AccessLevels, in_variant: bool, // set of errors produced by this obsolete visitor @@ -1565,10 +1328,7 @@ fn visit_variant(&mut self, v: &hir::Variant, g: &hir::Generics, item_id: ast::N } fn visit_struct_field(&mut self, s: &hir::StructField) { - let vis = match s.node.kind { - hir::NamedField(_, vis) | hir::UnnamedField(vis) => vis - }; - if vis == hir::Public || self.in_variant { + if s.vis == hir::Public || self.in_variant { intravisit::walk_struct_field(self, s); } } @@ -1592,7 +1352,7 @@ fn visit_expr(&mut self, _: &hir::Expr) {} /////////////////////////////////////////////////////////////////////////////// struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { - tcx: &'a ty::ctxt<'tcx>, + tcx: &'a TyCtxt<'tcx>, // Do not report an error when a private type is found is_quiet: bool, // Is private component found? @@ -1723,7 +1483,7 @@ fn visit_pat(&mut self, _: &hir::Pat) {} } struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> { - tcx: &'a ty::ctxt<'tcx>, + tcx: &'a TyCtxt<'tcx>, old_error_set: &'a NodeSet, } @@ -1779,7 +1539,7 @@ fn visit_item(&mut self, item: &hir::Item) { if item.vis == hir::Public { check.visit_generics(generics); for field in struct_def.fields() { - if field.node.kind.visibility() == hir::Public { + if field.vis == hir::Public { check.visit_struct_field(field); } } @@ -1813,7 +1573,7 @@ fn visit_item(&mut self, item: &hir::Item) { } } -pub fn check_crate(tcx: &ty::ctxt, +pub fn check_crate(tcx: &TyCtxt, export_map: &def::ExportMap, external_exports: ExternalExports) -> AccessLevels { @@ -1821,13 +1581,9 @@ pub fn check_crate(tcx: &ty::ctxt, let krate = tcx.map.krate(); - // Sanity check to make sure that all privacy usage and controls are - // reasonable. - let mut visitor = SanePrivacyVisitor { - tcx: tcx, - in_block: false, - }; - intravisit::walk_crate(&mut visitor, krate); + // Sanity check to make sure that all privacy usage is reasonable. + let mut visitor = SanePrivacyVisitor { tcx: tcx }; + krate.visit_all_items(&mut visitor); // Figure out who everyone's parent is let mut visitor = ParentVisitor {