trait solver: account for universes from replace_bound_vars by Dnreikronos · Pull Request #158362 · rust-lang/rust

Dnreikronos · 2026-06-24T15:19:03Z

The ICE was not really because eager placeholder handling looked at too many universes. Boxy was right: with -Zassumptions-on-binders, if the new solver creates a placeholder universe, that universe needs an assumptions entry.

The bad path is FindParamInClause entering a binder through replace_bound_vars. BoundVarReplacer materializes a placeholder universe for the escaping bound vars, but nothing records placeholder assumptions for it. Later eager placeholder handling walks all non-input universes, which imo is the right behavior, and get_placeholder_assumptions hits the missing entry.

This moves the empty-assumptions bookkeeping into EvalCtxt::replace_bound_vars instead of keeping it local to FindParamInClause. The helper snapshots the universe slots before replacement and inserts Assumptions::empty() for any slot that got filled.

There is still a FIXME there because idk that empty assumptions is the final shape irl. replace_bound_vars does not have the param-env context to compute proper assumptions. But for this PR, imo this is the least weird local fix: it keeps the eager pass looking at all non-input universes and fixes the missing bookkeeping where the universe is created.

The repro is covered by a UI test. It reports the overflow diagnostic instead of ICEing.

rustbot · 2026-06-24T15:19:08Z

r? @JonathanBrouwer

rustbot has assigned @JonathanBrouwer.
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 20 candidates

JonathanBrouwer · 2026-06-24T15:27:15Z

r? types

jackh726 · 2026-06-25T03:18:34Z

I'm not entirely sure if this is the right fix (I would actually expect that self.delegate.universe() to return the proper universe), but this is also quite new code. So going to reassign to Boxy.

r? @BoxyUwU

Dnreikronos · 2026-06-25T13:27:40Z

yeah, i think you're right that this probably is not the ideal long-term shape.

what this pr does is more of a local bound: eager placeholder handling only walks universes the current solver region constraint can actually name. imo that part still makes sense to me. self.delegate.universe() is just the highest universe the infcx has created, and solver work can bump that even if the final constraint no longer mentions that universe. then we ask get_placeholder_assumptions for a universe that is not relevant to the constraint and may not have an entry, which is the unwrap in the repro.

so yeah, idk if i would call this the best fix in the abstract. long term i'd expect us to have a cleaner invariant between created universes and placeholder assumptions, or maybe an explicit set of universes that eager handling should process.

but for this pr, i still prefer this local fix over making missing assumptions silently mean ambiguity. if the constraint cannot name the universe, i don't think this pass should care about it.

so this still lgtm to me as a short term fix, with the caveat that self.delegate.universe() being too global here is probably worth documenting.

JorgeCepeda · 2026-06-26T00:22:01Z

Offtopic: what does ltm mean?

Dnreikronos · 2026-06-26T00:25:41Z

Offtopic: what does ltm mean?

long term.
i just edited to be easier to understand, my bad....

BoxyUwU · 2026-06-26T11:23:10Z

this in theory makes sense as an optimization but it shouldn't make anything stop ICEing. We should not be entering binders inside of the trait solver without accounting for their assumptions 🤔

long term i'd expect us to have a cleaner invariant between created universes and placeholder assumptions, or maybe an explicit set of universes that eager handling should process.

the set of universes it should handle is all of them (other than input universes). can you look into where/why we're entering a binder and not inserting assumptions for it

Dnreikronos · 2026-06-27T01:29:39Z

this in theory makes sense as an optimization but it shouldn't make anything stop ICEing. We should not be entering binders inside of the trait solver without accounting for their assumptions 🤔

long term i'd expect us to have a cleaner invariant between created universes and placeholder assumptions, or maybe an explicit set of universes that eager handling should process.

the set of universes it should handle is all of them (other than input universes). can you look into where/why we're entering a binder and not inserting assumptions for it

yeah, you were right. i looked into where the unaccounted binder entry was happening, and it seems like the issue was in findparaminclause.

the bad path was basically: findparaminclause walks into a binder, then replace_bound_vars materializes a placeholder universe through boundvarreplacer, but that universe never got an entry in the placeholder assumptions map. later eager placeholder handling still walks all non-input universes, as it should, and then get_placeholder_assumptions hits the missing entry and ices.

i changed the patch so eager handling keeps using all non-input universes. instead, when findparaminclause calls replace_bound_vars, it tracks any previously-empty universe slot that got filled and registers empty placeholder assumptions for that universe. imo this is a better shape than bounding eager handling by the current constraint, since it fixes the missing bookkeeping directly and does not change the global/non-global classification by bailing on binders.

i also added the issue repro as a ui test. with this version it no longer ices and reports the overflow diagnostic instead.

pushed this version for you to validate. idk if long-term we want boundvarreplacer to report newly-created universes more explicitly, but for this pr i think this keeps the fix small and matches the existing empty-assumptions pattern. lgtm to me, but i'd like your take on whether empty assumptions is the right local accounting here.

BoxyUwU

thx for looking into that, yeah this makes total sense to me ✨ I think this is the right fix though would like to do something a little more general (see comment). can you update your title/description now that the approach has been changed

View changes since this review

BoxyUwU · 2026-07-14T19:44:04Z

+{
+    fn replace_bound_vars<T: TypeFoldable<I>>(&mut self, value: T) -> T {
+        let old_universes = self.universes.clone();
+        let value = self.ecx.replace_bound_vars(value, &mut self.universes);


I think slightly more generally than FindParamInClause, any part of the new solver which calls replace_bound_vars is just wrong under -Zassumptions-on-binders 🤔

I think ideally we would update replace_bound_vars to, when it creates new universes, also insert empty assumptions (and add an associated fixme to fix that in the long term :3)

yeah, makes sense. i changed this to be less FindParamInClause-specific.

replace_bound_vars now snapshots the universe slots before calling BoundVarReplacer, then inserts Assumptions::empty() for any slot that got materialized. FindParamInClause is back to just calling ecx.replace_bound_vars, so btw this should cover the other new-solver callers of that helper too.

imo this is a better shape than the previous patch. idk if empty assumptions is what we want irl, since replace_bound_vars still does not know enough to compute the real assumptions, so i left a FIXME there as you requested. ltm if you'd rather push this closer to BoundVarReplacer, but i think this is the right small step for this PR.

fyi i also updated the title/description so they no longer talk about limiting eager placeholder handling.

rustbot · 2026-07-16T00:46:38Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Dnreikronos · 2026-07-22T16:09:19Z

@rustbot ready

BoxyUwU · 2026-07-23T10:07:07Z

@bors r+

rust-bors · 2026-07-23T10:07:10Z

📌 Commit 6860c6b has been approved by BoxyUwU

It is now in the queue for this repository.

…er_universe_bounds, r=BoxyUwU trait solver: account for universes from replace_bound_vars Fixes rust-lang#157840. The ICE was not really because eager placeholder handling looked at too many universes. Boxy was right: with `-Zassumptions-on-binders`, if the new solver creates a placeholder universe, that universe needs an assumptions entry. The bad path is `FindParamInClause` entering a binder through `replace_bound_vars`. `BoundVarReplacer` materializes a placeholder universe for the escaping bound vars, but nothing records placeholder assumptions for it. Later eager placeholder handling walks all non-input universes, which imo is the right behavior, and `get_placeholder_assumptions` hits the missing entry. This moves the empty-assumptions bookkeeping into `EvalCtxt::replace_bound_vars` instead of keeping it local to `FindParamInClause`. The helper snapshots the universe slots before replacement and inserts `Assumptions::empty()` for any slot that got filled. There is still a FIXME there because idk that empty assumptions is the final shape irl. `replace_bound_vars` does not have the param-env context to compute proper assumptions. But for this PR, imo this is the least weird local fix: it keeps the eager pass looking at all non-input universes and fixes the missing bookkeeping where the universe is created. The repro is covered by a UI test. It reports the overflow diagnostic instead of ICEing.

Rollup of 18 pull requests Successful merges: - #138618 (Support using const pointers in asm `const` operand) - #150161 (Remove 'static requirement on try_as_dyn) - #158362 (trait solver: account for universes from replace_bound_vars) - #159173 (Add allowed list check on EII implementations attributes) - #159466 (cmse: clear variant-dependent padding in `enum`s) - #159718 (Make `DocLinkResMap` an `FxIndexMap`) - #155795 (constify `vec![1, 2, 3]` macro) - #157776 (ci: Enable autodiff tests on x86_64 linux) - #157905 (Update comments and add tests for `-Zrandomize-layout` for some guaranteed ZSTs) - #159041 (Reorganize `tests/ui/issues` [22/N]) - #159108 (Reorganize `tests/ui/issues` [23/N]) - #159138 (doc: document wasm import symbol mangling) - #159531 (Reorganize `tests/ui/issues` [28/N]) - #159608 (early_otherwise: Don't hoist dereferences when the otherwise branch is reachable) - #159612 (Reorganize `tests/ui/issues` [29/N]) - #159653 (run `tests/assembly-llvm/asm/aarch64-types.rs` for `aarch64_be`) - #159759 (rustc-dev-guide subtree update) - #159761 (Remove outdated comment for resolve_vars_with_obligations)

…er_universe_bounds, r=BoxyUwU trait solver: account for universes from replace_bound_vars Fixes rust-lang#157840. The ICE was not really because eager placeholder handling looked at too many universes. Boxy was right: with `-Zassumptions-on-binders`, if the new solver creates a placeholder universe, that universe needs an assumptions entry. The bad path is `FindParamInClause` entering a binder through `replace_bound_vars`. `BoundVarReplacer` materializes a placeholder universe for the escaping bound vars, but nothing records placeholder assumptions for it. Later eager placeholder handling walks all non-input universes, which imo is the right behavior, and `get_placeholder_assumptions` hits the missing entry. This moves the empty-assumptions bookkeeping into `EvalCtxt::replace_bound_vars` instead of keeping it local to `FindParamInClause`. The helper snapshots the universe slots before replacement and inserts `Assumptions::empty()` for any slot that got filled. There is still a FIXME there because idk that empty assumptions is the final shape irl. `replace_bound_vars` does not have the param-env context to compute proper assumptions. But for this PR, imo this is the least weird local fix: it keeps the eager pass looking at all non-input universes and fixes the missing bookkeeping where the universe is created. The repro is covered by a UI test. It reports the overflow diagnostic instead of ICEing.

Rollup of 19 pull requests Successful merges: - #150161 (Remove 'static requirement on try_as_dyn) - #158362 (trait solver: account for universes from replace_bound_vars) - #159173 (Add allowed list check on EII implementations attributes) - #159466 (cmse: clear variant-dependent padding in `enum`s) - #159718 (Make `DocLinkResMap` an `FxIndexMap`) - #155795 (constify `vec![1, 2, 3]` macro) - #157776 (ci: Enable autodiff tests on x86_64 linux) - #157905 (Update comments and add tests for `-Zrandomize-layout` for some guaranteed ZSTs) - #158766 (Promote riscv64-unknown-linux-musl to tier 2 with host tools) - #159041 (Reorganize `tests/ui/issues` [22/N]) - #159108 (Reorganize `tests/ui/issues` [23/N]) - #159138 (doc: document wasm import symbol mangling) - #159531 (Reorganize `tests/ui/issues` [28/N]) - #159608 (early_otherwise: Don't hoist dereferences when the otherwise branch is reachable) - #159612 (Reorganize `tests/ui/issues` [29/N]) - #159653 (run `tests/assembly-llvm/asm/aarch64-types.rs` for `aarch64_be`) - #159667 (Make some parser structured suggestions verbose and tweak their wording) - #159759 (rustc-dev-guide subtree update) - #159761 (Remove outdated comment for resolve_vars_with_obligations)

…er_universe_bounds, r=BoxyUwU trait solver: account for universes from replace_bound_vars Fixes rust-lang#157840. The ICE was not really because eager placeholder handling looked at too many universes. Boxy was right: with `-Zassumptions-on-binders`, if the new solver creates a placeholder universe, that universe needs an assumptions entry. The bad path is `FindParamInClause` entering a binder through `replace_bound_vars`. `BoundVarReplacer` materializes a placeholder universe for the escaping bound vars, but nothing records placeholder assumptions for it. Later eager placeholder handling walks all non-input universes, which imo is the right behavior, and `get_placeholder_assumptions` hits the missing entry. This moves the empty-assumptions bookkeeping into `EvalCtxt::replace_bound_vars` instead of keeping it local to `FindParamInClause`. The helper snapshots the universe slots before replacement and inserts `Assumptions::empty()` for any slot that got filled. There is still a FIXME there because idk that empty assumptions is the final shape irl. `replace_bound_vars` does not have the param-env context to compute proper assumptions. But for this PR, imo this is the least weird local fix: it keeps the eager pass looking at all non-input universes and fixes the missing bookkeeping where the universe is created. The repro is covered by a UI test. It reports the overflow diagnostic instead of ICEing.

Rollup of 12 pull requests Successful merges: - #159765 (Avoid spurious rebuilds of JSON docs in bootstrap) - #159781 (Update bootstrap to use -Zembed-metadata=no instead of -Zno-embed-metadata) - #158362 (trait solver: account for universes from replace_bound_vars) - #159173 (Add allowed list check on EII implementations attributes) - #159718 (Make `DocLinkResMap` an `FxIndexMap`) - #159722 ( [rustdoc] Retrieve `cfg_attr` information for derived impls for `doc_cfg` feature) - #155795 (constify `vec![1, 2, 3]` macro) - #157776 (ci: Enable autodiff tests on x86_64 linux) - #158766 (Promote riscv64-unknown-linux-musl to tier 2 with host tools) - #159271 (str: add ASCII fast path to word_to_titlecase) - #159666 (fix(ld64.lld): route version mismatch warnings to linker_info on macOS) - #159667 (Make some parser structured suggestions verbose and tweak their wording)

Rollup of 14 pull requests Successful merges: - #159765 (Avoid spurious rebuilds of JSON docs in bootstrap) - #159781 (Update bootstrap to use -Zembed-metadata=no instead of -Zno-embed-metadata) - #158362 (trait solver: account for universes from replace_bound_vars) - #158372 (rustfmt: Discover modules via `cfg_select!`) - #159173 (Add allowed list check on EII implementations attributes) - #159718 (Make `DocLinkResMap` an `FxIndexMap`) - #159722 ( [rustdoc] Retrieve `cfg_attr` information for derived impls for `doc_cfg` feature) - #159731 (std: Implement futex on wasip3 targets, update target spec) - #159755 (Improve consistency of attribute error messages) - #155795 (constify `vec![1, 2, 3]` macro) - #157776 (ci: Enable autodiff tests on x86_64 linux) - #158766 (Promote riscv64-unknown-linux-musl to tier 2 with host tools) - #159271 (str: add ASCII fast path to word_to_titlecase) - #159667 (Make some parser structured suggestions verbose and tweak their wording)

Rollup merge of #158362 - Dnreikronos:trait_solver/placeholder_universe_bounds, r=BoxyUwU trait solver: account for universes from replace_bound_vars Fixes #157840. The ICE was not really because eager placeholder handling looked at too many universes. Boxy was right: with `-Zassumptions-on-binders`, if the new solver creates a placeholder universe, that universe needs an assumptions entry. The bad path is `FindParamInClause` entering a binder through `replace_bound_vars`. `BoundVarReplacer` materializes a placeholder universe for the escaping bound vars, but nothing records placeholder assumptions for it. Later eager placeholder handling walks all non-input universes, which imo is the right behavior, and `get_placeholder_assumptions` hits the missing entry. This moves the empty-assumptions bookkeeping into `EvalCtxt::replace_bound_vars` instead of keeping it local to `FindParamInClause`. The helper snapshots the universe slots before replacement and inserts `Assumptions::empty()` for any slot that got filled. There is still a FIXME there because idk that empty assumptions is the final shape irl. `replace_bound_vars` does not have the param-env context to compute proper assumptions. But for this PR, imo this is the least weird local fix: it keeps the eager pass looking at all non-input universes and fixes the missing bookkeeping where the universe is created. The repro is covered by a UI test. It reports the overflow diagnostic instead of ICEing.

rustbot assigned JonathanBrouwer Jun 24, 2026

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 Jun 24, 2026

This comment has been minimized.

Sign in to view

rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Jun 24, 2026

rustbot assigned jackh726 and unassigned JonathanBrouwer Jun 24, 2026

Dnreikronos force-pushed the trait_solver/placeholder_universe_bounds branch from 9cc11df to 697770e Compare June 24, 2026 15:28

Dnreikronos mentioned this pull request Jun 24, 2026

[ICE]: assumptions on binders: called Option::unwrap() on a None value context.rs #157840

Closed

rustbot assigned BoxyUwU and unassigned jackh726 Jun 25, 2026

BoxyUwU mentioned this pull request Jul 13, 2026

Assumptions on Binders rust-lang/rust-project-goals#621

Open

BoxyUwU reviewed Jul 14, 2026

View reviewed changes

Dnreikronos changed the title ~~trait solver: limit eager placeholder handling~~ trait solver: record assumptions for replaced bound vars Jul 15, 2026

Dnreikronos changed the title ~~trait solver: record assumptions for replaced bound vars~~ trait solver: account for universes from replace_bound_vars Jul 15, 2026

Dnreikronos requested a review from BoxyUwU July 15, 2026 19:57

This comment has been minimized.

Sign in to view

Dnreikronos force-pushed the trait_solver/placeholder_universe_bounds branch from ad0815f to 25bd5cc Compare July 16, 2026 00:46

This comment has been minimized.

Sign in to view

Drop redundant canonical universe assumptions

6860c6b

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 22, 2026

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2026

jhpratt mentioned this pull request Jul 23, 2026

Rollup of 18 pull requests #159762

Closed

jhpratt mentioned this pull request Jul 23, 2026

Rollup of 19 pull requests #159766

Closed

jhpratt mentioned this pull request Jul 23, 2026

Rollup of 19 pull requests #159769

Closed

jhpratt mentioned this pull request Jul 23, 2026

Rollup of 23 pull requests #159793

Closed

jhpratt mentioned this pull request Jul 23, 2026

Rollup of 11 pull requests #159803

Closed

jhpratt mentioned this pull request Jul 24, 2026

Rollup of 12 pull requests #159806

Closed

jhpratt mentioned this pull request Jul 24, 2026

Rollup of 12 pull requests #159807

Closed

jhpratt mentioned this pull request Jul 24, 2026

Rollup of 14 pull requests #159814

Merged

rust-bors Bot merged commit 9da519c into rust-lang:main Jul 24, 2026
13 checks passed

rustbot added this to the 1.99.0 milestone Jul 24, 2026

Uh oh!

Uh oh!

Conversation

Dnreikronos commented Jun 24, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 24, 2026

Uh oh!

This comment has been minimized.

JonathanBrouwer commented Jun 24, 2026

Uh oh!

jackh726 commented Jun 25, 2026

Uh oh!

Dnreikronos commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JorgeCepeda commented Jun 26, 2026

Uh oh!

Dnreikronos commented Jun 26, 2026

Uh oh!

BoxyUwU commented Jun 26, 2026

Uh oh!

Dnreikronos commented Jun 27, 2026

Uh oh!

BoxyUwU left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BoxyUwU Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Dnreikronos Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

rustbot commented Jul 16, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Dnreikronos commented Jul 22, 2026

Uh oh!

BoxyUwU commented Jul 23, 2026

Uh oh!

rust-bors Bot commented Jul 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Dnreikronos commented Jun 24, 2026 •

edited by rustbot

Loading

Dnreikronos commented Jun 25, 2026 •

edited

Loading

BoxyUwU left a comment •

edited by rustbot

Loading

Dnreikronos Jul 15, 2026 •

edited

Loading