fix regression with the next-generation trait solver

[lcnr]

Hi there 👋 unfortunately minijinja currently relies on a type system bug. This code will break with the -Znext-solver.

Let's look at the relevant section of the minimization in rust-lang/trait-system-refactor-initiative#195 (comment) [godbolt]:

trait Arg<'a> {
    type Output;
}

trait Filter<A> {}
impl<A, F: FnMut(A)> Filter<A> for F {}

fn inspect_values<F, A>(_: F)
where
    A: for<'a> Arg<'a>,
    F: for<'a> Filter<<A as Arg<'a>>::Output> + Filter<A>,
{
}

fn generic_caller<F, A>(f: F)
where
    A: for<'a> Arg<'a>,
    F: for<'a> Filter<<A as Arg<'a>>::Output> + Filter<A>,
{
    inspect_values(f)
}

The call to inspect_values starts out with A and F being unconstrained inference variables, let's call them ?a and ?f. ?f then gets constrained to the F parameter from generic_caller. This leaves us with the following trait bounds:

• ?a: for<'a> Arg<'a>
• F: for<'a> Filter<<?a as Arg<'a>>::Output>
• F: Filter<?a>

One of these bounds has to constrain ?a to the A parameter for this to compile. ?a: for<'a> Arg<'a> certainly cannot do so, so we're left with the F: Filter bounds.

---

F: Filter<?a> can be proven by either using the F: for<'a> Filter<<A as Arg<'a>>::Output> or the F: Filter<A> where-bound of generic_caller. The first instantiates ?a with <<A as Arg<'a>>::Output, the second with A. As both where-bounds are valid, this remains ambiguous.

---

F: for<'a> Filter<<?a as Arg<'a>>::Output> can in theory also be proven by using both where-bounds of generic_caller. The fact that F: for<'a> Filter<<A as Arg<'a>>::Output> may apply should be clear, but F: Filter<A> is also totally valid. We'd just need to instantiate ?a with some type T for which <T as Arg<'a>>::Output normalizes to A.

What's even more subtle is that even if we use the F: for<'a> Filter<<A as Arg<'a>>::Output> where-bound to to prove F: for<'a> Filter<<?a as Arg<'a>>::Output>, this does not necessarily have to constrain ?a to A. It would also be valid to instantiate ?a with some type T != A for which <T as Arg<'a>>::Output normalizes to <A as Arg<'a>>::Output>.

---

The current type system implementation incorrectly relates associated types structurally, i.e. by simply checking that their generic arguments are equal, if these associated types reference higher ranked regions, the 'a from for<'a>. This can result in a lot of weird errors and questionable inference behavior, see rust-lang/trait-system-refactor-initiative#9 for an example. Trying to support this behavior with the new implementation is pretty much impossible without adding explicit hacks for each individual breakage.

See rust-lang/trait-system-refactor-initiative#195 and rust-lang/trait-system-refactor-initiative#168 for more details about the breakage.

What this PR is doing

To avoid the ambiguity errors with -Znext-solver, I am moving the duplicate where-bounds into the Function impl so that generic functions have a single F: Function where-bound which references A directly.

I don't believe this to be a breaking change unless people try to use the Function trait for functions whose signature is not higher ranked, i.e. fn(&'a str) for some early-bound lifetime 'a. However, given that fn invoke is #[doc(hidden)] and all uses of the trait inside of minijinja already had a higher-ranked bound, this should not be an issue.

If you think this change is acceptable, it would be awesome to also backport this change to older versions as the crater run in rust-lang/rust#133502 discovered quite a few projects which depend on older versions of this crate.

I am sorry for the inconvenience and please ask any questions if you would like some additional clarification. Thanks :>

[mitsuhiko]

I was already expecting this to be a bit hacky since I managed to trigger some internal compiler logs in some error conditions. Unfortunately it looks like some crates are stuck on a really old version with slightly different interfaces. I'll have a look though if I can backport this though.

[mitsuhiko]

Closing for #788 because I cannot push to your branch.

Leaving it open for now until it's in a mergeable state.

[mitsuhiko]

This is great. Since I still can't push to this, could you run rustfmt for me?

[lcnr]

Alright, as stated in #788 (comment) proving goals via item bounds - i.e. the fact that impl Trait: Trait - does not try to normalize associated items after instantiating the binder with placeholders. This then causes us to structurally relate a normalizeable alias with its underlying type, causing a type mismatch.

We can solve this by hiding the goal we want to prove via an item bound inside of another impl. This means there's now a normalization call between instantiating the binder and proving the goal, meaning that the associated type has already been normalized when we try to prove it by using the item bound of the opaque type. It's a bit annoying that we can't just do trait FnIndir<Rv, Args>: Fn<Args, Output = Rv> to support different arities, so it needs to go through another fn invoke instead.

Isn't Rust beautiful :3

[mitsuhiko]

Well I consider this generally a step in the right direction because it hides some of that lifetime mess away from the more public interfaces (unless someone looks closer at the blanket impls of Function :P).

[lcnr]

wrote gi push -f, woops 😅