Auto merge of #29973 - petrochenkov:privinpub, r=nikomatsakis

Some notes:
This patch enforces the rules from [RFC 136](https://github.com/rust-lang/rfcs/blob/master/text/0136-no-privates-in-public.md) and makes "private in public" a module-level concept and not crate-level. Only `pub` annotations are used by the new algorithm, crate-level exported node set produced by `EmbargoVisitor` is not used. The error messages are tweaked accordingly and don't use the word "exported" to avoid confusing people (#29668).

The old algorithm tried to be extra smart with impls, but it mostly led to unpredictable behavior and bugs like #28325.
The new algorithm tries to be as simple as possible - an impl is considered public iff its type is public and its trait is public (if presents).
A type or trait is considered public if all its components are public, [complications](https://internals.rust-lang.org/t/limits-of-type-inference-smartness/2919) with private types leaking to other crates/modules through trait impls and type inference are deliberately ignored so far.

The new algorithm is not recursive and uses the nice new facility `Crate::visit_all_items`!

Obsolete pre-1.0 feature `visible_private_types` is removed.

This is a [breaking-change].
The two main vectors of breakage are type aliases (#28450) and impls (#28325).
I need some statistics from a crater run (cc @alexcrichton) to decide on the breakage mitigation strategy.
UPDATE: All the new errors are reported as warnings controlled by a lint `private_in_public` and lint group `future_incompatible`, but the intent is to make them hard errors eventually.

Closes #28325
Closes #28450
Closes #29524
Closes #29627
Closes #29668
Closes #30055

r? @nikomatsakis

src/doc/reference.md

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

src/libcollections/btree/node.rs

@@ -78,7 +78,7 @@ pub struct Node<K, V> {
     _capacity: usize,
 }
 
-struct NodeSlice<'a, K: 'a, V: 'a> {
+pub struct NodeSlice<'a, K: 'a, V: 'a> {
     keys: &'a [K],
     vals: &'a [V],
     pub edges: &'a [Node<K, V>],
@@ -87,7 +87,7 @@ struct NodeSlice<'a, K: 'a, V: 'a> {
     has_edges: bool,
 }
 
-struct MutNodeSlice<'a, K: 'a, V: 'a> {
+pub struct MutNodeSlice<'a, K: 'a, V: 'a> {
     keys: &'a [K],
     vals: &'a mut [V],
     pub edges: &'a mut [Node<K, V>],
@@ -1344,7 +1344,7 @@ fn min_load_from_capacity(cap: usize) -> usize {
 /// A trait for pairs of `Iterator`s, one over edges and the other over key/value pairs. This is
 /// necessary, as the `MoveTraversalImpl` needs to have a destructor that deallocates the `Node`,
 /// and a pair of `Iterator`s would require two independent destructors.
-trait TraversalImpl {
+pub trait TraversalImpl {
     type Item;
     type Edge;
 
@@ -1358,7 +1358,7 @@ trait TraversalImpl {
 /// A `TraversalImpl` that actually is backed by two iterators. This works in the non-moving case,
 /// as no deallocation needs to be done.
 #[derive(Clone)]
-struct ElemsAndEdges<Elems, Edges>(Elems, Edges);
+pub struct ElemsAndEdges<Elems, Edges>(Elems, Edges);
 
 impl<K, V, E, Elems: DoubleEndedIterator, Edges: DoubleEndedIterator>
         TraversalImpl for ElemsAndEdges<Elems, Edges>
@@ -1375,7 +1375,7 @@ impl<K, V, E, Elems: DoubleEndedIterator, Edges: DoubleEndedIterator>
 }
 
 /// A `TraversalImpl` taking a `Node` by value.
-struct MoveTraversalImpl<K, V> {
+pub struct MoveTraversalImpl<K, V> {
     keys: RawItems<K>,
     vals: RawItems<V>,
     edges: RawItems<Node<K, V>>,
@@ -1436,7 +1436,7 @@ impl<K, V> Drop for MoveTraversalImpl<K, V> {
 
 /// An abstraction over all the different kinds of traversals a node supports
 #[derive(Clone)]
-struct AbsTraversal<Impl> {
+pub struct AbsTraversal<Impl> {
     inner: Impl,
     head_is_edge: bool,
     tail_is_edge: bool,

src/librustc/lint/builtin.rs

@@ -117,6 +117,13 @@ declare_lint! {
     Allow,
     "detects trivial casts of numeric types which could be removed"
 }
+
+declare_lint! {
+    pub PRIVATE_IN_PUBLIC,
+    Warn,
+    "detect private items in public interfaces not caught by the old implementation"
+}
+
 /// Does nothing as a lint pass, but registers some `Lint`s
 /// which are used by other parts of the compiler.
 #[derive(Copy, Clone)]
@@ -141,6 +148,7 @@ impl LintPass for HardwiredLints {
             FAT_PTR_TRANSMUTES,
             TRIVIAL_CASTS,
             TRIVIAL_NUMERIC_CASTS,
+            PRIVATE_IN_PUBLIC,
             CONST_ERR
         )
     }

src/librustc_lint/lib.rs

@@ -146,6 +146,9 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                     UNUSED_MUT, UNREACHABLE_CODE, UNUSED_MUST_USE,
                     UNUSED_UNSAFE, PATH_STATEMENTS, UNUSED_ATTRIBUTES);
 
+    add_lint_group!(sess, "future_incompatible",
+                    PRIVATE_IN_PUBLIC);
+
     // We have one lint pass defined specially
     store.register_late_pass(sess, false, box lint::GatherNodeLevels);
 

src/librustc_mir/build/matches/mod.rs

@@ -209,7 +209,7 @@ struct ArmBlocks {
 }
 
 #[derive(Clone, Debug)]
-struct Candidate<'pat, 'tcx:'pat> {
+pub struct Candidate<'pat, 'tcx:'pat> {
     // all of these must be satisfied...
     match_pairs: Vec<MatchPair<'pat, 'tcx>>,
 
@@ -235,7 +235,7 @@ struct Binding<'tcx> {
 }
 
 #[derive(Clone, Debug)]
-struct MatchPair<'pat, 'tcx:'pat> {
+pub struct MatchPair<'pat, 'tcx:'pat> {
     // this lvalue...
     lvalue: Lvalue<'tcx>,
 
@@ -278,7 +278,7 @@ enum TestKind<'tcx> {
 }
 
 #[derive(Debug)]
-struct Test<'tcx> {
+pub struct Test<'tcx> {
     span: Span,
     kind: TestKind<'tcx>,
 }

src/librustc_mir/build/mod.rs

@@ -18,7 +18,7 @@ use rustc_front::hir;
 use syntax::ast;
 use syntax::codemap::Span;
 
-struct Builder<'a, 'tcx: 'a> {
+pub struct Builder<'a, 'tcx: 'a> {
     hir: Cx<'a, 'tcx>,
     cfg: CFG<'tcx>,
     scopes: Vec<scope::Scope<'tcx>>,
@@ -40,7 +40,7 @@ struct CFG<'tcx> {
 // convenient.
 
 #[must_use] // if you don't use one of these results, you're leaving a dangling edge
-struct BlockAnd<T>(BasicBlock, T);
+pub struct BlockAnd<T>(BasicBlock, T);
 
 trait BlockAndExtension {
     fn and<T>(self, v: T) -> BlockAnd<T>;

src/librustc_privacy/diagnostics.rs

@@ -21,13 +21,14 @@ trait Foo {
     fn dummy(&self) { }
 }
 
-pub trait Bar : Foo {} // error: private trait in exported type parameter bound
+pub trait Bar : Foo {} // error: private trait in public interface
 pub struct Bar<T: Foo>(pub T); // same error
 pub fn foo<T: Foo> (t: T) {} // same error
 ```
 
-To solve this error, please ensure that the trait is also public and accessible
-at the same level of the public functions or types which are bound on it.
+To solve this error, please ensure that the trait is also public. The trait
+can be made inaccessible if necessary by placing it into a private inner module,
+but it still has to be marked with `pub`.
 Example:
 
 ```
@@ -42,20 +43,22 @@ pub fn foo<T: Foo> (t: T) {} // ok!
 "##,
 
 E0446: r##"
-A private type was used in an exported type signature. Erroneous code example:
+A private type was used in a public type signature. Erroneous code example:
 
 ```
 mod Foo {
     struct Bar(u32);
 
-    pub fn bar() -> Bar { // error: private type in exported type signature
+    pub fn bar() -> Bar { // error: private type in public interface
         Bar(0)
     }
 }
 ```
 
-To solve this error, please ensure that the type is also public and accessible
-at the same level of the public functions or types which use it. Example:
+To solve this error, please ensure that the type is also public. The type
+can be made inaccessible if necessary by placing it into a private inner module,
+but it still has to be marked with `pub`.
+Example:
 
 ```
 mod Foo {