OpenE2K
/
rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #29291 - petrochenkov:privacy, r=alexcrichton 

The public set is expanded with trait items, impls and their items, foreign items, exported macros, variant fields, i.e. all the missing parts. Now it's a subset of the exported set.
This is needed for https://github.com/rust-lang/rust/pull/29083 because stability annotation pass uses the public set and all things listed above need to be annotated.
Rustdoc can now be migrated to the public set as well, I guess.

Exported set is now slightly more correct with regard to exported items in blocks - 1) blocks in foreign items are considered and 2) publicity is not inherited from the block's parent - if a function is public it doesn't mean structures defined in its body are public.

r? @alexcrichton or maybe someone else
This commit is contained in:
bors 2015-11-02 23:38:49 +00:00
parent a1fd944eb8 ab7b3456d0
commit e2bb53ca52
7 changed files with 218 additions and 144 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/librustc/middle/reachable.rs
View File

@ -125,16 +125,11 @@ impl<'a, 'tcx, 'v> Visitor<'v> for ReachableContext<'a, 'tcx> {
hir::ExprMethodCall(..) => {
let method_call = ty::MethodCall::expr(expr.id);
let def_id = self.tcx.tables.borrow().method_map[&method_call].def_id;
match self.tcx.impl_or_trait_item(def_id).container() {
ty::ImplContainer(_) => {
if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) {
if self.def_id_represents_local_inlined_item(def_id) {
self.worklist.push(node_id)
}
self.reachable_symbols.insert(node_id);
}
if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) {
if self.def_id_represents_local_inlined_item(def_id) {
self.worklist.push(node_id)
}
ty::TraitContainer(_) => {}
self.reachable_symbols.insert(node_id);
}
}
_ => {}
@ -228,14 +223,8 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> {
continue
}
match self.tcx.map.find(search_item) {
Some(ref item) => self.propagate_node(item, search_item),
None if search_item == ast::CRATE_NODE_ID => {}
None => {
self.tcx.sess.bug(&format!("found unmapped ID in worklist: \
{}",
search_item))
}
if let Some(ref item) = self.tcx.map.find(search_item) {
self.propagate_node(item, search_item);
}
}
}

4
src/librustc/middle/stability.rs
View File

@ -217,7 +217,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for Annotator<'a, 'tcx> {
fn visit_impl_item(&mut self, ii: &hir::ImplItem) {
self.annotate(ii.id, true, &ii.attrs, ii.span,
|v| visit::walk_impl_item(v, ii), true);
|v| visit::walk_impl_item(v, ii), false);
}
fn visit_variant(&mut self, var: &Variant, g: &'v Generics, item_id: NodeId) {
@ -227,7 +227,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for Annotator<'a, 'tcx> {
fn visit_struct_field(&mut self, s: &StructField) {
self.annotate(s.node.id, true, &s.node.attrs, s.span,
|v| visit::walk_struct_field(v, s), true);
|v| visit::walk_struct_field(v, s), !s.node.kind.is_unnamed());
}
fn visit_foreign_item(&mut self, i: &hir::ForeignItem) {

6
src/librustc_front/hir.rs
View File

@ -1148,6 +1148,12 @@ impl StructFieldKind {
NamedField(..) => false,
}
}
pub fn visibility(&self) -> Visibility {
match *self {
NamedField(_, vis) | UnnamedField(vis) => vis
}
}
}
/// Fields and Ids of enum variants and structs

275
src/librustc_privacy/lib.rs
View File

@ -162,7 +162,7 @@ struct EmbargoVisitor<'a, 'tcx: 'a> {
// This is a list of all exported items in the AST. An exported item is any
// function/method/item which is usable by external crates. This essentially
// means that the result is "public all the way down", but the "path down"
// may jump across private boundaries through reexport statements.
// may jump across private boundaries through reexport statements or type aliases.
exported_items: ExportedItems,
// This sets contains all the destination nodes which are publicly
@ -172,168 +172,162 @@ struct EmbargoVisitor<'a, 'tcx: 'a> {
// destination must also be exported.
reexports: NodeSet,
// Items that are directly public without help of reexports or type aliases.
// These two fields are closely related to one another in that they are only
// used for generation of the 'PublicItems' set, not for privacy checking at
// all
// used for generation of the `public_items` set, not for privacy checking at
// all. Invariant: at any moment public items are a subset of exported items.
public_items: PublicItems,
prev_public: bool,
}
impl<'a, 'tcx> EmbargoVisitor<'a, 'tcx> {
// There are checks inside of privacy which depend on knowing whether a
// trait should be exported or not. The two current consumers of this are:
//
// 1. Should default methods of a trait be exported?
// 2. Should the methods of an implementation of a trait be exported?
//
// The answer to both of these questions partly rely on whether the trait
// itself is exported or not. If the trait is somehow exported, then the
// answers to both questions must be yes. Right now this question involves
// more analysis than is currently done in rustc, so we conservatively
// answer "yes" so that all traits need to be exported.
fn exported_trait(&self, _id: ast::NodeId) -> bool {
true
// Returns tuple (is_public, is_exported) for a type
fn is_public_exported_ty(&self, ty: &hir::Ty) -> (bool, bool) {
if let hir::TyPath(..) = ty.node {
match self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def() {
def::DefPrimTy(..) | def::DefSelfTy(..) => (true, true),
def => {
if let Some(node_id) = self.tcx.map.as_local_node_id(def.def_id()) {
(self.public_items.contains(&node_id),
self.exported_items.contains(&node_id))
} else {
(true, true)
}
}
}
} else {
(true, true)
}
}
// Returns tuple (is_public, is_exported) for a trait
fn is_public_exported_trait(&self, trait_ref: &hir::TraitRef) -> (bool, bool) {
let did = self.tcx.trait_ref_to_def_id(trait_ref);
if let Some(node_id) = self.tcx.map.as_local_node_id(did) {
(self.public_items.contains(&node_id), self.exported_items.contains(&node_id))
} else {
(true, true)
}
}
fn maybe_insert_id(&mut self, id: ast::NodeId) {
if self.prev_public {
self.public_items.insert(id);
}
if self.prev_exported {
self.exported_items.insert(id);
}
}
}
impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> {
fn visit_item(&mut self, item: &hir::Item) {
let orig_all_pub = self.prev_public;
self.prev_public = orig_all_pub && item.vis == hir::Public;
if self.prev_public {
self.public_items.insert(item.id);
}
let orig_all_public = self.prev_public;
let orig_all_exported = self.prev_exported;
match item.node {
// impls/extern blocks do not break the "public chain" because they
// cannot have visibility qualifiers on them anyway
// cannot have visibility qualifiers on them anyway. They are also not
// added to public/exported sets based on inherited publicity.
hir::ItemImpl(..) | hir::ItemDefaultImpl(..) | hir::ItemForeignMod(..) => {}
// Traits are a little special in that even if they themselves are
// not public they may still be exported.
hir::ItemTrait(..) => {
self.prev_exported = self.exported_trait(item.id);
}
// Private by default, hence we only retain the "public chain" if
// `pub` is explicitly listed.
_ => {
self.prev_exported =
(orig_all_exported && item.vis == hir::Public) ||
self.reexports.contains(&item.id);
self.prev_public = self.prev_public && item.vis == hir::Public;
self.prev_exported = (self.prev_exported && item.vis == hir::Public) ||
self.reexports.contains(&item.id);
self.maybe_insert_id(item.id);
}
}
let public_first = self.prev_exported &&
self.exported_items.insert(item.id);
match item.node {
// Enum variants inherit from their parent, so if the enum is
// public all variants are public unless they're explicitly priv
hir::ItemEnum(ref def, _) if public_first => {
// public all variants are public
hir::ItemEnum(ref def, _) => {
for variant in &def.variants {
self.exported_items.insert(variant.node.data.id());
self.public_items.insert(variant.node.data.id());
self.maybe_insert_id(variant.node.data.id());
for field in variant.node.data.fields() {
// Variant fields are always public
self.maybe_insert_id(field.node.id);
}
}
}
// Implementations are a little tricky to determine what's exported
// out of them. Here's a few cases which are currently defined:
//
// * Impls for private types do not need to export their methods
// (either public or private methods)
//
// * Impls for public types only have public methods exported
//
// * Public trait impls for public types must have all methods
// exported.
//
// * Private trait impls for public types can be ignored
//
// * Public trait impls for private types have their methods
// exported. I'm not entirely certain that this is the correct
// thing to do, but I have seen use cases of where this will cause
// undefined symbols at linkage time if this case is not handled.
//
// * Private trait impls for private types can be completely ignored
hir::ItemImpl(_, _, _, _, ref ty, ref impl_items) => {
let public_ty = match ty.node {
hir::TyPath(..) => {
match self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def() {
def::DefPrimTy(..) => true,
def::DefSelfTy(..) => true,
def => {
let did = def.def_id();
if let Some(node_id) = self.tcx.map.as_local_node_id(did) {
self.exported_items.contains(&node_id)
} else {
true
}
}
}
}
_ => true,
};
let tr = self.tcx.impl_trait_ref(self.tcx.map.local_def_id(item.id));
let public_trait = tr.clone().map_or(false, |tr| {
if let Some(node_id) = self.tcx.map.as_local_node_id(tr.def_id) {
self.exported_items.contains(&node_id)
} else {
true
}
});
// Public items in inherent impls for public/exported types are public/exported
// Inherent impls themselves are not public/exported, they are nothing more than
// containers for other items
hir::ItemImpl(_, _, _, None, ref ty, ref impl_items) => {
let (public_ty, exported_ty) = self.is_public_exported_ty(&ty);
if public_ty || public_trait {
for impl_item in impl_items {
match impl_item.node {
hir::ConstImplItem(..) => {
if (public_ty && impl_item.vis == hir::Public)
|| tr.is_some() {
self.exported_items.insert(impl_item.id);
}
}
hir::MethodImplItem(ref sig, _) => {
let meth_public = match sig.explicit_self.node {
hir::SelfStatic => public_ty,
_ => true,
} && impl_item.vis == hir::Public;
if meth_public || tr.is_some() {
self.exported_items.insert(impl_item.id);
}
}
hir::TypeImplItem(_) => {}
for impl_item in impl_items {
if impl_item.vis == hir::Public {
if public_ty {
self.public_items.insert(impl_item.id);
}
if exported_ty {
self.exported_items.insert(impl_item.id);
}
}
}
}
// Default methods on traits are all public so long as the trait
// is public
hir::ItemTrait(_, _, _, ref trait_items) if public_first => {
// It's not known until monomorphization if a trait impl item should be reachable
// from external crates or not. So, we conservatively mark all of them exported and
// the reachability pass (middle::reachable) marks all exported items as reachable.
// For example of private trait impl for private type that should be reachable see
// src/test/auxiliary/issue-11225-3.rs
hir::ItemImpl(_, _, _, Some(ref trait_ref), ref ty, ref impl_items) => {
let (public_ty, _exported_ty) = self.is_public_exported_ty(&ty);
let (public_trait, _exported_trait) = self.is_public_exported_trait(trait_ref);
if public_ty && public_trait {
self.public_items.insert(item.id);
}
self.exported_items.insert(item.id);
for impl_item in impl_items {
if public_ty && public_trait {
self.public_items.insert(impl_item.id);
}
self.exported_items.insert(impl_item.id);
}
}
// Default trait impls are public/exported for public/exported traits
hir::ItemDefaultImpl(_, ref trait_ref) => {
let (public_trait, exported_trait) = self.is_public_exported_trait(trait_ref);
if public_trait {
self.public_items.insert(item.id);
}
if exported_trait {
self.exported_items.insert(item.id);
}
}
// Default methods on traits are all public/exported so long as the trait
// is public/exported
hir::ItemTrait(_, _, _, ref trait_items) => {
for trait_item in trait_items {
debug!("trait item {}", trait_item.id);
self.exported_items.insert(trait_item.id);
self.maybe_insert_id(trait_item.id);
}
}
// Struct constructors are public if the struct is all public.
hir::ItemStruct(ref def, _) if public_first => {
hir::ItemStruct(ref def, _) => {
if !def.is_struct() {
self.exported_items.insert(def.id());
self.maybe_insert_id(def.id());
}
// fields can be public or private, so lets check
for field in def.fields() {
let vis = match field.node.kind {
hir::NamedField(_, vis) | hir::UnnamedField(vis) => vis
};
if vis == hir::Public {
self.public_items.insert(field.node.id);
// Struct fields can be public or private, so lets check
if field.node.kind.visibility() == hir::Public {
self.maybe_insert_id(field.node.id);
}
}
}
hir::ItemTy(ref ty, _) if public_first => {
hir::ItemTy(ref ty, _) if self.prev_exported => {
if let hir::TyPath(..) = ty.node {
match self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def() {
def::DefPrimTy(..) | def::DefSelfTy(..) | def::DefTyParam(..) => {},
@ -347,19 +341,41 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> {
}
}
hir::ItemForeignMod(ref foreign_mod) => {
for foreign_item in &foreign_mod.items {
let public = self.prev_public && foreign_item.vis == hir::Public;
let exported = (self.prev_exported && foreign_item.vis == hir::Public) ||
self.reexports.contains(&foreign_item.id);
if public {
self.public_items.insert(foreign_item.id);
}
if exported {
self.exported_items.insert(foreign_item.id);
}
}
}
_ => {}
}
visit::walk_item(self, item);
self.prev_public = orig_all_public;
self.prev_exported = orig_all_exported;
self.prev_public = orig_all_pub;
}
fn visit_foreign_item(&mut self, a: &hir::ForeignItem) {
if (self.prev_exported && a.vis == hir::Public) || self.reexports.contains(&a.id) {
self.exported_items.insert(a.id);
}
fn visit_block(&mut self, b: &'v hir::Block) {
let orig_all_public = replace(&mut self.prev_public, false);
let orig_all_exported = replace(&mut self.prev_exported, false);
// Blocks can have exported and public items, for example impls, but they always
// start as non-public and non-exported regardless of publicity of a function,
// constant, type, field, etc. in which this block resides
visit::walk_block(self, b);
self.prev_public = orig_all_public;
self.prev_exported = orig_all_exported;
}
fn visit_mod(&mut self, m: &hir::Mod, _sp: Span, id: ast::NodeId) {
@ -375,6 +391,10 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> {
}
visit::walk_mod(self, m)
}
fn visit_macro_def(&mut self, md: &'v hir::MacroDef) {
self.maybe_insert_id(md.id);
}
}
////////////////////////////////////////////////////////////////////////////////
@ -1460,11 +1480,13 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> {
}
}
// we don't need to introspect into these at all: an
// expression/block context can't possibly contain exported things.
// (Making them no-ops stops us from traversing the whole AST without
// having to be super careful about our `walk_...` calls above.)
// FIXME(#29524): Unfortunately this ^^^ is not true, blocks can contain
// exported items (e.g. impls) and actual code in rustc itself breaks
// if we don't traverse blocks in `EmbargoVisitor`
fn visit_block(&mut self, _: &hir::Block) {}
fn visit_expr(&mut self, _: &hir::Expr) {}
}
@ -1514,9 +1536,12 @@ pub fn check_crate(tcx: &ty::ctxt,
prev_public: true,
};
loop {
let before = visitor.exported_items.len();
let before = (visitor.exported_items.len(), visitor.public_items.len(),
visitor.reexports.len());
visit::walk_crate(&mut visitor, krate);
if before == visitor.exported_items.len() {
let after = (visitor.exported_items.len(), visitor.public_items.len(),
visitor.reexports.len());
if after == before {
break
}
}

6
src/libsyntax/ast.rs
View File

@ -1726,6 +1726,12 @@ impl StructFieldKind {
NamedField(..) => false,
}
}
pub fn visibility(&self) -> Visibility {
match *self {
NamedField(_, vis) | UnnamedField(vis) => vis
}
}
}
/// Fields and Ids of enum variants and structs

29
src/test/auxiliary/issue-11225-3.rs Normal file
View File

@ -0,0 +1,29 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
trait PrivateTrait {
fn private_trait_method(&self);
}
struct PrivateStruct;
impl PrivateStruct {
fn private_inherent_method(&self) { }
}
impl PrivateTrait for PrivateStruct {
fn private_trait_method(&self) { }
}
#[inline]
pub fn public_generic_function() {
PrivateStruct.private_trait_method();
PrivateStruct.private_inherent_method();
}

19
src/test/run-pass/issue-11225-3.rs Normal file
View File

@ -0,0 +1,19 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// aux-build:issue-11225-3.rs
// pretty-expanded FIXME #23616
extern crate issue_11225_3;
pub fn main() {
issue_11225_3::public_generic_function();
}