trait_solver: normalize next-gen region constraints

[Dnreikronos]

View all comments

fixes #157729

-zassumptions-on-binders can produce next-gen region constraints that mean the same thing but don't have the same shape. in this case object candidate merging stayed ambiguous and instance resolution later hit the ice. imo normalizing the constraint at the response boundary is the least weird place for this, because candidate selection shouldn't need to know which vtable looks nicer.

canonicalize and evaluate the next-gen region constraint before response canonicalization, then cover the dyn derived

supertrait case from the issue. lgtm locally with the focused test, tests/ui/traits/next-solver, and tests/ui/assumptions_on_binders. idk if there's a better home for the helper call, but ltm this keeps the fix pretty narrow.

[rustbot]

r? @camelid

rustbot has assigned @camelid.
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 73 candidates
• Random selection from 19 candidates

[camelid]

I don't have familiarity with this code, so rerolling. @rustbot reroll

[mati865]

Me neither, @rustbot reroll

[hanna-kruppe]

Me three. Let's try something more targeted: r? types

[lcnr]

r? BoxyUwU

[rustbot]

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

[BoxyUwU]

so it makes sense to me why having a non-normal form constraint here would cause issues 🤔 what I don't get is why we have a non-normal one here. Though admittedly we don't put much effort into tracking where/when we expect stuff to be normal form lol.

I would add an assertion to this that self.delegate.get_solver_region_constraint() returns something for which evaluating it & computing a canonical form doesn't actually do anything. In theory the only way we should call compute_external_query_constraints should be if we've already called eagerly_handle_placeholders which should evaluate/convert to normal form the constraints and then set that back into the infcx/delegate 🤔

it seems to me that for whatever reason eagerly_handle_placeholders isnt actually giving back normal form constraints for whatever reason, I would expect the "right" fix would be to look into why that's happening and see if we can fix that

View changes since the review

[rust-bors]

💔 Test for 7e65599 failed: CI

[JonathanBrouwer]

@bors retry
Spurious

[jhpratt]

@bors r- #159926 (comment)

diff of stderr:

20 LL | |     Self::Item: Baz,
21    | |____________________^
22    = note: ...which again requires coherence checking all impls of trait `Foo`, completing the cycle
- note: cycle used when checking that `<impl at $DIR/next-solver-region-resolution.rs:12:1: 14:20>` is well-formed
-   --> $DIR/next-solver-region-resolution.rs:12:1
+ note: cycle used when checking that `<impl at $DIR/next-solver-region-resolution.rs:18:1: 20:21>` is well-formed
+   --> $DIR/next-solver-region-resolution.rs:18:1
25    |
- LL | / impl<'a, T> Foo for &'a T
+ LL | / impl<'a, T> Foo for &T
27 LL | | where
- LL | |     Self::Item: 'a,
-    | |___________________^
+ LL | |     Self::Item: Baz,
+    | |____________________^
30    = note: for more information, see <https://rustc-dev-guide.rust-lang.org/overview.html#queries> and <https://rustc-dev-guide.rust-lang.org/query.html>
31 
32 error: aborting due to 1 previous error

Note: some mismatched output was normalized before being compared
- note: cycle used when checking that `<impl at /checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs:18:1: 20:21>` is well-formed
-   --> /checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs:18:1
+ note: cycle used when checking that `<impl at $DIR/next-solver-region-resolution.rs:18:1: 20:21>` is well-formed
+   --> $DIR/next-solver-region-resolution.rs:18:1
+ LL | / impl<'a, T> Foo for &T
+ LL | |     Self::Item: Baz,
+    | |____________________^

Compare output by lines enabled, diff by lines:
Expected contains these lines that are not in actual:
   | |___________________^
  --> $DIR/next-solver-region-resolution.rs:12:1
LL | / impl<'a, T> Foo for &'a T
LL | |     Self::Item: 'a,
note: cycle used when checking that `<impl at $DIR/next-solver-region-resolution.rs:12:1: 14:20>` is well-formed
Actual contains these lines that are not in expected:
   | |____________________^
  --> $DIR/next-solver-region-resolution.rs:18:1
LL | / impl<'a, T> Foo for &T
LL | |     Self::Item: Baz,
note: cycle used when checking that `<impl at $DIR/next-solver-region-resolution.rs:18:1: 20:21>` is well-formed

The actual stderr differed from the expected stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args specialization/min_specialization/next-solver-region-resolution.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "-Zthreads=4" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/specialization/min_specialization/next-solver-region-resolution" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "incomplete_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Znext-solver=globally"
stdout: none
--- stderr -------------------------------
error[E0391]: cycle detected when coherence checking all impls of trait `Foo`
##[error]  --> /checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs:6:1
   |
LL | trait Foo { //~ ERROR cycle detected when coherence checking all impls of trait `Foo`
   | ^^^^^^^^^
   |
   = note: ...which requires building specialization graph of trait `Foo`...
note: ...which requires computing whether impls specialize one another...
  --> /checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs:12:1
   |
LL | / impl<'a, T> Foo for &'a T
LL | | where
LL | |     Self::Item: 'a,
   | |___________________^
note: ...which requires computing normalized predicates of `<impl at /checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs:18:1: 20:21>`...
  --> /checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs:18:1
   |
LL | / impl<'a, T> Foo for &T
LL | | where
LL | |     Self::Item: Baz,
   | |____________________^
   = note: ...which again requires coherence checking all impls of trait `Foo`, completing the cycle
note: cycle used when checking that `<impl at /checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs:18:1: 20:21>` is well-formed
  --> /checkout/tests/ui/specialization/min_specialization/next-solver-region-resolution.rs:18:1
   |
LL | / impl<'a, T> Foo for &T
LL | | where
LL | |     Self::Item: Baz,
   | |____________________^
   = note: for more information, see <https://rustc-dev-guide.rust-lang.org/overview.html#queries> and <https://rustc-dev-guide.rust-lang.org/query.html>

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0391`.
------------------------------------------

[rust-bors]

This pull request was unapproved.

This PR was contained in a rollup (#159926), which was unapproved.

View changes since this unapproval