Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

librustc: Forbid private types in public APIs. #17401

Merged
merged 1 commit into from
Sep 23, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libcore/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub enum Void { }
pub trait Any: AnyPrivate {}

/// An inner trait to ensure that only this module can call `get_type_id()`.
trait AnyPrivate {
pub trait AnyPrivate {
/// Get the `TypeId` of `self`
fn get_type_id(&self) -> TypeId;
}
Expand Down
1 change: 0 additions & 1 deletion src/libcore/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2178,7 +2178,6 @@ pub type Iterate<'a, T> = Unfold<'a, T, IterateState<'a, T>>;

/// Creates a new iterator that produces an infinite sequence of
/// repeated applications of the given function `f`.
#[allow(visible_private_types)]
pub fn iterate<'a, T: Clone>(seed: T, f: |T|: 'a -> T) -> Iterate<'a, T> {
Unfold::new((f, Some(seed), true), |st| {
let &(ref mut f, ref mut val, ref mut first) = st;
Expand Down
2 changes: 1 addition & 1 deletion src/libdebug/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ macro_rules! try( ($me:expr, $e:expr) => (

/// Representations

trait Repr {
pub trait Repr {
fn write_repr(&self, writer: &mut io::Writer) -> io::IoResult<()>;
}

Expand Down
4 changes: 2 additions & 2 deletions src/libgreen/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@

// NB this does *not* include globs, please keep it that way.
#![feature(macro_rules, phase, default_type_params)]
#![allow(visible_private_types, deprecated)]
#![allow(deprecated)]

#[cfg(test)] #[phase(plugin, link)] extern crate log;
#[cfg(test)] extern crate rustuv;
Expand Down Expand Up @@ -385,7 +385,7 @@ pub struct SchedPool {
/// keep track of how many tasks are currently running in the pool and then
/// sending on a channel once the entire pool has been drained of all tasks.
#[deriving(Clone)]
struct TaskState {
pub struct TaskState {
cnt: Arc<AtomicUint>,
done: Sender<()>,
}
Expand Down
3 changes: 1 addition & 2 deletions src/libnative/io/timer_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,14 @@ pub struct Timer {
inner: Option<Box<Inner>>,
}

struct Inner {
pub struct Inner {
cb: Option<Box<rtio::Callback + Send>>,
interval: u64,
repeat: bool,
target: u64,
id: uint,
}

#[allow(visible_private_types)]
pub enum Req {
// Add a new timer to the helper thread.
NewTimer(Box<Inner>),
Expand Down
1 change: 0 additions & 1 deletion src/libregex/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

// Enable this to squash warnings due to exporting pieces of the representation
// for use with the regex! macro. See lib.rs for explanation.
#![allow(visible_private_types)]

use std::cmp;
use parse;
Expand Down
4 changes: 1 addition & 3 deletions src/libregex/re.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ pub fn is_match(regex: &str, text: &str) -> Result<bool, parse::Error> {
/// More details about the `regex!` macro can be found in the `regex` crate
/// documentation.
#[deriving(Clone)]
#[allow(visible_private_types)]
pub enum Regex {
// The representation of `Regex` is exported to support the `regex!`
// syntax extension. Do not rely on it.
Expand Down Expand Up @@ -516,7 +515,6 @@ impl Regex {
}

#[doc(hidden)]
#[allow(visible_private_types)]
#[experimental]
pub fn names_iter<'a>(&'a self) -> NamesIter<'a> {
match *self {
Expand All @@ -534,7 +532,7 @@ impl Regex {

}

enum NamesIter<'a> {
pub enum NamesIter<'a> {
NamesIterNative(::std::slice::Items<'a, Option<&'static str>>),
NamesIterDynamic(::std::slice::Items<'a, Option<String>>)
}
Expand Down
4 changes: 0 additions & 4 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1601,9 +1601,6 @@ declare_lint!(pub DEAD_ASSIGNMENT, Warn,
declare_lint!(pub DEAD_CODE, Warn,
"detect piece of code that will never be used")

declare_lint!(pub VISIBLE_PRIVATE_TYPES, Warn,
"detect use of private types in exported type signatures")

declare_lint!(pub UNREACHABLE_CODE, Warn,
"detects unreachable code")

Expand Down Expand Up @@ -1636,7 +1633,6 @@ impl LintPass for HardwiredLints {
UNUSED_VARIABLE,
DEAD_ASSIGNMENT,
DEAD_CODE,
VISIBLE_PRIVATE_TYPES,
UNREACHABLE_CODE,
WARNINGS,
UNKNOWN_FEATURES,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ pub fn deref_kind(tcx: &ty::ctxt, t: ty::t) -> deref_kind {
}
}

trait ast_node {
pub trait ast_node {
fn id(&self) -> ast::NodeId;
fn span(&self) -> Span;
}
Expand Down
66 changes: 52 additions & 14 deletions src/librustc/middle/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use std::mem::replace;

use metadata::csearch;
use middle::def;
use lint;
use middle::resolve;
use middle::ty;
use middle::typeck::{MethodCall, MethodMap, MethodOrigin, MethodParam, MethodTypeParam};
Expand Down Expand Up @@ -1289,19 +1288,38 @@ impl<'a, 'tcx> VisiblePrivateTypesVisitor<'a, 'tcx> {
};
// 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()
if !is_local(did) {
return false
}
// .. and it corresponds to a private type in the AST (this returns
// None for type parameters)
match self.tcx.map.find(did.node) {
Some(ast_map::NodeItem(ref item)) => item.vis != ast::Public,
Some(_) | None => 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.public_items.contains(&trait_id)
}

fn check_ty_param_bound(&self,
span: Span,
ty_param_bound: &ast::TyParamBound) {
match *ty_param_bound {
ast::TraitTyParamBound(ref trait_ref) => {
if !self.tcx.sess.features.borrow().visible_private_types &&
self.path_is_private_type(trait_ref.ref_id) {
self.tcx.sess.span_err(span,
"private type in exported type \
parameter bound");
}
}
_ => {}
}
}
}

impl<'a, 'b, 'tcx, 'v> Visitor<'v> for CheckTypeForPrivatenessVisitor<'a, 'b, 'tcx> {
Expand Down Expand Up @@ -1338,7 +1356,15 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> {
// namespace (the contents have their own privacies).
ast::ItemForeignMod(_) => {}

ast::ItemTrait(..) if !self.trait_is_public(item.id) => return,
ast::ItemTrait(_, _, ref bounds, _) => {
if !self.trait_is_public(item.id) {
return
}

for bound in bounds.iter() {
self.check_ty_param_bound(item.span, bound)
}
}

// impls need some special handling to try to offer useful
// error messages without (too many) false positives
Expand Down Expand Up @@ -1471,6 +1497,19 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> {
visit::walk_item(self, item);
}

fn visit_generics(&mut self, generics: &ast::Generics) {
for ty_param in generics.ty_params.iter() {
for bound in ty_param.bounds.iter() {
self.check_ty_param_bound(ty_param.span, bound)
}
}
for predicate in generics.where_clause.predicates.iter() {
for bound in predicate.bounds.iter() {
self.check_ty_param_bound(predicate.span, bound)
}
}
}

fn visit_foreign_item(&mut self, item: &ast::ForeignItem) {
if self.exported_items.contains(&item.id) {
visit::walk_foreign_item(self, item)
Expand All @@ -1488,12 +1527,11 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> {
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::builtin::VISIBLE_PRIVATE_TYPES,
path_id, p.span,
"private type in exported type \
signature".to_string());
if !self.tcx.sess.features.borrow().visible_private_types &&
self.path_is_private_type(path_id) {
self.tcx.sess.span_err(p.span,
"private type in exported type \
signature");
}
}
_ => {}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/typeck/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1645,7 +1645,7 @@ impl<'a, 'tcx> ErrorReportingHelpers for InferCtxt<'a, 'tcx> {
}
}

trait Resolvable {
pub trait Resolvable {
fn resolve(&self, infcx: &InferCtxt) -> Self;
fn contains_error(&self) -> bool;
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_llvm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ pub enum AttributeSet {
FunctionIndex = !0
}

trait AttrHelper {
pub trait AttrHelper {
fn apply_llfn(&self, idx: c_uint, llfn: ValueRef);
fn apply_callsite(&self, idx: c_uint, callsite: ValueRef);
}
Expand Down
1 change: 0 additions & 1 deletion src/librustrt/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ 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: Box<Task>) { unsafe { local_ptr::put(value) } }
Expand Down
1 change: 0 additions & 1 deletion src/librustrt/local_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,6 @@ 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
Expand Down
12 changes: 4 additions & 8 deletions src/librustrt/unwind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ fn rust_exception_class() -> uw::_Unwind_Exception_Class {

#[cfg(not(target_arch = "arm"), not(windows, target_arch = "x86_64"), not(test))]
#[doc(hidden)]
#[allow(visible_private_types)]
pub mod eabi {
use libunwind as uw;
use libc::c_int;
Expand Down Expand Up @@ -291,7 +290,6 @@ pub mod eabi {

#[cfg(target_os = "ios", target_arch = "arm", not(test))]
#[doc(hidden)]
#[allow(visible_private_types)]
pub mod eabi {
use libunwind as uw;
use libc::c_int;
Expand Down Expand Up @@ -347,7 +345,6 @@ pub mod eabi {
// but otherwise works the same.
#[cfg(target_arch = "arm", not(target_os = "ios"), not(test))]
#[doc(hidden)]
#[allow(visible_private_types)]
pub mod eabi {
use libunwind as uw;
use libc::c_int;
Expand Down Expand Up @@ -397,21 +394,20 @@ pub mod eabi {

#[cfg(windows, target_arch = "x86_64", not(test))]
#[doc(hidden)]
#[allow(visible_private_types)]
#[allow(non_camel_case_types, non_snake_case)]
pub mod eabi {
use libunwind as uw;
use libc::{c_void, c_int};

#[repr(C)]
struct EXCEPTION_RECORD;
pub struct EXCEPTION_RECORD;
#[repr(C)]
struct CONTEXT;
pub struct CONTEXT;
#[repr(C)]
struct DISPATCHER_CONTEXT;
pub struct DISPATCHER_CONTEXT;

#[repr(C)]
enum EXCEPTION_DISPOSITION {
pub enum EXCEPTION_DISPOSITION {
ExceptionContinueExecution,
ExceptionContinueSearch,
ExceptionNestedException,
Expand Down
2 changes: 1 addition & 1 deletion src/librustuv/addrinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use net;
use super::{Loop, UvError, Request, wait_until_woken_after, wakeup};
use uvll;

struct Addrinfo {
pub struct Addrinfo {
handle: *const libc::addrinfo,
}

Expand Down
1 change: 0 additions & 1 deletion src/librustuv/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ via `close` and `delete` methods.

#![feature(macro_rules, unsafe_destructor)]
#![deny(unused_result, unused_must_use)]
#![allow(visible_private_types)]

#![reexport_test_harness_main = "test_main"]

Expand Down
4 changes: 4 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ static KNOWN_FEATURES: &'static [(&'static str, Status)] = &[
("advanced_slice_patterns", Active),
("tuple_indexing", Active),
("associated_types", Active),
("visible_private_types", Active),

// if you change this list without updating src/doc/rust.md, cmr will be sad

Expand Down Expand Up @@ -100,6 +101,7 @@ pub struct Features {
pub overloaded_calls: bool,
pub rustc_diagnostic_macros: bool,
pub import_shadowing: bool,
pub visible_private_types: bool,
}

impl Features {
Expand All @@ -109,6 +111,7 @@ impl Features {
overloaded_calls: false,
rustc_diagnostic_macros: false,
import_shadowing: false,
visible_private_types: false,
}
}
}
Expand Down Expand Up @@ -479,6 +482,7 @@ pub fn check_crate(span_handler: &SpanHandler, krate: &ast::Crate) -> (Features,
overloaded_calls: cx.has_feature("overloaded_calls"),
rustc_diagnostic_macros: cx.has_feature("rustc_diagnostic_macros"),
import_shadowing: cx.has_feature("import_shadowing"),
visible_private_types: cx.has_feature("visible_private_types"),
},
unknown_features)
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/auxiliary/iss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

// part of issue-6919.rs

struct C<'a> {
pub struct C<'a> {
pub k: ||: 'a,
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/auxiliary/noexporttypelib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

type oint = Option<int>;
pub type oint = Option<int>;
pub fn foo() -> oint { Some(3) }
2 changes: 1 addition & 1 deletion src/test/auxiliary/priv-impl-prim-ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

trait A {
pub trait A {
fn frob(&self);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub enum MaybeOwned<'a> {
Borrowed(&'a int)
}

struct Inv<'a> { // invariant w/r/t 'a
pub struct Inv<'a> { // invariant w/r/t 'a
x: &'a mut &'a int
}

Expand Down
Loading