mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

auto merge of #12595 : huonw/rust/pub-vis-typ, r=alexcrichton

Browse Source 
These are types that are in exported type signatures, but are not
exported themselves, e.g.

    struct Foo { ... }

    pub fn bar() -> Foo { ... }

will warn about the Foo.

Such types are not listed in documentation, and cannot be named outside
the crate in which they are declared, which is very user-unfriendly.

cc #10573.
This commit is contained in:
bors 2014-02-28 06:06:31 -08:00
commit 2e51e8d926
20 changed files with 428 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/libextra/workcache.rs
View File

@ -9,6 +9,7 @@
// except according to those terms.
#[allow(missing_doc)];
#[allow(visible_private_types)];
use serialize::json;
use serialize::json::ToJson;

1
src/libgreen/lib.rs
View File

@ -173,6 +173,7 @@
// NB this does *not* include globs, please keep it that way.
#[feature(macro_rules)];
#[allow(visible_private_types)];
use std::mem::replace;
use std::os;

1
src/libnative/io/timer_other.rs
View File

@ -71,6 +71,7 @@ struct Inner {
id: uint,
}
#[allow(visible_private_types)]
pub enum Req {
// Add a new timer to the helper thread.
NewTimer(~Inner),

1
src/libnative/io/timer_timerfd.rs
View File

@ -44,6 +44,7 @@ pub struct Timer {
priv on_worker: bool,
}
#[allow(visible_private_types)]
pub enum Req {
NewTimer(libc::c_int, Chan<()>, bool, imp::itimerspec),
RemoveTimer(libc::c_int, Chan<()>),

2
src/librustc/lib.rs
View File

@ -30,6 +30,8 @@ This API is completely unstable and subject to change.
#[feature(macro_rules, globs, struct_variant, managed_boxes)];
#[feature(quote)];
#[allow(visible_private_types)];
extern crate extra;
extern crate flate;
extern crate arena;

7
src/librustc/middle/lint.rs
View File

@ -98,6 +98,7 @@ pub enum Lint {
UnusedMut,
UnnecessaryAllocation,
DeadCode,
VisiblePrivateTypes,
UnnecessaryTypecast,
MissingDoc,
@ -312,6 +313,12 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
desc: "detect piece of code that will never be used",
default: warn
}),
("visible_private_types",
LintSpec {
lint: VisiblePrivateTypes,
desc: "detect use of private types in exported type signatures",
default: warn
}),
("missing_doc",
LintSpec {

255
src/librustc/middle/privacy.rs
View File

@ -16,6 +16,7 @@ use std::mem::replace;
use collections::{HashSet, HashMap};
use metadata::csearch;
use middle::lint;
use middle::resolve;
use middle::ty;
use middle::typeck::{MethodMap, MethodOrigin, MethodParam};
@ -1169,6 +1170,251 @@ impl SanePrivacyVisitor {
}
}
struct VisiblePrivateTypesVisitor<'a> {
tcx: ty::ctxt,
exported_items: &'a ExportedItems,
public_items: &'a PublicItems,
}
struct CheckTypeForPrivatenessVisitor<'a, 'b> {
inner: &'b VisiblePrivateTypesVisitor<'a>,
/// 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> VisiblePrivateTypesVisitor<'a> {
fn path_is_private_type(&self, path_id: ast::NodeId) -> bool {
let did = match self.tcx.def_map.borrow().get().find_copy(&path_id) {
// `int` etc. (None doesn't seem to occur.)
None | Some(ast::DefPrimTy(..)) => return false,
Some(def) => def_id_of_def(def)
};
// A path can only be private if:
// it's in this crate...
is_local(did) &&
// ... it's not exported (obviously) ...
!self.exported_items.contains(&did.node) &&
// .. and it corresponds to a type in the AST (this returns None for
// type parameters)
self.tcx.map.find(did.node).is_some()
}
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.public_items.contains(&trait_id)
}
}
impl<'a, 'b> Visitor<()> for CheckTypeForPrivatenessVisitor<'a, 'b> {
fn visit_ty(&mut self, ty: &ast::Ty, _: ()) {
match ty.node {
ast::TyPath(_, _, path_id) => {
if self.inner.path_is_private_type(path_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;
}
}
_ => {}
}
self.at_outer_type = false;
visit::walk_ty(self, ty, ())
}
// don't want to recurse into [, .. expr]
fn visit_expr(&mut self, _: &ast::Expr, _: ()) {}
}
impl<'a> Visitor<()> for VisiblePrivateTypesVisitor<'a> {
fn visit_item(&mut self, item: &ast::Item, _: ()) {
match item.node {
// contents of a private mod can be reexported, so we need
// to check internals.
ast::ItemMod(_) => {}
// An `extern {}` doesn't introduce a new privacy
// namespace (the contents have their own privacies).
ast::ItemForeignMod(_) => {}
ast::ItemTrait(..) if !self.trait_is_public(item.id) => return,
// 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
// publically visible.
ast::ItemImpl(ref g, ref trait_ref, self_, ref methods) => {
// `impl [... for] Private` is never visible.
let self_contains_private;
// impl [... for] Public<...>, but not `impl [... for]
// ~[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;
}
// miscellanous 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 = ty::trait_ref_to_def_id(self.tcx, tr);
!is_local(did) || self.trait_is_public(did.node)
});
// `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() ||
methods.iter().any(|m| self.exported_items.contains(&m.id));
if !self_contains_private &&
not_private_trait &&
trait_or_some_public_method {
visit::walk_generics(self, g, ());
match *trait_ref {
None => {
for method in methods.iter() {
visit::walk_method_helper(self, *method, ())
}
}
Some(ref tr) => {
// Any private types in a trait impl fall into two
// categories.
// 1. mentioned in the trait definition
// 2. mentioned in the type params/generics
//
// 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.
visit::walk_trait_ref_helper(self, tr, ())
}
}
} 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 method in methods.iter() {
if method.explicit_self.node == ast::SelfStatic &&
self.exported_items.contains(&method.id) {
found_pub_static = true;
visit::walk_method_helper(self, *method, ());
}
}
if found_pub_static {
visit::walk_generics(self, g, ())
}
}
return
}
// `type ... = ...;` can contain private types, because
// we're introducing a new name.
ast::ItemTy(..) => return,
// not at all public, so we don't care
_ if !self.exported_items.contains(&item.id) => 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.
visit::walk_item(self, item, ());
}
fn visit_foreign_item(&mut self, item: &ast::ForeignItem, _: ()) {
if self.exported_items.contains(&item.id) {
visit::walk_foreign_item(self, item, ())
}
}
fn visit_fn(&mut self,
fk: &visit::FnKind, fd: &ast::FnDecl, b: &ast::Block, s: Span, id: ast::NodeId,
_: ()) {
// needs special handling for methods.
if self.exported_items.contains(&id) {
visit::walk_fn(self, fk, fd, b, s, id, ());
}
}
fn visit_ty(&mut self, t: &ast::Ty, _: ()) {
match t.node {
ast::TyPath(ref p, _, path_id) => {
if self.path_is_private_type(path_id) {
self.tcx.sess.add_lint(lint::VisiblePrivateTypes,
path_id, p.span,
~"private type in exported type signature");
}
}
_ => {}
}
visit::walk_ty(self, t, ())
}
fn visit_variant(&mut self, v: &ast::Variant, g: &ast::Generics, _: ()) {
if self.exported_items.contains(&v.node.id) {
visit::walk_variant(self, v, g, ());
}
}
fn visit_struct_field(&mut self, s: &ast::StructField, _: ()) {
match s.node.kind {
// the only way to get here is by being inside a public
// struct/enum variant, so the only way to have a private
// field is with an explicit `priv`.
ast::NamedField(_, ast::Private) => {}
_ => visit::walk_struct_field(self, s, ())
}
}
// we don't need to introspect into these at all: an
// expression/block context can't possibly contain exported
// things, and neither do view_items. (Making them no-ops stops us
// from traversing the whole AST without having to be super
// careful about our `walk_...` calls above.)
fn visit_view_item(&mut self, _: &ast::ViewItem, _: ()) {}
fn visit_block(&mut self, _: &ast::Block, _: ()) {}
fn visit_expr(&mut self, _: &ast::Expr, _: ()) {}
}
pub fn check_crate(tcx: ty::ctxt,
method_map: &MethodMap,
exp_map2: &resolve::ExportMap2,
@ -1225,5 +1471,14 @@ pub fn check_crate(tcx: ty::ctxt,
}
let EmbargoVisitor { exported_items, public_items, .. } = visitor;
{
let mut visitor = VisiblePrivateTypesVisitor {
tcx: tcx,
exported_items: &exported_items,
public_items: &public_items
};
visit::walk_crate(&mut visitor, krate, ());
}
return (exported_items, public_items);
}

2
src/librustdoc/html/render.rs
View File

@ -99,7 +99,7 @@ pub enum ExternalLocation {
}
/// Different ways an implementor of a trait can be rendered.
enum Implementor {
pub enum Implementor {
/// Paths are displayed specially by omitting the `impl XX for` cruft
PathType(clean::Type),
/// This is the generic representation of a trait implementor, used for

1
src/librustuv/lib.rs
View File

@ -41,6 +41,7 @@ via `close` and `delete` methods.
#[feature(macro_rules)];
#[deny(unused_result, unused_must_use)];
#[allow(visible_private_types)];
#[cfg(test)] extern crate green;

2
src/libstd/libc.rs
View File

@ -1130,7 +1130,7 @@ pub mod types {
Data4: [BYTE, ..8],
}
struct WSAPROTOCOLCHAIN {
pub struct WSAPROTOCOLCHAIN {
ChainLen: c_int,
ChainEntries: [DWORD, ..MAX_PROTOCOL_CHAIN],
}

2
src/libstd/rt/local.rs
View File

@ -24,6 +24,7 @@ pub trait Local<Borrowed> {
unsafe fn try_unsafe_borrow() -> Option<*mut Self>;
}
#[allow(visible_private_types)]
impl Local<local_ptr::Borrowed<Task>> for Task {
#[inline]
fn put(value: ~Task) { unsafe { local_ptr::put(value) } }
@ -127,4 +128,3 @@ mod test {
}
}

1
src/libstd/rt/local_ptr.rs
View File

@ -366,6 +366,7 @@ pub mod native {
#[inline]
#[cfg(not(test))]
#[allow(visible_private_types)]
pub fn maybe_tls_key() -> Option<tls::Key> {
unsafe {
// NB: This is a little racy because, while the key is

2
src/libstd/rt/unwind.rs
View File

@ -280,6 +280,7 @@ fn rust_exception_class() -> uw::_Unwind_Exception_Class {
#[cfg(not(target_arch = "arm"), not(test))]
#[doc(hidden)]
#[allow(visible_private_types)]
pub mod eabi {
use uw = super::libunwind;
use libc::c_int;
@ -333,6 +334,7 @@ pub mod eabi {
// ARM EHABI uses a slightly different personality routine signature,
// but otherwise works the same.
#[cfg(target_arch = "arm", not(test))]
#[allow(visible_private_types)]
pub mod eabi {
use uw = super::libunwind;
use libc::c_int;

21
src/libsync/arc.rs
View File

@ -49,14 +49,15 @@ use std::kinds::marker;
use std::sync::arc::UnsafeArc;
use std::task;
/// As sync::condvar, a mechanism for unlock-and-descheduling and signaling.
pub struct Condvar<'a> {
/// As sync::condvar, a mechanism for unlock-and-descheduling and
/// signaling, for use with the Arc types.
pub struct ArcCondvar<'a> {
priv is_mutex: bool,
priv failed: &'a bool,
priv cond: &'a sync::Condvar<'a>
}
impl<'a> Condvar<'a> {
impl<'a> ArcCondvar<'a> {
/// Atomically exit the associated Arc and block until a signal is sent.
#[inline]
pub fn wait(&self) { self.wait_on(0) }
@ -219,14 +220,14 @@ impl<T:Send> MutexArc<T> {
/// As access(), but with a condvar, as sync::mutex.lock_cond().
#[inline]
pub fn access_cond<U>(&self, blk: |x: &mut T, c: &Condvar| -> U) -> U {
pub fn access_cond<U>(&self, blk: |x: &mut T, c: &ArcCondvar| -> U) -> U {
let state = self.x.get();
unsafe {
(&(*state).lock).lock_cond(|cond| {
check_poison(true, (*state).failed);
let _z = PoisonOnFail::new(&mut (*state).failed);
blk(&mut (*state).data,
&Condvar {is_mutex: true,
&ArcCondvar {is_mutex: true,
failed: &(*state).failed,
cond: cond })
})
@ -345,7 +346,7 @@ impl<T:Freeze + Send> RWArc<T> {
/// As write(), but with a condvar, as sync::rwlock.write_cond().
#[inline]
pub fn write_cond<U>(&self,
blk: |x: &mut T, c: &Condvar| -> U)
blk: |x: &mut T, c: &ArcCondvar| -> U)
-> U {
unsafe {
let state = self.x.get();
@ -353,7 +354,7 @@ impl<T:Freeze + Send> RWArc<T> {
check_poison(false, (*state).failed);
let _z = PoisonOnFail::new(&mut (*state).failed);
blk(&mut (*state).data,
&Condvar {is_mutex: false,
&ArcCondvar {is_mutex: false,
failed: &(*state).failed,
cond: cond})
})
@ -481,7 +482,7 @@ impl<'a, T:Freeze + Send> RWWriteMode<'a, T> {
/// Access the pre-downgrade RWArc in write mode with a condvar.
pub fn write_cond<U>(&mut self,
blk: |x: &mut T, c: &Condvar| -> U)
blk: |x: &mut T, c: &ArcCondvar| -> U)
-> U {
match *self {
RWWriteMode {
@ -491,7 +492,7 @@ impl<'a, T:Freeze + Send> RWWriteMode<'a, T> {
} => {
token.write_cond(|cond| {
unsafe {
let cvar = Condvar {
let cvar = ArcCondvar {
is_mutex: false,
failed: &*poison.flag,
cond: cond
@ -915,7 +916,7 @@ mod tests {
// rwarc gives us extra shared state to help check for the race.
// If you want to see this test fail, go to sync.rs and replace the
// line in RWLock::write_cond() that looks like:
// "blk(&Condvar { order: opt_lock, ..*cond })"
// "blk(&ArcCondvar { order: opt_lock, ..*cond })"
// with just "blk(cond)".
let x = RWArc::new(true);
let (wp, wc) = Chan::new();

2
src/libsync/lib.rs
View File

@ -17,7 +17,7 @@
#[crate_type = "dylib"];
#[license = "MIT/ASL2"];
pub use arc::{Arc, MutexArc, RWArc, RWWriteMode, RWReadMode, Condvar, CowArc};
pub use arc::{Arc, MutexArc, RWArc, RWWriteMode, RWReadMode, ArcCondvar, CowArc};
pub use sync::{Mutex, RWLock, Condvar, Semaphore, RWLockWriteMode,
RWLockReadMode, Barrier, one, mutex};
pub use comm::{DuplexStream, SyncChan, SyncPort, rendezvous};

4
src/libsyntax/abi.rs
View File

@ -48,7 +48,7 @@ pub enum Architecture {
static IntelBits: u32 = (1 << (X86 as uint)) | (1 << (X86_64 as uint));
static ArmBits: u32 = (1 << (Arm as uint));
struct AbiData {
pub struct AbiData {
abi: Abi,
// Name of this ABI as we like it called.
@ -59,7 +59,7 @@ struct AbiData {
abi_arch: AbiArchitecture
}
enum AbiArchitecture {
pub enum AbiArchitecture {
RustArch, // Not a real ABI (e.g., intrinsic)
AllArch, // An ABI that specifies cross-platform defaults (e.g., "C")
Archs(u32) // Multiple architectures (bitset)

1
src/libsyntax/lib.rs
View File

@ -31,6 +31,7 @@ This API is completely unstable and subject to change.
#[feature(quote)];
#[deny(non_camel_case_types)];
#[allow(visible_private_types)];
extern crate serialize;
extern crate term;

13
src/libsyntax/visit.rs
View File

@ -193,9 +193,11 @@ fn walk_explicit_self<E: Clone, V: Visitor<E>>(visitor: &mut V,
}
}
fn walk_trait_ref<E: Clone, V: Visitor<E>>(visitor: &mut V,
trait_ref: &TraitRef,
env: E) {
/// Like with walk_method_helper this doesn't correspond to a method
/// in Visitor, and so it gets a _helper suffix.
pub fn walk_trait_ref_helper<E: Clone, V: Visitor<E>>(visitor: &mut V,
trait_ref: &TraitRef,
env: E) {
visitor.visit_path(&trait_ref.path, trait_ref.ref_id, env)
}
@ -239,7 +241,8 @@ pub fn walk_item<E: Clone, V: Visitor<E>>(visitor: &mut V, item: &Item, env: E)
ref methods) => {
visitor.visit_generics(type_parameters, env.clone());
match *trait_reference {
Some(ref trait_reference) => walk_trait_ref(visitor, trait_reference, env.clone()),
Some(ref trait_reference) => walk_trait_ref_helper(visitor,
trait_reference, env.clone()),
None => ()
}
visitor.visit_ty(typ, env.clone());
@ -459,7 +462,7 @@ pub fn walk_ty_param_bounds<E: Clone, V: Visitor<E>>(visitor: &mut V,
for bound in bounds.iter() {
match *bound {
TraitTyParamBound(ref typ) => {
walk_trait_ref(visitor, typ, env.clone())
walk_trait_ref_helper(visitor, typ, env.clone())
}
RegionTyParamBound => {}
}

1
src/test/compile-fail/lint-dead-code-1.rs
View File

@ -11,6 +11,7 @@
#[no_std];
#[allow(unused_variable)];
#[allow(non_camel_case_types)];
#[allow(visible_private_types)];
#[deny(dead_code)];
#[crate_type="lib"];

129
src/test/compile-fail/lint-visible-private-types.rs Normal file
View File

@ -0,0 +1,129 @@
// Copyright 2014 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.
#[feature(struct_variant)];
#[deny(visible_private_types)];
#[allow(dead_code)];
#[crate_type="lib"];
struct Private<T>;
pub struct Public<T>;
impl Private<Public<int>> {
pub fn a(&self) -> Private<int> { fail!() }
fn b(&self) -> Private<int> { fail!() }
pub fn c() -> Private<int> { fail!() }
fn d() -> Private<int> { fail!() }
}
impl Private<int> {
pub fn e(&self) -> Private<int> { fail!() }
fn f(&self) -> Private<int> { fail!() }
}
impl Public<Private<int>> {
pub fn a(&self) -> Private<int> { fail!() }
fn b(&self) -> Private<int> { fail!() }
pub fn c() -> Private<int> { fail!() } //~ ERROR private type in exported type signature
fn d() -> Private<int> { fail!() }
}
impl Public<int> {
pub fn e(&self) -> Private<int> { fail!() } //~ ERROR private type in exported type signature
fn f(&self) -> Private<int> { fail!() }
}
pub fn x(_: Private<int>) {} //~ ERROR private type in exported type signature
fn y(_: Private<int>) {}
pub struct Foo {
x: Private<int>, //~ ERROR private type in exported type signature
priv y: Private<int>
}
struct Bar {
x: Private<int>,
}
pub enum Baz {
Baz1(Private<int>), //~ ERROR private type in exported type signature
Baz2 {
x: Private<int>, //~ ERROR private type in exported type signature
priv y: Private<int>
},
priv Baz3(Private<int>),
priv Baz4 {
x: Private<int>,
}
}
enum Qux {
Qux1(Private<int>),
Qux2 {
x: Private<int>,
}
}
pub trait PubTrait {
fn foo(&self) -> Private<int> { fail!( )} //~ ERROR private type in exported type signature
fn bar(&self) -> Private<int>; //~ ERROR private type in exported type signature
fn baz() -> Private<int>; //~ ERROR private type in exported type signature
}
impl PubTrait for Public<int> {
fn bar(&self) -> Private<int> { fail!() }
fn baz() -> Private<int> { fail!() }
}
impl PubTrait for Public<Private<int>> {
fn bar(&self) -> Private<int> { fail!() }
fn baz() -> Private<int> { fail!() }
}
impl PubTrait for Private<int> {
fn bar(&self) -> Private<int> { fail!() }
fn baz() -> Private<int> { fail!() }
}
impl PubTrait for (Private<int>,) {
fn bar(&self) -> Private<int> { fail!() }
fn baz() -> Private<int> { fail!() }
}
trait PrivTrait {
fn foo(&self) -> Private<int> { fail!( )}
fn bar(&self) -> Private<int>;
}
impl PrivTrait for Private<int> {
fn bar(&self) -> Private<int> { fail!() }
}
impl PrivTrait for (Private<int>,) {
fn bar(&self) -> Private<int> { fail!() }
}
pub trait ParamTrait<T> {
fn foo() -> T;
}
impl ParamTrait<Private<int>> //~ ERROR private type in exported type signature
for Public<int> {
fn foo() -> Private<int> { fail!() }
}
impl ParamTrait<Private<int>> for Private<int> {
fn foo() -> Private<int> { fail!( )}
}
impl<T: ParamTrait<Private<int>>> //~ ERROR private type in exported type signature
ParamTrait<T> for Public<i8> {
fn foo() -> T { fail!() }
}