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

Rewrite VisiblePrivateTypesVisitor

Browse Source
This commit is contained in:
Vadim Petrochenkov 2015-11-21 17:38:17 +03:00
parent 48700be9cb
commit f3f27a5c64
6 changed files with 277 additions and 341 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/doc/reference.md
View File

@ -2372,10 +2372,6 @@ The currently implemented features of the reference compiler are:
Such items should not be allowed by the compiler to exist,
so if you need this there probably is a compiler bug.
* `visible_private_types` - Allows public APIs to expose otherwise private
types, e.g. as the return type of a public function.
This capability may be removed in the future.
* `allow_internal_unstable` - Allows `macro_rules!` macros to be tagged with the
`#[allow_internal_unstable]` attribute, designed
to allow `std` macros to call

491
src/librustc_privacy/lib.rs
View File

@ -1101,342 +1101,173 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> {
}
}
struct VisiblePrivateTypesVisitor<'a, 'tcx: 'a> {
///////////////////////////////////////////////////////////////////////////////
/// SearchInterfaceForPrivateItemsVisitor traverses an item's interface and
/// finds any private components in it.
/// PrivateItemsInPublicInterfacesVisitor ensures there are no private types
/// and traits in public interfaces.
///////////////////////////////////////////////////////////////////////////////
struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> {
tcx: &'a ty::ctxt<'tcx>,
access_levels: &'a AccessLevels,
in_variant: bool,
// Do not report an error when a private type is found
is_quiet: bool,
// Is private component found?
is_public: bool,
}
struct CheckTypeForPrivatenessVisitor<'a, 'b: 'a, 'tcx: 'b> {
inner: &'a VisiblePrivateTypesVisitor<'b, 'tcx>,
/// whether the type refers to private types.
contains_private: bool,
/// whether we've recurred at all (i.e. if we're pointing at the
/// first type on which visit_ty was called).
at_outer_type: bool,
// whether that first type is a public path.
outer_type_is_public_path: bool,
}
impl<'a, 'tcx> VisiblePrivateTypesVisitor<'a, 'tcx> {
fn path_is_private_type(&self, path_id: ast::NodeId) -> bool {
let did = match self.tcx.def_map.borrow().get(&path_id).map(|d| d.full_def()) {
// `int` etc. (None doesn't seem to occur.)
None | Some(def::DefPrimTy(..)) | Some(def::DefSelfTy(..)) => return false,
Some(def) => def.def_id(),
};
// A path can only be private if:
// it's in this crate...
if let Some(node_id) = self.tcx.map.as_local_node_id(did) {
// .. and it corresponds to a private type in the AST (this returns
// None for type parameters)
match self.tcx.map.find(node_id) {
Some(ast_map::NodeItem(ref item)) => item.vis != hir::Public,
Some(_) | None => false,
}
} else {
return false
}
}
fn trait_is_public(&self, trait_id: ast::NodeId) -> bool {
// FIXME: this would preferably be using `exported_items`, but all
// traits are exported currently (see `EmbargoVisitor.exported_trait`)
self.access_levels.is_public(trait_id)
}
fn check_ty_param_bound(&self,
ty_param_bound: &hir::TyParamBound) {
if let hir::TraitTyParamBound(ref trait_ref, _) = *ty_param_bound {
if !self.tcx.sess.features.borrow().visible_private_types &&
self.path_is_private_type(trait_ref.trait_ref.ref_id) {
let span = trait_ref.trait_ref.path.span;
span_err!(self.tcx.sess, span, E0445,
"private trait in exported type parameter bound");
}
}
}
fn item_is_public(&self, id: &ast::NodeId, vis: hir::Visibility) -> bool {
self.access_levels.is_reachable(*id) || vis == hir::Public
}
}
impl<'a, 'b, 'tcx, 'v> Visitor<'v> for CheckTypeForPrivatenessVisitor<'a, 'b, 'tcx> {
impl<'a, 'tcx: 'a, 'v> Visitor<'v> for SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> {
fn visit_ty(&mut self, ty: &hir::Ty) {
if self.is_quiet && !self.is_public {
// We are in quiet mode and a private type is already found, no need to proceed
return
}
if let hir::TyPath(..) = ty.node {
if self.inner.path_is_private_type(ty.id) {
self.contains_private = true;
// found what we're looking for so let's stop
// working.
return
} else if self.at_outer_type {
self.outer_type_is_public_path = true;
let def = self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def();
match def {
def::DefPrimTy(..) | def::DefSelfTy(..) | def::DefTyParam(..) => {
// Public
}
def::DefStruct(def_id) | def::DefTy(def_id, _) |
def::DefTrait(def_id) | def::DefAssociatedTy(def_id, _) => {
// Non-local means public, local needs to be checked
if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) {
if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) {
if item.vis != hir::Public {
if !self.is_quiet {
span_err!(self.tcx.sess, ty.span, E0446,
"private type in exported type signature");
}
self.is_public = false;
}
}
}
}
_ => {}
}
}
self.at_outer_type = false;
intravisit::walk_ty(self, ty)
intravisit::walk_ty(self, ty);
}
// don't want to recurse into [, .. expr]
fn visit_trait_ref(&mut self, trait_ref: &hir::TraitRef) {
if self.is_quiet && !self.is_public {
// We are in quiet mode and a private type is already found, no need to proceed
return
}
// Non-local means public, local needs to be checked
let def_id = self.tcx.trait_ref_to_def_id(trait_ref);
if let Some(node_id) = self.tcx.map.as_local_node_id(def_id) {
if let Some(ast_map::NodeItem(ref item)) = self.tcx.map.find(node_id) {
if item.vis != hir::Public {
if !self.is_quiet {
span_err!(self.tcx.sess, trait_ref.path.span, E0445,
"private trait in exported type parameter bound");
}
self.is_public = false;
}
}
}
intravisit::walk_trait_ref(self, trait_ref);
}
// Don't recurse into function bodies
fn visit_block(&mut self, _: &hir::Block) {}
// Don't recurse into expressions in array sizes or const initializers
fn visit_expr(&mut self, _: &hir::Expr) {}
// Don't recurse into patterns in function arguments
fn visit_pat(&mut self, _: &hir::Pat) {}
}
impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'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))
}
struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> {
tcx: &'a ty::ctxt<'tcx>,
}
fn visit_item(&mut self, item: &hir::Item) {
match item.node {
// contents of a private mod can be reexported, so we need
// to check internals.
hir::ItemMod(_) => {}
// An `extern {}` doesn't introduce a new privacy
// namespace (the contents have their own privacies).
hir::ItemForeignMod(_) => {}
hir::ItemTrait(_, _, ref bounds, _) => {
if !self.trait_is_public(item.id) {
return
}
for bound in bounds.iter() {
self.check_ty_param_bound(bound)
}
}
// impls need some special handling to try to offer useful
// error messages without (too many) false positives
// (i.e. we could just return here to not check them at
// all, or some worse estimation of whether an impl is
// publicly visible).
hir::ItemImpl(_, _, ref g, ref trait_ref, ref self_, ref impl_items) => {
// `impl [... for] Private` is never visible.
let self_contains_private;
// impl [... for] Public<...>, but not `impl [... for]
// Vec<Public>` or `(Public,)` etc.
let self_is_public_path;
// check the properties of the Self type:
{
let mut visitor = CheckTypeForPrivatenessVisitor {
inner: self,
contains_private: false,
at_outer_type: true,
outer_type_is_public_path: false,
};
visitor.visit_ty(&**self_);
self_contains_private = visitor.contains_private;
self_is_public_path = visitor.outer_type_is_public_path;
}
// miscellaneous info about the impl
// `true` iff this is `impl Private for ...`.
let not_private_trait =
trait_ref.as_ref().map_or(true, // no trait counts as public trait
|tr| {
let did = self.tcx.trait_ref_to_def_id(tr);
if let Some(node_id) = self.tcx.map.as_local_node_id(did) {
self.trait_is_public(node_id)
} else {
true // external traits must be public
}
});
// `true` iff this is a trait impl or at least one method is public.
//
// `impl Public { $( fn ...() {} )* }` is not visible.
//
// This is required over just using the methods' privacy
// directly because we might have `impl<T: Foo<Private>> ...`,
// and we shouldn't warn about the generics if all the methods
// are private (because `T` won't be visible externally).
let trait_or_some_public_method =
trait_ref.is_some() ||
impl_items.iter()
.any(|impl_item| {
match impl_item.node {
hir::ImplItemKind::Const(..) |
hir::ImplItemKind::Method(..) => {
self.access_levels.is_reachable(impl_item.id)
}
hir::ImplItemKind::Type(_) => false,
}
});
if !self_contains_private &&
not_private_trait &&
trait_or_some_public_method {
intravisit::walk_generics(self, g);
match *trait_ref {
None => {
for impl_item in impl_items {
// This is where we choose whether to walk down
// further into the impl to check its items. We
// should only walk into public items so that we
// don't erroneously report errors for private
// types in private items.
match impl_item.node {
hir::ImplItemKind::Const(..) |
hir::ImplItemKind::Method(..)
if self.item_is_public(&impl_item.id, impl_item.vis) =>
{
intravisit::walk_impl_item(self, impl_item)
}
hir::ImplItemKind::Type(..) => {
intravisit::walk_impl_item(self, impl_item)
}
_ => {}
}
}
}
Some(ref tr) => {
// Any private types in a trait impl fall into three
// categories.
// 1. mentioned in the trait definition
// 2. mentioned in the type params/generics
// 3. mentioned in the associated types of the impl
//
// Those in 1. can only occur if the trait is in
// this crate and will've been warned about on the
// trait definition (there's no need to warn twice
// so we don't check the methods).
//
// Those in 2. are warned via walk_generics and this
// call here.
intravisit::walk_path(self, &tr.path);
// Those in 3. are warned with this call.
for impl_item in impl_items {
if let hir::ImplItemKind::Type(ref ty) = impl_item.node {
self.visit_ty(ty);
}
}
}
}
} else if trait_ref.is_none() && self_is_public_path {
// impl Public<Private> { ... }. Any public static
// methods will be visible as `Public::foo`.
let mut found_pub_static = false;
for impl_item in impl_items {
match impl_item.node {
hir::ImplItemKind::Const(..) => {
if self.item_is_public(&impl_item.id, impl_item.vis) {
found_pub_static = true;
intravisit::walk_impl_item(self, impl_item);
}
}
hir::ImplItemKind::Method(ref sig, _) => {
if sig.explicit_self.node == hir::SelfStatic &&
self.item_is_public(&impl_item.id, impl_item.vis) {
found_pub_static = true;
intravisit::walk_impl_item(self, impl_item);
}
}
_ => {}
}
}
if found_pub_static {
intravisit::walk_generics(self, g)
}
}
return
}
// `type ... = ...;` can contain private types, because
// we're introducing a new name.
hir::ItemTy(..) => return,
// not at all public, so we don't care
_ if !self.item_is_public(&item.id, item.vis) => {
return;
}
_ => {}
}
// We've carefully constructed it so that if we're here, then
// any `visit_ty`'s will be called on things that are in
// public signatures, i.e. things that we're interested in for
// this visitor.
debug!("VisiblePrivateTypesVisitor entering item {:?}", item);
intravisit::walk_item(self, item);
}
fn visit_generics(&mut self, generics: &hir::Generics) {
for ty_param in generics.ty_params.iter() {
for bound in ty_param.bounds.iter() {
self.check_ty_param_bound(bound)
}
}
for predicate in &generics.where_clause.predicates {
match predicate {
&hir::WherePredicate::BoundPredicate(ref bound_pred) => {
for bound in bound_pred.bounds.iter() {
self.check_ty_param_bound(bound)
}
}
&hir::WherePredicate::RegionPredicate(_) => {}
&hir::WherePredicate::EqPredicate(ref eq_pred) => {
self.visit_ty(&*eq_pred.ty);
}
}
}
}
fn visit_foreign_item(&mut self, item: &hir::ForeignItem) {
if self.access_levels.is_reachable(item.id) {
intravisit::walk_foreign_item(self, item)
}
}
fn visit_ty(&mut self, t: &hir::Ty) {
debug!("VisiblePrivateTypesVisitor checking ty {:?}", t);
if let hir::TyPath(_, ref p) = t.node {
if !self.tcx.sess.features.borrow().visible_private_types &&
self.path_is_private_type(t.id) {
span_err!(self.tcx.sess, p.span, E0446,
"private type in exported type signature");
}
}
intravisit::walk_ty(self, t)
}
fn visit_variant(&mut self, v: &hir::Variant, g: &hir::Generics, item_id: ast::NodeId) {
if self.access_levels.is_reachable(v.node.data.id()) {
self.in_variant = true;
intravisit::walk_variant(self, v, g, item_id);
self.in_variant = false;
}
}
fn visit_struct_field(&mut self, s: &hir::StructField) {
let vis = match s.node.kind {
hir::NamedField(_, vis) | hir::UnnamedField(vis) => vis
impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> {
// A type is considered public if it doesn't contain any private components
fn is_public_ty(&self, ty: &hir::Ty) -> bool {
let mut check = SearchInterfaceForPrivateItemsVisitor {
tcx: self.tcx, is_quiet: true, is_public: true
};
if vis == hir::Public || self.in_variant {
intravisit::walk_struct_field(self, s);
}
check.visit_ty(ty);
check.is_public
}
// 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) {}
// A trait is considered public if it doesn't contain any private components
fn is_public_trait(&self, trait_ref: &hir::TraitRef) -> bool {
let mut check = SearchInterfaceForPrivateItemsVisitor {
tcx: self.tcx, is_quiet: true, is_public: true
};
check.visit_trait_ref(trait_ref);
check.is_public
}
}
impl<'a, 'tcx, 'v> Visitor<'v> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> {
fn visit_item(&mut self, item: &hir::Item) {
let mut check = SearchInterfaceForPrivateItemsVisitor {
tcx: self.tcx, is_quiet: false, is_public: true
};
match item.node {
// Crates are always public
hir::ItemExternCrate(..) => {}
// All nested items are checked by visit_item
hir::ItemMod(..) => {}
// Checked in resolve
hir::ItemUse(..) => {}
// Subitems of these items have inherited publicity
hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemFn(..) |
hir::ItemEnum(..) | hir::ItemTrait(..) | hir::ItemTy(..) => {
if item.vis == hir::Public {
check.visit_item(item);
}
}
// Subitems of foreign modules have their own publicity
hir::ItemForeignMod(ref foreign_mod) => {
for foreign_item in &foreign_mod.items {
if foreign_item.vis == hir::Public {
check.visit_foreign_item(foreign_item);
}
}
}
// Subitems of structs have their own publicity
hir::ItemStruct(ref struct_def, ref generics) => {
if item.vis == hir::Public {
check.visit_generics(generics);
for field in struct_def.fields() {
if field.node.kind.visibility() == hir::Public {
check.visit_struct_field(field);
}
}
}
}
// The interface is empty
hir::ItemDefaultImpl(..) => {}
// An inherent impl is public when its type is public
// Subitems of inherent impls have their own publicity
hir::ItemImpl(_, _, ref generics, None, ref ty, ref impl_items) => {
if self.is_public_ty(ty) {
check.visit_generics(generics);
for impl_item in impl_items {
if impl_item.vis == hir::Public {
check.visit_impl_item(impl_item);
}
}
}
}
// A trait impl is public when both its type and its trait are public
// Subitems of trait impls have inherited publicity
hir::ItemImpl(_, _, ref generics, Some(ref trait_ref), ref ty, ref impl_items) => {
if self.is_public_ty(ty) && self.is_public_trait(trait_ref) {
check.visit_generics(generics);
for impl_item in impl_items {
check.visit_impl_item(impl_item);
}
}
}
}
}
}
pub fn check_crate(tcx: &ty::ctxt,
@ -1473,6 +1304,12 @@ pub fn check_crate(tcx: &ty::ctxt,
tcx.sess.abort_if_errors();
// Check for private types and traits in public interfaces
let mut visitor = PrivateItemsInPublicInterfacesVisitor {
tcx: tcx,
};
krate.visit_all_items(&mut visitor);
// Build up a set of all exported items in the AST. This is a set of all
// items which are reachable from external crates based on visibility.
let mut visitor = EmbargoVisitor {
@ -1491,19 +1328,7 @@ pub fn check_crate(tcx: &ty::ctxt,
}
}
visitor.update(ast::CRATE_NODE_ID, Some(AccessLevel::Public));
let EmbargoVisitor { access_levels, .. } = visitor;
{
let mut visitor = VisiblePrivateTypesVisitor {
tcx: tcx,
access_levels: &access_levels,
in_variant: false,
};
intravisit::walk_crate(&mut visitor, krate);
}
access_levels
visitor.access_levels
}
__build_diagnostic_array! { librustc_privacy, DIAGNOSTICS }

5
src/libsyntax/feature_gate.rs
View File

@ -82,7 +82,7 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Option<u32>, Status
("advanced_slice_patterns", "1.0.0", Some(23121), Active),
("tuple_indexing", "1.0.0", None, Accepted),
("associated_types", "1.0.0", None, Accepted),
("visible_private_types", "1.0.0", Some(29627), Active),
("visible_private_types", "1.0.0", None, Removed),
("slicing_syntax", "1.0.0", None, Accepted),
("box_syntax", "1.0.0", Some(27779), Active),
("placement_in_syntax", "1.0.0", Some(27779), Active),
@ -514,7 +514,6 @@ pub enum AttributeGate {
pub struct Features {
pub unboxed_closures: bool,
pub rustc_diagnostic_macros: bool,
pub visible_private_types: bool,
pub allow_quote: bool,
pub allow_asm: bool,
pub allow_log_syntax: bool,
@ -551,7 +550,6 @@ impl Features {
Features {
unboxed_closures: false,
rustc_diagnostic_macros: false,
visible_private_types: false,
allow_quote: false,
allow_asm: false,
allow_log_syntax: false,
@ -1130,7 +1128,6 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler,
Features {
unboxed_closures: cx.has_feature("unboxed_closures"),
rustc_diagnostic_macros: cx.has_feature("rustc_diagnostic_macros"),
visible_private_types: cx.has_feature("visible_private_types"),
allow_quote: cx.has_feature("quote"),
allow_asm: cx.has_feature("asm"),
allow_log_syntax: cx.has_feature("log_syntax"),

39
src/test/compile-fail/issue-28325.rs Normal file
View File

@ -0,0 +1,39 @@
// 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.
// Checks for private types in public interfaces
mod y {
pub struct Foo { x: u32 }
struct Bar { x: u32 }
impl Foo {
pub fn foo(&self, x: Self, y: Bar) { } //~ ERROR private type in exported type signature
}
}
mod x {
pub struct Foo { pub x: u32 }
struct Bar { _x: u32 }
impl Foo {
pub fn foo(&self, _x: Self, _y: Bar) { } //~ ERROR private type in exported type signature
pub fn bar(&self) -> Bar { Bar { _x: self.x } }
//~^ ERROR private type in exported type signature
}
}
pub fn main() {
let f = x::Foo { x: 4 };
let b = f.bar();
f.foo(x::Foo { x: 5 }, b);
}

54
src/test/compile-fail/issue-28450.rs Normal file
View File

@ -0,0 +1,54 @@
// 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.
// Checks for private types in public interfaces
struct Priv;
pub use self::private::public;
mod private {
pub type Priv = super::Priv; //~ ERROR private type in exported type signature
pub fn public(_x: Priv) {
}
}
struct __CFArray;
pub type CFArrayRef = *const __CFArray; //~ ERROR private type in exported type signature
trait Pointer { type Pointee; }
impl<T> Pointer for *const T { type Pointee = T; }
pub type __CFArrayRevealed = <CFArrayRef as Pointer>::Pointee;
//~^ ERROR private type in exported type signature
type Foo = u8;
pub fn foo(f: Foo) {} //~ ERROR private type in exported type signature
pub trait Exporter {
type Output;
}
pub struct Helper;
pub fn block() -> <Helper as Exporter>::Output {
struct Inner;
impl Inner {
fn poke(&self) { println!("Hello!"); }
}
impl Exporter for Helper {
type Output = Inner; //~ ERROR private type in exported type signature
}
Inner
}
fn main() {
block().poke();
}

25
src/test/run-pass/issue-29668.rs Normal file
View File

@ -0,0 +1,25 @@
// 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.
// Functions can return unnameable types
mod m1 {
mod m2 {
#[derive(Debug)]
pub struct A;
}
use self::m2::A;
pub fn x() -> A { A }
}
fn main() {
let x = m1::x();
println!("{:?}", x);
}