avoid making substs of type aliases late bound when used as fn args

[BoxyUwU]

fixes #47511
fixes #85533
(although I did not know theses issues existed when i was working on this 🙃)

currently Alias<...> is treated the same as Struct<...> when deciding if generics should be late bound or early bound but this is not correct as Alias might normalize to a projection which does not constrain the generics.

I think this needs more tests before merging
more explanation of PR here

Hackmd inline for future readers:

This assumes reader is familiar with the concept of early/late bound lifetimes. There's a section on rustc-dev-guide if not (although i think some details are a bit out of date)

problem & background

Not all lifetimes on a fn can be late bound:

fn foo<'a>() -> &'a ();
impl<'a> Fn<()> for FooFnDef {
    type Output = &'a (); // uh oh unconstrained lifetime
}

so we make make them early bound

fn foo<'a>() -> &'a ();
impl<'a> Fn<()> for FooFnDef<'a> {// wow look at all that lifetimey
     type Output = &'a ();   
}   

(Closures have the same constraint however it is not enforced leading to soundness bugs, #84385 implements this "downgrading late bound to early bound" for closures)

lifetimes on fn items are only late bound when they are "constrained" by the fn args:

fn foo<'a>(_: &'a ()) -> &'a ();
//               late bound, not present on `FooFnItem`
//               vv
impl<'a> Trait<(&'a (),)> for FooFnItem {
    type Output = &'a ();    
}

// projections do not constrain inputs
fn bar<'a, T: Trait>(_: <T as Trait<'a>>::Assoc) -> &'a (); //  early bound
                                                            //  vv
impl<'a, T: Trait> Fn<(<T as Trait<'a>>::Assoc,)> for BarFnItem<'a, T> {
    type Output = &'a ();
}

current logic for determining if inputs "constrain" a lifetime works off of HIR so does not normalize aliases. It also assumes that any path with no self type constrains all its substs (i.e. Foo<'a, u32> has no self type but T::Assoc does). This falls apart for top level type aliases (see linked issues):

type Alias<'a, T> = <T as Trait<'a>>::Assoc;
//                      wow look its a path with no self type uwu
//                      i bet that constrains `'a` so it should be latebound
//                      vvvvvvvvvvv
fn foo<'a, T: Trait>(_: Alias<'a, T>) -> &'a ();
//                     `Alias` normalized to make things clearer
//                     vvvvvvvvvvvvvvvvvvvvvvv
impl<'a, T: Trait> Fn<(<T as Trait<'a>>::Assoc,)> for FooFnDef<T> {
    type Output = &'a (); 
    // oh no `'a` isnt constrained wah wah waaaah *trumbone noises* 
    // i think, idk what musical instrument that is
}

solution

The PR solves this by having the hir visitor that checks for lifetimes in constraining uses check if the path is a DefKind::Alias. If it is we ""normalize"" it by calling type_of and walking the returned type. This is a bit hacky as it requires a mapping between the substs on the path in hir, and the generics of the type Alias<...> which is on the ty layer.

Alternative solutions may involve calculating the "late boundness" of lifetimes after/during astconv rather than relying on hir at all. We already have code to determine whether a lifetime SHOULD be late bound or not as this is currently how the error for fn foo<'a, T: Trait>(_: Alias<'a, T>) -> &'a (); gets emitted.

It is probably not possible to do this right now, late boundness is used by generics_of and gather_explicit_predicates_of as we currently do not put late bound lifetimes in Generics. Although this seems sus to me as the long term goal is to make all generics late bound which would result in generics_of(function) being empty? #103448 places all lifetimes in Generics regardless of late boundness so that may be a good step towards making this possible.

[rust-highfive]

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

[oli-obk]

This probably needs lang team sign off as it adds potentially a lot more code that compiles.

[bors]

☔ The latest upstream changes (presumably #101152) made this pull request unmergeable. Please resolve the merge conflicts.

[BoxyUwU]

I'm not sure that this is really "a lot more code that compiles" (the situations in which this matter are relatively uncommon since you need the lifetime to be used in the return type and only used in the arguments as a projection hidden behind a type alias). It's also imo a clear bug so it doesn't seem T-lang but more T-types, it would probably be worth getting t-types input anyway to make sure this is not going to mess up any future plans.

[jackh726]

Niko is going to review this r? @nikomatsakis

Going to unnominate

[nikomatsakis]

r=me modulo the need for an explanatory comment and a cross-crate test

[nikomatsakis]

Discussed with @BoxyUwU -- in the fullness of time, I hope this goes away by removing the early/late distinction (and moving everything into generics), but for the time being, I agree with the reasoning of the hackmd that this is the best path forward.

The only real alternative I can see is being very conservative and making everything early bound that appears in a type alias. It seems very likely this will break people and I would be hard pressed to explain to users why it does, so I'm not inclined to go that path unless we find a real complication with this approach beyond that it's a messy hack.

The major concerns I can see are

• it might lead to cycles if it were possible for type_of on the type alias to invoke the function, but I don't see why that would happen, at least today
• duplicate code paths that could get out of sync, but this code changes very infrequently

[BoxyUwU]

@bors r=nikomatsakis

[bors]

📌 Commit 983a90d has been approved by nikomatsakis

It is now in the queue for this repository.