Semantic "private in public" enforcement.

[eddyb]

Rendered

[petrochenkov]

"able to name" is not entirely correct, "able to name after addition of arbitrary number of reexports" is closer to the truth. E.g.

mod m {
    mod detail {
        pub struct S;
    }
    pub fn f() -> detail::S { detail::S }
}

is allowed because S is potentially nameable outside of m (if reexported), but not actually nameable (private-in-public checker doesn't analyze reexport chains to determine actual nameability by design).
I'd like to avoid promising namebility in docs, including RFCs, because it's already one of the most common misconceptions about our privacy system.
EDIT: The definition of "less public" below also uses nameability ("can be referred to by any name").

[eddyb]

Ah, I remember reading about this before, but I wasn't clear on what decisions were taken.
It does weaken the "transitivity of abstraction" argument in that case, sadly.

[Ericson2314]

How odd, link to rational for this?

[petrochenkov]

@Ericson2314
historical overview (with links!)
:)

[Ericson2314]

Thanks so much!

[petrochenkov]

The basic idea behind the private-in-public checker is that values of private types (in addition to types themselves) can't leave their module.
Can a public function/method be tricked into returning a private type (with traits + associated types + type parameters + whatever) if it's used in one of its predicates like this PrivateType: Bounds?
Also, where can I read something about predicate elaboration?

[eddyb]

The return type is in the signature and checked as part of that, regardless of what happens with bounds.
With this RFC you can still always write a perfect proxy for functions, methods, impls, etc.

"Elaboration" as used in this RFC is described in the detailed design section. I should probably be more specific about it because it only does the work necessary to rewrite private bounds in terms of more public ones.

[petrochenkov]

Here's an example of leaking a private type through bounds, the returned type T::Alias seems to be completely public if checked separately. I wonder if something like this can be constructed with PrivateWrap<T>: Bound-like bounds. (I fully support the goals of the RFC, just trying to figure out possible holes.)

pub trait Tr<T> {
    type Alias = T;
}
impl<T, U> Tr<T> for U {}

struct Priv;
pub fn f<T>() -> T::Alias
    where T: Tr<Priv>
{
    panic!()
}

[eddyb]

After associated type resolution, the return type of f is Priv and that doesn't pass the check.
Even if it didn't resolve to any concrete type, the return type would be <T as Tr<Priv>>::Alias which still contains Priv so it doesn't pass the check either.

[petrochenkov]

By the way, if you do the T::Alias -> <T as Tr<Priv>>::Alias transformations before checking, then predicates can become completely unchecked, because nothing private can be leaked through them¹.
This is more significant relaxation of the rules, but it is simpler and removes the need in the elaboration procedure.
We discussed this with @pnkfelix last year and both were generally in favor.

¹"nothing private can be leaked" is the core guarantee from the current privacy system, as opposed to "nameability" from the old system.

[eddyb]

Well, that does reduce this RFC to "check types after normalization", I wish that was better advertised as an option (it seems perhaps even simpler than the current implementation).

[eddyb]

I want to have this outside of diff comments too: @petrochenkov pointed out that name, type alias, UFCS and associated type resolution combined (i.e. everything the compiler does to go from the syntactic to the semantic type-system) are enough to reveal all possible private type leaks in public APIs.
We then wouldn't even need to look at bounds, if all we care about is preventing private type leaks, and not the ability to write perfect proxies or anything else.

I agree this would greatly simplify the RFC and the implementation work and allow deriving to work even in dubious situations where the set of implementations available to a bound is completely hidden.

Does anyone else (e.g. from @rust-lang/lang) have anything against that?
If not, I'll update (or rewrite, even) the RFC to reflect the simpler solution.

[Ericson2314]

@eddyb sounds great to me! I'd ideally like the docs to evaluate/resolve types as little as possible---just enough not leak. Maybe we can someday leverage incremental compilation for this. (Type language is pure and thus can be safely lazily evaluated.)

[petrochenkov]

@eddyb

We then wouldn't even need to look at bounds

The main argument against this change probably lies outside of the language - documentation.
A function like fn f<T: PrivateBounds>(arg: T) { .... } is a black box, unless it's well documented in prose. It's not clear what arguments it can accept. If wrong arguments are provided then users see error messages like "PrivateTrait is not satisfied", what can they do with them? (more detailed comment about this).
On the other hand, derived impls - the main beneficiary from private bounds - don't care about documenting bounds.

[petrochenkov]

"nothing private can be leaked" is the core guarantee from the current privacy system (#1671 (comment))

I should write this out in more detail. The guarantee is that entities with visibility pub(m) "can't escape" from the module m.
What "can't escape" means for different entities?

1. For imports, extern crates, modules, type aliases, functions, statics, constants, foreign functions, foreign statics, struct fields, associated functions/types/constants this simply means that they can't be named outside of module m. This is ensured by the usual privacy checker ("can't access private X") and limitations on reexports ("X is private, and cannot be reexported").

2. For types (structs and enums) and traits-as-types this additionally means that values of those types cannot be obtained. This is ensured by the private-in-public checker. The private-in-public has to check bounds and where clauses due to situations like this, unless types are fully transformed into their "semantic" form (ty::Ty) before checking.

3. Traits. When should a trait be considered escaping from its module (In addition to conditions from 1. and 2.)? This is the key question to answer before relaxing the private-in-public rules.

   Regardless of the discussion about predicates above, I think a trait should be considered leaked if its items are available outside of its module. This can happen if the private trait has public sub-traits (pub trait PubTr: PrivTr { .... }), then its methods are available through its sub-traits. Returning a public subtrait like a trait object fn f() -> Box<PubTr> { .... } also kind of makes PrivTr leaked as a type too in some sense.
   So I think private-in-public checker still needs to check bounds on trait definitions (i.e. supertraits).

[crumblingstatue]

Does this aim to address rust-lang/rust#30905?
More specifically, I am very interested in resolving this type of API weirdness:

// This type is inacessible outside of the crate, but "public" to the entire
// crate, since it's at the crate root.
struct CratePrivType;

mod private {
    // This function is inacessible outside of the crate, so it should be okay
    // to return a crate-private type
    pub fn gimme_crate_priv() -> super::CratePrivType { unimplemented!() }
}

fn main() {}

error: private type in public interface [--explain E0446]
 --> <anon>:8:34
  |>
8 |>     pub fn gimme_crate_priv() -> super::CratePrivType { unimplemented!() }
  |>                                  ^^^^^^^^^^^^^^^^^^^^

To work around this without exposing the crate-private type, one can put it in a private module.

mod crate_priv {
    pub struct CratePrivType;
}

mod private {
    pub fn gimme_crate_priv() -> ::crate_priv::CratePrivType { unimplemented!() }
}

fn main() {}

But this once again shows the stupidity of the current analysis.
You can now use this type in your public API, even though it's supposed to be private.

mod crate_priv {
    pub struct CratePrivType;
}

mod private {
    pub fn gimme_crate_priv() -> ::crate_priv::CratePrivType { unimplemented!() }
}

pub fn pub_api_func() -> ::crate_priv::CratePrivType { unimplemented!() }

fn main() {}

Even rustdoc gets confused by this, and the crate private type will not get documented, even though
it will appear in the signature of pub_api_func.

[eddyb]

@crumblingstatue We could restrict the check to the same module, which would allow that code to compile.
It's orthogonal to how the check is done though, i.e. it could be implemented with the current system too.

[petrochenkov]

@crumblingstatue
This was resolved by RFC 1422, your example is supposed to be written like this:

#![feature(pub_restricted)]

// This type is inacessible outside of the crate, but "public" to the entire
// crate, since it's at the crate root.
struct CratePrivType;

mod private {
    // This function is inacessible outside of the crate, so it should be okay
    // to return a crate-private type
    pub(crate) fn gimme_crate_priv() -> super::CratePrivType { unimplemented!() }
}

fn main() {}

Playpen: https://is.gd/TFqG8o

[crumblingstatue]

@petrochenkov You're right, pub(restricted) does solve the issue I was having. Thanks, and apologies for invading this thread.

[nrc]

Alternative proposal on discuss (also from @eddyb): https://internals.rust-lang.org/t/rfc-reduce-false-positives-for-private-type-in-public-interface/3678

[nrc]

cc @pnkfelix do you remember the ins and outs of this from the last RFC discussion?

[ubsan]

ping @eddyb

status?

[eddyb]

@ubsan See the latest comments on rust-lang/rust#34537. We're more likely to switch to a different scheme, checking types as they appear after inference, as opposed to only checking direct uses.
Not sure what the next step is, though.

[ubsan]

@eddyb sounds like a close to me. Or at least a rewrite.

[eddyb]

@ubsan Oh right, it's my PR so I can just do this.