Skip to content

Avoid manually instantiating some binders in error reporting with -Znext-solver - #162262

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
sjwang05:forall-binder-ice
Sep 5, 2026
Merged

Avoid manually instantiating some binders in error reporting with -Znext-solver#162262
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
sjwang05:forall-binder-ice

Conversation

@sjwang05

@sjwang05 sjwang05 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

If we call enter_forall on a binder before passing it to the new solver, the placeholders that come from instantiation get canonicalized before solving happens. This can be a problem if we end up equating one variable whose universe can't name another variable's, since the equate will succeed in the solver since canonicalization sets all universe indices to the root universe but fail when we actually construct the response with fresh vars at the correct universes, which causes an ICE. We now just hand the unopened binder to the trait solver directly, since (I believe) both solvers are able to handle that just fine.

This came up while I was looking at https://rust-lang.zulipchat.com/#narrow/channel/618216-t-types.2Fcall-for-participation/topic/next-solver.20UI.20test.20triage/with/612965995, and the universe-error-host-effect.rs/poly_select change was basically the same type of ICE that I found while investigating the first one.

@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to the core trait solver

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Sep 3, 2026
@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 22 candidates

@fee1-dead

fee1-dead commented Sep 4, 2026

Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot

This comment was marked as resolved.

@rustbot rustbot assigned Enselic and unassigned fee1-dead Sep 4, 2026
| -----^-----
| | |
| | the trait solver is unable to infer the generic types that should be inferred from this argument
| add turbofish arguments to this call to specify the types manually, even if it's redundant

@sjwang05 sjwang05 Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of a diagnostics regression, since before, we called ocx.normalize on the obligation before registering it, whereas now it doesn't seem like we can handle the unnormalized <Self as ATC<'a>>::Type inside the binder if it's not normalized beforehand.

View changes since the review

@Enselic

Enselic commented Sep 4, 2026

Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned mati865 and unassigned Enselic Sep 4, 2026

@ShoyuVanilla ShoyuVanilla Sep 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used to normalize the predicate here. Don't we need it anymore?

View changes since the review

@mati865

mati865 commented Sep 4, 2026

Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned ShoyuVanilla and unassigned mati865 Sep 4, 2026
@BoxyUwU

BoxyUwU commented Sep 4, 2026

Copy link
Copy Markdown
Member

reminds me a bit of #160955. tbh in general im not convinced that erasing universes in the inputs makes sense if we have placeholder types... 🤷‍♀️ I don't have the capacity to properly engage with this though 😅 seems like a nice enough cleanup regardless so...

r? me

@bors r+

@rust-bors

rust-bors Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 368791e has been approved by BoxyUwU

It is now in the queue for this repository.

@rust-bors rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 4, 2026
@rust-bors rust-bors Bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 4, 2026
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 4, 2026
Avoid manually instantiating some binders in error reporting with `-Znext-solver`

If we call `enter_forall` on a binder before passing it to the new solver, the placeholders that come from instantiation get canonicalized before solving happens. This can be a problem if we end up equating one variable whose universe can't name another variable's, since the equate will succeed in the solver since canonicalization sets all universe indices to the root universe but fail when we actually construct the response with fresh vars at the correct universes, which causes an ICE. We now just hand the unopened binder to the trait solver directly, since (I believe) both solvers are able to handle that just fine.

This came up while I was looking at https://rust-lang.zulipchat.com/#narrow/channel/618216-t-types.2Fcall-for-participation/topic/next-solver.20UI.20test.20triage/with/612965995, and the `universe-error-host-effect.rs`/`poly_select` change was basically the same type of ICE that I found while investigating the first one.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 4, 2026
Avoid manually instantiating some binders in error reporting with `-Znext-solver`

If we call `enter_forall` on a binder before passing it to the new solver, the placeholders that come from instantiation get canonicalized before solving happens. This can be a problem if we end up equating one variable whose universe can't name another variable's, since the equate will succeed in the solver since canonicalization sets all universe indices to the root universe but fail when we actually construct the response with fresh vars at the correct universes, which causes an ICE. We now just hand the unopened binder to the trait solver directly, since (I believe) both solvers are able to handle that just fine.

This came up while I was looking at https://rust-lang.zulipchat.com/#narrow/channel/618216-t-types.2Fcall-for-participation/topic/next-solver.20UI.20test.20triage/with/612965995, and the `universe-error-host-effect.rs`/`poly_select` change was basically the same type of ICE that I found while investigating the first one.
rust-bors Bot pushed a commit that referenced this pull request Sep 4, 2026
Rollup of 27 pull requests

Successful merges:

 - #159074 ([PAC] FnAbi, llvm.ptrauth.resign and Session API change (2/8))
 - #159792 (A more readable debug map for IndexMaps)
 - #160745 (make closures act like MaybeDangling)
 - #161940 (Promote `wasm32-wasip3` to a tier 2 target)
 - #162030 (Prevent `--test` to be used in `rustdoc-html` testsuite)
 - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`)
 - #162179 (type system const items via direct rhs)
 - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`)
 - #162277 (Introduce `rustc_middle::middel::resolve`)
 - #162285 (box: fixup map/try_map deallocate calls)
 - #162286 (string: don't unwind prematurely)
 - #162289 (alloc: a bunch of safety comments)
 - #162290 (abby test DSL: AliasTyOutlivesViaEnv)
 - #162292 (Update `askama` version to `0.16.1`)
 - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)
 - #160906 (Suggest usize instead of placeholder type for array length constants)
 - #160936 (traits: Represent live alias arguments as bitsets)
 - #161400 (Improve diagnostics for references to closures)
 - #161656 (Suggest mutable references for FnMut closure arguments)
 - #161711 (Add more splat fn type tests)
 - #161786 (Make `tcx.def_id_partial_cmp` public)
 - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers)
 - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute)
 - #162212 (Implement `Rng` for `Box`)
 - #162246 (Fix incorrect meta span)
 - #162266 (std: fix typo)
 - #162291 (Add regression test from 1.98.1)
rust-bors Bot pushed a commit that referenced this pull request Sep 5, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`)
 - #162179 (type system const items via direct rhs)
 - #162255 (let people change rustc-dev-guide in-tree)
 - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`)
 - #162277 (Introduce `rustc_middle::middel::resolve`)
 - #162285 (box: fixup map/try_map deallocate calls)
 - #162286 (string: don't unwind prematurely)
 - #162289 (alloc: a bunch of safety comments)
 - #162290 (abby test DSL: AliasTyOutlivesViaEnv)
 - #162292 (Update `askama` version to `0.16.1`)
 - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)
 - #160906 (Suggest usize instead of placeholder type for array length constants)
 - #160936 (traits: Represent live alias arguments as bitsets)
 - #161394 (fix `is_homogeneous_aggregate`: use unit size, not total size )
 - #161400 (Improve diagnostics for references to closures)
 - #161656 (Suggest mutable references for FnMut closure arguments)
 - #161711 (Add more splat fn type tests)
 - #161786 (Make `tcx.def_id_partial_cmp` public)
 - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers)
 - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute)
 - #162212 (Implement `Rng` for `Box`)
 - #162246 (Fix incorrect meta span)
 - #162266 (std: fix typo)
 - #162291 (Add regression test from 1.98.1)
 - #162303 (Fix a minor spelling error in `library/std/src/sys/fs/unix.rs`)
rust-bors Bot pushed a commit that referenced this pull request Sep 5, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`)
 - #162179 (type system const items via direct rhs)
 - #162255 (let people change rustc-dev-guide in-tree)
 - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`)
 - #162277 (Introduce `rustc_middle::middel::resolve`)
 - #162285 (box: fixup map/try_map deallocate calls)
 - #162286 (string: don't unwind prematurely)
 - #162289 (alloc: a bunch of safety comments)
 - #162290 (abby test DSL: AliasTyOutlivesViaEnv)
 - #162292 (Update `askama` version to `0.16.1`)
 - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)
 - #160906 (Suggest usize instead of placeholder type for array length constants)
 - #160936 (traits: Represent live alias arguments as bitsets)
 - #161394 (fix `is_homogeneous_aggregate`: use unit size, not total size )
 - #161400 (Improve diagnostics for references to closures)
 - #161656 (Suggest mutable references for FnMut closure arguments)
 - #161711 (Add more splat fn type tests)
 - #161786 (Make `tcx.def_id_partial_cmp` public)
 - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers)
 - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute)
 - #162212 (Implement `Rng` for `Box`)
 - #162246 (Fix incorrect meta span)
 - #162266 (std: fix typo)
 - #162291 (Add regression test from 1.98.1)
 - #162303 (Fix a minor spelling error in `library/std/src/sys/fs/unix.rs`)
@rust-bors
rust-bors Bot merged commit 772a5dd into rust-lang:main Sep 5, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 5, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 5, 2026
Rollup merge of #162262 - sjwang05:forall-binder-ice, r=BoxyUwU

Avoid manually instantiating some binders in error reporting with `-Znext-solver`

If we call `enter_forall` on a binder before passing it to the new solver, the placeholders that come from instantiation get canonicalized before solving happens. This can be a problem if we end up equating one variable whose universe can't name another variable's, since the equate will succeed in the solver since canonicalization sets all universe indices to the root universe but fail when we actually construct the response with fresh vars at the correct universes, which causes an ICE. We now just hand the unopened binder to the trait solver directly, since (I believe) both solvers are able to handle that just fine.

This came up while I was looking at https://rust-lang.zulipchat.com/#narrow/channel/618216-t-types.2Fcall-for-participation/topic/next-solver.20UI.20test.20triage/with/612965995, and the `universe-error-host-effect.rs`/`poly_select` change was basically the same type of ICE that I found while investigating the first one.
@rust-timer

Copy link
Copy Markdown
Collaborator

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (08587d4): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
7.1% [7.1%, 7.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-7.2% [-11.6%, -2.8%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.5% [-11.6%, 7.1%] 3

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: missing data
Artifact size: 403.29 MiB -> 403.36 MiB (0.02%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants