finish up type alias PRs

[nikomatsakis]

I am going to pick up @eddyb's type-alias PRs, at least the ones that should become hard errors in Rust 2018:

• Too many bounds on a type alias (we need to issue suggestions for T: ?Sized in this case)
• "Global" bounds

The plan is to issue warnings in Rust 2015 and hard errors in Rust 2018, at least for now. "Too few bounds" unfortunately had too many regressions to make into a hard error without some form of migration. We'll tackle that too, but later.

Adding this issue to help us keep track of this.

Question: Do we feel it's an RC2 blocker?

[nikomatsakis]

cc @Centril

[Centril]

Seems good to RC2-block on this, as it might otherwise not get done at all. :)

[Centril]

For reference here are the PRs eddy has made:

• [WIP] rustc_typeck: make "global" WF requirements hard errors, in type aliases. #54394
• [WIP] rustc: check that type alias where clauses are well-formed. #54352
• [WIP] rustc_typeck: ensure type alias bounds are implied by the type being well-formed. #54090
• [WIP] rustc_typeck: check well-formedness of type aliases. #54033

[nikomatsakis]

So I spent a few hours today studying the PRs and rebasing #54090. Landing this PR as is is clearly not an option:

• it excludes T: Sized for no particular reason (we need a migration)
• it breaks diesel (introduces an overflow error for reasons I do not fully understand, but it occurs when trying to normalize the WF conditions of helper_types::Select)
• the diagnostics are not great
• it is not yet a warning on Rust 2015

Obviously, moving to a warning on Rust 2015 would help with the "breaks Diesel" thing.

I'm feeling a bit nervous about trying to block on this for the edition, simply from a "Project Management" point of view -- and especially for RC2. I am worried that there will be unexpected fallout and I'm not sure if it's a change we have to do, at least not now. Obviously, it would lay the groundwork for a rationalization of type behavior, and it would let us cleanly act on that in Rust 2018, so that's appealing.

On the other hand, it adds a certain amount of risk, as the Diesel example shows. It's not going to be so easy to do the migration without elaborating and normalizing, and that creates a cycle error right now.

I might rather hold off for now and then add lints for all editions that warn about extra bounds and globally non-WF types. We can use the evaluate queries for this and migrate to Chalk once that work is done (at which point everything will be doing evaluate-style queries). This will avoid complications from overflow though it will mean we don't presently handle regions correctly; I think we can fix that in time too.

At that point, once we have warnings in place, we can tighten them and eventually move to a hard-error. The crater runs do suggest we could migrate the ecosystem globally but at worst we can do it for Rust 2021.

All in all, it feels better to me to make this transition slowly rather than trying to squeeze it in last minute.

[pnkfelix]

I am in agreement that we should land this in a slow transition via gradual lints. We are very close to the release for the 2018 edition and the risk/reward proposition strikes me as pointing clearly towards taking the gradual route.

@Centril said that we should RC2-block on this, or else it might not get done at all. Usually I take that to mean “if we don’t make this adjustment before initial deployment, too much code will be written that relies on the undesirable semantics”

But in this case we’re taking about a feature that has been in Rust since 1.0; there’s going to be pain associationed with the shift here no matter what (unless rustfix gets really smart...)

Am I misunderstanding something about why @Centril says we should RC2-block on this?

[pnkfelix]

(I do understand the idea that new code will be written targeting the 2018 edition, and that’s why it’s good to try to time the painful shifts with that transition; that’s been part of why I’ve pushed to couple the initial NLL deployment with the edition. But again: I’m trying to understand the risk/reward proposition in this case.)

[nikomatsakis]

But in this case we’re taking about a feature that has been in Rust since 1.0; there’s going to be pain associationed with the shift here no matter what (unless rustfix gets really smart...)

I wanted to note one other thing: I feel like we ought to be able to make rustfix able to handle this, at least in the majority of cases. This may not change anything fundamental, just an interesting thing to note.

that’s been part of why I’ve pushed to couple the initial NLL deployment with the edition

indeed, the difference being that NLL is also a major edition "deliverable" -- i.e., a big part of the reason to have an edition in the first place is being able to tout "come experience the new Rust borrow checker, it's friendlier"

[Centril]

Usually I take that to mean “if we don’t make this adjustment before initial deployment, too much code will be written that relies on the undesirable semantics”

Yeah; basically that's it.

But if you think it won't be a problem and we can do the gradual way instead, then I'll defer to y'all's judgement.

[nikomatsakis]

At this point I just don't see a path to get this into RC2 at all. I'm removing from milestones — I think we have no real choice but the gradual path. Slow and steady, we'll get there!

[eddyb]

^{I'm really sorry I wasn't able to carry this to completion.}

Solving the diesel overflow (or turning it into a warning on all editions?) would be needed.

Excluding Sized means we get to pretend type aliases don't have Sized by default like traits do - we might even be able to implement this and warn that ?Sized is a noop?

But overall, if we don't break anything on the edition, there's still hope.
We can integrate type aliases into the typesystem while marking their obligations as lint-only.
That's less likely to cause regressions (ignoring Sized, ofc) than denying "too many bounds", because those bounds are likely to actually be satisfied wherever the type alias is used.

I think one of my PRs has enough type/trait diagnostic infrastructure improvements for this, we just need to get lazy normalization to be able to defer resolving the alias.

[nikomatsakis]

(I still plan on following up on this, though I'm focusing my immediate energy more on the universe transition and lazy normalization.)

[Centril]

and lazy normalization.

🙏

[nikomatsakis]

I was thinking about this some more. It may be that we can preserve the existing behavior, though it's not entirely clear we want to, for consistency reasons. But backwards compatibility may be king here.

The basic idea was that we wanted to treat the use Foo<X1..Xn> of a type alias Foo:

type Foo<P0...Pn> = Bar;

as effectively "desugaring" to something like <() as FooTrait<X1..Xn>>::Out where

trait FooTrait<P0...Pn> {
    type Out;
}

impl<P0...Pn> FooTrait<P0..Pn> for () {
    type Out = Bar;
}

The problem here is that we would, given our ordinary rules around impls, try to prove that (in chalk terms):

forall<P0...Pn> { WF(Bar) }

and -- for many types Bar -- we might not be able to prove that.

However, we could also desugar to an impl like this:

impl<P0...Pn> FooTrait<P0..Pn> for ()
where
    WF(Bar)
{
    type Out = Bar;
}

This would effectively push the problem of proving that the expansion type Bar is well-formed up to the place where we do the projection (ie, where we have to prove that <() as FooTrait<X1::Xn>>::Out can be normalized, I guess).

At this point you could certainly imagine transitioning this scheme "slightly" on an edition boundary, and it'd be relatively easy to do.

(cc @scalexm, who may have thoughts on whether this makes any sense)

[alexreg]

@nikomatsakis Would we actually want to desugar to trait and impl, or directly to Chalk. Anyway, I am intuitively in favour of "including the where-clause" and thus pushing the duty of proving the WF up to the use site site. I reckon that would work fairly well.

Should we get @eddyb's thoughts on this too?

[nikomatsakis]

We would not actually desugar to traits/impls, but rather to (roughly equivalent) chalk predicates.

[alexreg]

In that case, sounds like a reasonable plan. The question is now how necessary lazy normalisation is... anyway, if either @eddyb or @scalexm is interesting in working on this, that would be super. (I can offer my help too.)

[scalexm]

@nikomatsakis I've not followed everything. So the deal is that in this program:

struct Bar<T: Clone>(T);

type Foo<T> = Bar<T>; // (*)

we'd want the line (*) to error out even in the presence of implied bounds, but only after some transition period, right?

Regarding your desugaring, I'm always a bit wary about having WF(Type) as a where clause. Indeed, in chalk's model for implied bounds, having WF(Type) in your environment does nothing: what you'd need is FromEnv(Type) instead. That could work provided that we implement rustc_traits::lowering::IntoFromEnvGoal for DomainGoal::WellFormed, but it seems a bit like a hack.

I presume that in every case, we need chalk to implement your desugaring (because having WF(Type) in your environment with the current trait solver does nothing either). Then I wonder if we could instead do something like:

type Foo<P1...Pn> = Bar<...>;

// Desugars to
trait FooTrait<X> {
    type Out;
}

impl<P1...Pn> FooTrait<Bar<...>> for (P1, ..., Pn) {
    type Out = Bar<...>;
}

This way, Bar<...> is now an input type of the impl, so FromEnv(Bar<...>) is in the impl's environment by design.

Then it would be used like so:

let x: Foo<X1...Xn>;

// Desugars to

let x: <(X1, ..., Xn) as FooTrait<_>>::Out;

Because FooTrait is a local trait and there exists exactly one implementation of this trait, we could use type inference to deduce that _ is actually Bar<...>. Then proving that Bar<...> is well-formed would indeed naturally be done at the use site because we check that all types appearing in a function (including resolved type vars) are well-formed.

[nikomatsakis]

@scalexm

we'd want the line (*) to error out even in the presence of implied bounds, but only after some transition period, right?

Actually, if we adopted the desugaring I was proposed, then I would not expect any error in that program. The error would occur later, when Foo is applied to some type (and normalized).

I think that — all things being equal — I would want your example to be an error at the point you indicated. But backwards compatibility may be a factor here; I could imagine trying to make that change over an edition, for example.

Regarding your desugaring, I'm always a bit wary about having WF(Type) as a where clause.
... That could work provided that we implement rustc_traits::lowering::IntoFromEnvGoal for DomainGoal::WellFormed, but it seems a bit like a hack.

What makes that seem like a hack to you? It doesn't seem particularly different to me from the usual shift we do on where-clauses.

Then I wonder if we could instead do something like:

Interesting. I suppose that is equivalent. (I think this also means that we wouldn't get an error until the use-site, right?)

[scalexm]

@nikomatsakis

What makes that seem like a hack to you? It doesn't seem particularly different to me from the usual shift we do on where-clauses.

On where-clauses, we map Implemented(T: Trait) to FromEnv(T: Trait), that's all. The thing is that we never have WF(T: Trait) or WF(Type) as a where clause (for example because a user cannot even write it).

Interesting. I suppose that is equivalent. (I think this also means that we wouldn't get an error until the use-site, right?)

Right.

[nikomatsakis]

cc'ing #51626, which presents an interesting case to remember -- basically, if you have a type with a default but the default makes the resulting type not well formed

[Centril]

cc #21903

[nikomatsakis]

We had a design meeting on this topic, notes and recording available here.

[RalfJung]

@rust-lang/types I assume the general problem area around type aliases falls under your purview? I wonder what the plans are here for properly Wf-checking type aliases?

[oli-obk]

I'm assuming we'll need something like #97974 as a first step, though there may be other implementation avenues we could explore.

[fmease]

Has this issue been superseded by the tracking issue for lazy type aliases (#112792)? CC #108860.