trait_solver: Reject canonical responses with placeholder universe leaks

[Dnreikronos]

Fixes #160802

-Zassumptions-on-binders plus an impl for<T> Trait<T> RPIT was ICEing in proof-tree replay when unify_query_var_values hit .unwrap() on a Sorts error for the same binder placeholder living in two universes. Imo that unwrap was always a bit cursed on the diagnostics path, idk why we kept trusting it this long.

Canonical response/state apply is fallible now and turns those leaks into NoSolution instead of panicking. Region-only placeholders don't block valid region lowering. Also made CoerceVisitor Break when nested candidate replay fails, since Continue was treating a failed instantiate as "unsizing ok" (btw VisitorResult::output is Continue for ControlFlow, easy to miss). Added the issue repro plus a few universe-leak regressions.

Fyi the prev_universe proof-tree fix is already on main and I kept it. Irl I'd rather get this in asap than bike-shed the diagnostics wording further, but ltm if you'd rather the failed replay surface louder than a quiet NoSolution.

[rustbot]

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

[rustbot]

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 75 candidates
• Random selection from 19 candidates

[folkertdev]

r? types

[oli-obk]

r? @BoxyUwU for assumptions on binders stuff

[rustbot]

BoxyUwU is currently at their maximum review capacity.
They may take a while to respond.

[BoxyUwU]

I don't understand how this differs from #159725. Are these PRs not both handling the case of placeholder types/consts in canonical responses resulting in universe errors?

I think that in general we don't really want to be complicating the trait solver for non_lifetime_binders support, especially by making canonical responses fallible since iirc there were good reasons why we made them infallible (though I could be wrong about that.

I don't understand what about this test case is made worse by -Zassumptions-on-binders. Can you write a bit about what goals we're proving and what binders we have and what the involved region constraints are and their associated rewriting steps? That'll help me figure out whether there's some other solution we can do here or if we should just wontfix this for the time being

[Dnreikronos]

Sup, @BoxyUwU :)!
yeah it's the same thing as #159725. Type/const placeholders coming back in a canonical response, caller can't name them, things explode.

I still pushed this one because the AOB ICE looks different (impl for<T> Trait<T> on an RPIT) and because diagnostics was still hitting that unwrap when replaying the proof tree. Region placeholders I left alone on purpose, those do need to lower or HRTB starts failing.

I made apply fallible because I don't want to put a placeholder into a var that can't see it. The query never notices. Canonicalization dumps both sides in the root universe, leak check is looking at that, and only when we instantiate again do we have the same !T in two universes. That's when writeback gets garbage or diagnostics panics. Returning NoSolution there felt better than keeping the unwrap. I know responses are supposed to always apply. I still think applying this one is wrong.

For the AOB test we're proving i32: for<T> Trait<T>. Enter the binder, T becomes a placeholder, then the impl proves i32: Trait<!T>. That's the easy part. AOB skips leak check and tries to rewrite region constraints when leaving that universe. There's no region binder though, so the assumptions are basically nothing. idk if that rewrite is even involved or if it's just next-solver seeing a type placeholder like the other PR. AOB turns next-solver on, and this already compiles on the old solver.

The panic is !T from one universe vs the same !T from another, during proof tree replay. Same story as #159725.

imo applying that is wrong. I also don't want to twist the solver around non_lifetime_binders if nobody's maintaining it. wontfix is reasonable. I'd still take the crash out though, panicking in diagnostics for this feels worse than the goal just failing. Please, let me know what you think about it :)