Consider substituting type aliases in "public-in-private" checker

[petrochenkov]

E.g. allowing things like

pub struct SomethingLongButPublicWithLotsOfParameters;
type ShortPrivAlias = SomethingLongButPublicWithLotsOfParameters;
pub fn public_function(arg: ShortPrivAlias) {}

This was originally done in #29973, but reverted later to follow the letter of RFC 136.
Now it starts hitting people. I think this should be reconsidered sooner than later to reduce the number of annoyed people.
Type alias substitution reduces the breakage by 25% according to the crater data.

[sgrif]

Just to give a concrete example, the current behavior even disallows type aliases in locations which are effectively private. An example from diesel is https://github.com/sgrif/diesel/blob/c8127d408f759faa056503014cd00e92479215e0/diesel/src/query_builder/select_statement/dsl_impls.rs#L54-L67, where the type alias is purely used for brevity in what is effectively an internal impl of a trait, but is disallowed because the trait itself is public.

[petrochenkov]

what is effectively an internal impl of a trait, but is disallowed because the trait itself is public.

The logic is that both the trait and the type are public, so the impl is public as well. (But this is a digression)

[sgrif]

Right, I fully understand the logic. My argument is that it's an ergonomics
issue in places where while that is technically true, it is not
semantically true (for lack of a better word).

On Sun, Dec 20, 2015, 6:04 PM Vadim Petrochenkov [email protected]
wrote:

what is effectively an internal impl of a trait, but is disallowed because
the trait itself is public.

The logic is that both the trait and the type are public, so the impl is
public as well.

—
Reply to this email directly or view it on GitHub
#30503 (comment).

[petrochenkov]

cc @seanmonstar
(You also wanted this)

[seanmonstar]

Yes! 👍

[pnkfelix]

I'm basically in favor of this; I have some vague concerns about specifying it literally as substitution of the right hand side given that we have parameters to consider in the general case, but it seems plausible that kind of substitution works out fine.

[petrochenkov]

@pnkfelix
An implementation is here.
(It may be missing some corner cases, but I'm not aware of them.)

[petrochenkov]

cc @retep998
I guess this can affect -sys crates (maybe winapi?) with lots of typedefs positively.

[retep998]

I pretty much make everything pub anyway so this doesn't really affect winapi either way. The thing that did affect winapi was things like DECLARE_HANDLE! which created a private opaque type, with a public typedef to a raw pointer to that.

[nixpulvis]

I'd be in favor of something like

type Foo = usize;  // This is a type alias which must always be preserved.
alias Bar = usize;  // This is a type alias which always gets substituted when crossing modules.

This allows

alias Return = (usize, isize);

mod a {
    fn foo() -> super::Return;
}

mod b {
    fn foo() -> super::Return;
}

But is documented as returning a (usize, isize) everywhere.

[nikomatsakis]

I think the primary concern with this was rustdoc -- which will show these private aliases -- but we could probably change rustdoc's behavior to expand private aliases.

[nikomatsakis]

I'm nominating this issue -- I agree with @petrochenkov that it makes sense to make this change before making the warnings into hard errors -- I am a bit unclear on whether we think an RFC would be required or what.

[nikomatsakis]

Also as it happens @wycats was just complaining to me today that private type aliases cause these issues. :)

[eddyb]

Just opened rust-lang/rfcs#1671 which includes this feature.

[nikomatsakis]

So @nrc and I discussed this -- but we're the only ones in the @rust-lang/lang meeting due to various reasons -- we both feel like we should accept this change (use the semantic type, not the type alias) and probably also adopt the simpler version of @eddyb's proposal that is described on internals.