librustc: Require that types mentioned anywhere in public signatures be

public, including in trait bounds and typedefs.

There are three main patterns that this breaks. First is code that uses
private trait bounds in public APIs:

    pub mod a {
        trait Trait { ... }
        pub fn f<T:Trait>() { ... }
    }

Change that code to export the trait. For example:

    pub mod a {
        pub trait Trait { ... }     // note `pub`
        pub fn f<T:Trait>() { ... }
    }

Second, this change breaks code that was relying on
`#[allow(visible_private_types)]`. That lint is now a hard error, and
cannot be overridden. For example:

    mod helper {
        pub struct Foo { ... }
    }
    pub mod public {
        use helper::Foo;
        pub fn f() -> Foo;
    }

Because there is no way for code that uses `public::f` to name `Foo`,
this is no longer allowed. Change affected code to look like this:

    pub mod helper {    // note `pub`
        pub struct Foo { ... }
    }
    pub mod public {
        use helper::Foo;
        pub fn f() -> Foo;
    }

Third, this change breaks code that was relying on being able to
reference private typedefs in public APIs. For example:

    pub mod b {
        type Foo = int;
        pub fn f(_: Foo) { ... }
    }

Change this code to:

    pub mod b {
        pub type Foo = int; // note `pub`
        pub fn f(_: Foo) { ... }
    }

For now, the old behavior can be obtained with
`#[feature(visible_private_types)]`. However, the preciase semantics of
this feature gate may be unstable.

This implements RFC #48.

Closes #16463.

[breaking-change]

Author: pcwalton

src/libarena/lib.rs

@@ -28,9 +28,12 @@
        html_favicon_url = "http://www.rust-lang.org/favicon.ico",
        html_root_url = "http://doc.rust-lang.org/master/")]
 
-#![feature(unsafe_destructor)]
+#![feature(unsafe_destructor, visible_private_types)]
 #![allow(missing_doc)]
 
+// NOTE(stage0, pcwalton): Remove after snapshot.
+#![allow(unknown_features)]
+
 use std::cell::{Cell, RefCell};
 use std::cmp;
 use std::intrinsics::{TyDesc, get_tydesc};
@@ -359,6 +362,7 @@ pub struct TypedArena<T> {
     /// A pointer to the first arena segment.
     first: RefCell<TypedArenaChunkRef<T>>,
 }
+
 type TypedArenaChunkRef<T> = Option<Box<TypedArenaChunk<T>>>;
 
 struct TypedArenaChunk<T> {

src/libcore/iter.rs

@@ -2182,7 +2182,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>(f: |T|: 'a -> T, seed: T) -> Iterate<'a, T> {
     Unfold::new((f, Some(seed), true), |st| {
         let &(ref mut f, ref mut val, ref mut first) = st;

src/libcore/lib.rs

@@ -58,9 +58,12 @@
 
 #![no_std]
 #![feature(globs, intrinsics, lang_items, macro_rules, managed_boxes, phase)]
-#![feature(simd, unsafe_destructor)]
+#![feature(simd, unsafe_destructor, visible_private_types)]
 #![deny(missing_doc)]
 
+// NOTE(stage0, pcwalton): Remove after snapshot.
+#![allow(unknown_features)]
+
 mod macros;
 
 #[path = "num/float_macros.rs"] mod float_macros;

src/libdebug/lib.rs

@@ -25,9 +25,12 @@
        html_favicon_url = "http://www.rust-lang.org/favicon.ico",
        html_root_url = "http://doc.rust-lang.org/master/")]
 #![experimental]
-#![feature(managed_boxes, macro_rules)]
+#![feature(managed_boxes, macro_rules, visible_private_types)]
 #![allow(experimental)]
 
+// NOTE(stage0, pcwalton): Remove after snapshot.
+#![allow(unknown_features)]
+
 pub mod fmt;
 pub mod reflect;
 pub mod repr;

src/libgreen/lib.rs

@@ -217,8 +217,11 @@
        html_playground_url = "http://play.rust-lang.org/")]
 
 // NB this does *not* include globs, please keep it that way.
-#![feature(macro_rules, phase, default_type_params)]
-#![allow(visible_private_types, deprecated)]
+#![feature(macro_rules, phase, default_type_params, visible_private_types)]
+#![allow(deprecated)]
+
+// NOTE(stage0, pcwalton): Remove after snapshot.
+#![allow(unknown_features)]
 
 #[cfg(test)] #[phase(plugin, link)] extern crate log;
 #[cfg(test)] extern crate rustuv;

src/libnative/io/timer_unix.rs

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

src/libnative/lib.rs

@@ -63,6 +63,10 @@
 //    consider whether they're needed before adding that feature here (the
 //    answer is that you don't need them)
 #![feature(macro_rules, unsafe_destructor, default_type_params)]
+#![feature(visible_private_types)]
+
+// NOTE(stage0, pcwalton): Remove after snapshot.
+#![allow(unknown_features)]
 
 extern crate alloc;
 extern crate libc;

src/libregex/compile.rs

@@ -8,9 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// 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)]
+#![allow(missing_doc)]
 
 use std::cmp;
 use parse;

src/libregex/lib.rs

@@ -368,9 +368,12 @@
        html_root_url = "http://doc.rust-lang.org/master/",
        html_playground_url = "http://play.rust-lang.org/")]
 
-#![feature(macro_rules, phase)]
+#![feature(macro_rules, phase, visible_private_types)]
 #![deny(missing_doc)]
 
+// NOTE(stage0, pcwalton): Remove after snapshot.
+#![allow(unknown_features)]
+
 #[cfg(test)]
 extern crate stdtest = "test";
 #[cfg(test)]

src/libregex/re.rs

@@ -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.
@@ -516,7 +515,6 @@ impl Regex {
     }
 
     #[doc(hidden)]
-    #[allow(visible_private_types)]
     #[experimental]
     pub fn names_iter<'a>(&'a self) -> NamesIter<'a> {
         match *self {

src/librustc/front/feature_gate.rs

@@ -68,6 +68,7 @@ static KNOWN_FEATURES: &'static [(&'static str, Status)] = &[
 
     ("rustc_diagnostic_macros", Active),
     ("unboxed_closures", Active),
+    ("visible_private_types", Active),
 
     // if you change this list without updating src/doc/rust.md, cmr will be sad
 
@@ -98,7 +99,8 @@ pub struct Features {
     pub default_type_params: Cell<bool>,
     pub issue_5723_bootstrap: Cell<bool>,
     pub overloaded_calls: Cell<bool>,
-    pub rustc_diagnostic_macros: Cell<bool>
+    pub rustc_diagnostic_macros: Cell<bool>,
+    pub visible_private_types: Cell<bool>,
 }
 
 impl Features {
@@ -107,7 +109,8 @@ impl Features {
             default_type_params: Cell::new(false),
             issue_5723_bootstrap: Cell::new(false),
             overloaded_calls: Cell::new(false),
-            rustc_diagnostic_macros: Cell::new(false)
+            rustc_diagnostic_macros: Cell::new(false),
+            visible_private_types: Cell::new(false),
         }
     }
 }
@@ -439,4 +442,5 @@ pub fn check_crate(sess: &Session, krate: &ast::Crate) {
     sess.features.issue_5723_bootstrap.set(cx.has_feature("issue_5723_bootstrap"));
     sess.features.overloaded_calls.set(cx.has_feature("overloaded_calls"));
     sess.features.rustc_diagnostic_macros.set(cx.has_feature("rustc_diagnostic_macros"));
+    sess.features.visible_private_types.set(cx.has_feature("visible_private_types"));
 }

src/librustc/lint/builtin.rs

@@ -1539,9 +1539,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")
 
@@ -1570,7 +1567,6 @@ impl LintPass for HardwiredLints {
             UNUSED_VARIABLE,
             DEAD_ASSIGNMENT,
             DEAD_CODE,
-            VISIBLE_PRIVATE_TYPES,
             UNREACHABLE_CODE,
             WARNINGS,
             UNKNOWN_FEATURES,

src/librustc/middle/mem_categorization.rs

@@ -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;
 }

src/librustc/middle/privacy.rs

@@ -17,7 +17,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};
@@ -163,23 +162,6 @@ struct EmbargoVisitor<'a> {
     prev_public: bool,
 }
 
-impl<'a> EmbargoVisitor<'a> {
-    // 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
-    }
-}
-
 impl<'a> Visitor<()> for EmbargoVisitor<'a> {
     fn visit_item(&mut self, item: &ast::Item, _: ()) {
         let orig_all_pub = self.prev_public;
@@ -194,12 +176,6 @@ impl<'a> Visitor<()> for EmbargoVisitor<'a> {
             // cannot have visibility qualifiers on them anyway
             ast::ItemImpl(..) | ast::ItemForeignMod(..) => {}
 
-            // Traits are a little special in that even if they themselves are
-            // not public they may still be exported.
-            ast::ItemTrait(..) => {
-                self.prev_exported = self.exported_trait(item.id);
-            }
-
             // Private by default, hence we only retain the "public chain" if
             // `pub` is explicitly listed.
             _ => {
@@ -1218,7 +1194,6 @@ impl<'a> SanePrivacyVisitor<'a> {
 struct VisiblePrivateTypesVisitor<'a> {
     tcx: &'a ty::ctxt,
     exported_items: &'a ExportedItems,
-    public_items: &'a PublicItems,
 }
 
 struct CheckTypeForPrivatenessVisitor<'a, 'b> {
@@ -1250,9 +1225,14 @@ impl<'a> VisiblePrivateTypesVisitor<'a> {
     }
 
     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)
+        self.exported_items.contains(&trait_id)
+    }
+
+    fn check_path(&self, path: &ast::Path, path_id: ast::NodeId) {
+        if self.path_is_private_type(path_id) {
+            self.tcx.sess.span_err(path.span,
+                                   "private type in exported type signature");
+        }
     }
 }
 
@@ -1407,10 +1387,6 @@ impl<'a> Visitor<()> for VisiblePrivateTypesVisitor<'a> {
                 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,
 
@@ -1439,17 +1415,24 @@ impl<'a> Visitor<()> for VisiblePrivateTypesVisitor<'a> {
         }
     }
 
-    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());
+    fn visit_generics(&mut self, g: &ast::Generics, _: ()) {
+        for type_parameter in g.ty_params.iter() {
+            for bound in type_parameter.bounds.iter() {
+                match *bound {
+                    ast::TraitTyParamBound(ref trait_ref) => {
+                        self.check_path(&trait_ref.path, trait_ref.ref_id)
+                    }
+                    ast::StaticRegionTyParamBound |
+                    ast::UnboxedFnTyParamBound(_) |
+                    ast::OtherRegionTyParamBound(_) => {}
                 }
             }
+        }
+    }
+
+    fn visit_ty(&mut self, t: &ast::Ty, _: ()) {
+        match t.node {
+            ast::TyPath(ref p, _, path_id) => self.check_path(p, path_id),
             _ => {}
         }
         visit::walk_ty(self, t, ())
@@ -1535,13 +1518,13 @@ pub fn check_crate(tcx: &ty::ctxt,
 
     let EmbargoVisitor { exported_items, public_items, .. } = visitor;
 
-    {
+    if !tcx.sess.features.visible_private_types.get() {
         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);
 }

src/librustc/middle/resolve.rs

@@ -44,7 +44,7 @@ use std::uint;
 // Definition mapping
 pub type DefMap = RefCell<NodeMap<Def>>;
 
-struct binding_info {
+pub struct binding_info {
     span: Span,
     binding_mode: BindingMode,
 }

src/librustc/middle/resolve_lifetime.rs

@@ -56,7 +56,7 @@ struct LifetimeContext<'a> {
     named_region_map: NamedRegionMap,
 }
 
-enum ScopeChain<'a> {
+pub enum ScopeChain<'a> {
     /// EarlyScope(i, ['a, 'b, ...], s) extends s with early-bound
     /// lifetimes, assigning indexes 'a => i, 'b => i+1, ... etc.
     EarlyScope(subst::ParamSpace, &'a Vec<ast::LifetimeDef>, Scope<'a>),
@@ -69,7 +69,7 @@ enum ScopeChain<'a> {
     RootScope
 }
 
-type Scope<'a> = &'a ScopeChain<'a>;
+pub type Scope<'a> = &'a ScopeChain<'a>;
 
 pub fn krate(sess: &Session, krate: &ast::Crate) -> NamedRegionMap {
     let mut ctxt = LifetimeContext {

src/librustc/middle/typeck/infer/error_reporting.rs

@@ -1429,7 +1429,7 @@ impl<'a> ErrorReportingHelpers for InferCtxt<'a> {
     }
 }
 
-trait Resolvable {
+pub trait Resolvable {
     fn resolve(&self, infcx: &InferCtxt) -> Self;
     fn contains_error(&self) -> bool;
 }

src/librustc/middle/typeck/infer/lattice.rs

@@ -45,7 +45,7 @@ use util::ppaux::Repr;
 
 use std::collections::HashMap;
 
-trait LatticeValue : Clone + Repr + PartialEq {
+pub trait LatticeValue : Clone + Repr + PartialEq {
     fn sub(cf: CombineFields, a: &Self, b: &Self) -> ures;
     fn lub(cf: CombineFields, a: &Self, b: &Self) -> cres<Self>;
     fn glb(cf: CombineFields, a: &Self, b: &Self) -> cres<Self>;

src/librustc/middle/typeck/variance.rs

@@ -230,11 +230,11 @@ pub fn infer_variance(tcx: &ty::ctxt,
  * a variable.
  */
 
-type VarianceTermPtr<'a> = &'a VarianceTerm<'a>;
+pub type VarianceTermPtr<'a> = &'a VarianceTerm<'a>;
 
-struct InferredIndex(uint);
+pub struct InferredIndex(uint);
 
-enum VarianceTerm<'a> {
+pub enum VarianceTerm<'a> {
     ConstantTerm(ty::Variance),
     TransformTerm(VarianceTermPtr<'a>, VarianceTermPtr<'a>),
     InferredTerm(InferredIndex),

src/librustc_llvm/lib.rs

@@ -140,7 +140,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);
 }