Allow instantiating object trait binder when upcasting

[compiler-errors]

This PR fixes two bugs (that probably need an FCP).

We use equality rather than subtyping for upcasting dyn conversions

This code should be valid:

#![feature(trait_upcasting)]

trait Foo: for<'h> Bar<'h> {}
trait Bar<'a> {}

fn foo(x: &dyn Foo) {
    let y: &dyn Bar<'static> = x;
}

But instead:

error[E0308]: mismatched types
 --> src/lib.rs:7:32
  |
7 |     let y: &dyn Bar<'static> = x;
  |                                ^ one type is more general than the other
  |
  = note: expected existential trait ref `for<'h> Bar<'h>`
             found existential trait ref `Bar<'_>`

And so should this:

#![feature(trait_upcasting)]

fn foo(x: &dyn for<'h> Fn(&'h ())) {
    let y: &dyn FnOnce(&'static ()) = x;
}

But instead:

error[E0308]: mismatched types
 --> src/lib.rs:4:39
  |
4 |     let y: &dyn FnOnce(&'static ()) = x;
  |                                       ^ one type is more general than the other
  |
  = note: expected existential trait ref `for<'h> FnOnce<(&'h (),)>`
             found existential trait ref `FnOnce<(&(),)>`

Specifically, both of these fail because we use equality when comparing the supertrait to the target of the unsize goal. For the first example, since our supertrait is for<'h> Bar<'h> but our target is Bar<'static>, there's a higher-ranked type mismatch even though we should be able to instantiate that supertrait binder when upcasting. Similarly for the second example.

New solver uses equality rather than subtyping for no-op (i.e. non-upcasting) dyn conversions

This code should be valid in the new solver, like it is with the old solver:

// -Znext-solver

fn foo<'a>(x: &mut for<'h> dyn Fn(&'h ())) {
   let _: &mut dyn Fn(&'a ()) = x;
}

But instead:

error: lifetime may not live long enough
 --> <source>:2:11
  |
1 | fn foo<'a>(x: &mut dyn for<'h> Fn(&'h ())) {
  |        -- lifetime `'a` defined here
2 |    let _: &mut dyn Fn(&'a ()) = x;
  |           ^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static`
  |
  = note: requirement occurs because of a mutable reference to `dyn Fn(&())`

Specifically, this fails because we try to coerce &mut dyn for<'h> Fn(&'h ()) to &mut dyn Fn(&'a ()), which registers an dyn for<'h> Fn(&'h ()): dyn Fn(&'a ()) goal. This fails because the new solver uses equating rather than subtyping in Unsize goals.

This is mostly not a problem... You may wonder why the same code passes on the new solver for immutable references:

// -Znext-solver

fn foo<'a>(x: &dyn Fn(&())) {
   let _: &dyn Fn(&'a ()) = x; // works
}

That's because in this case, we first try to coerce via Unsize, but due to the leak check the goal fails. Then, later in coercion, we fall back to a simple subtyping operation, which does work.

Since &T is covariant over T, but &mut T is invariant, that's where the discrepancy between these two examples crops up.

---

r? lcnr or reassign :D

[lcnr]

we previously incorrectly used sub there in the wrong order, resulting in an unsoundness fixed by #119849, cc https://github.com/rust-lang/rust/pull/119849/files#r1525200073

I fully agree that not requiring the binders to be equal is desirable here.

I would like to manually instantiate the binders here instead of relying on sub. This is easier to read imo and makes it far clearer what's going on.

r=me after FCP

[lcnr]

Wait, why would this need an FCP? trait upcasting is still unstable, is it not?

[compiler-errors]

Oh yeah, I guess it's double unstable: new solver and/or feature gate.

[lcnr]

alright, so r=me after manually instantiating the binders :3 unless you're strongly opposed to doing that

[compiler-errors]

No, instantiating the binder is a bit more verbose but I agree it makes it much clearer who is the placeholders and who is the infer vars.

[compiler-errors]

Could use another review. The implementation is less than beautiful in both the new and old solvers:

1. In the new solver, we need to introduce enter_binder_mut to be able to call enter_binder and then use &mut EvalCtxt within it.
2. In the old solver, we need to do some trickery with TypeTraces to make sure that we don't regress "expected for<'a, 'b> Foo<'a, 'b> found for<'a> Foo<'a, 'a>" into "expected for<'a, 'b> Foo<'a, 'b> found Foo<'_, '_>" because of the fact that we instantiate the binder with infer.

[lcnr]

given that enter_forall is only used once, maybe it's easier to always use enter_forall_mut by modifying the signature of neter_forall.

While you're at it, change fn compute_goal to use self.enter_forall from self.delegate.enter_forall 😁

[compiler-errors]

I remember there being a specific problem where we had an infectious &mut self that ended up changing quite a lot if we did this

[compiler-errors]

nvm, just needed to move an arg lol

[lcnr]

r=me after nits

[compiler-errors]

@bors r=lcnr

[bors]

📌 Commit d753aba has been approved by lcnr

It is now in the queue for this repository.