`compute_trait_goal` structural equality fast path #144258

lcnr · 2025-07-21T14:08:05Z

Fixes the hang in rayon and rust-lang/trait-system-refactor-initiative#210.

This changes means we now have a far stronger reliance on structural identity making rust-lang/trait-system-refactor-initiative#127 a significantly larger problem. However, I've come to believe that fast paths based on structural equality are just too desirable. Solves that issue by uniquifying root goals only during HIR typeck. Fixes github.com/rust-lang/trait-system-refactor-initiative/issues/27

Fixes #139409.

r? @compiler-errors

rustbot · 2025-07-21T14:08:30Z

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

lcnr · 2025-07-21T14:09:20Z

@ors2 try @rust-timer queue

lcnr · 2025-07-21T14:09:59Z

compiler/rustc_next_trait_solver/src/solve/trait_goals.rs

+            .any(|p| p == goal.predicate)
+        {
+            let candidate = self
+                .probe_trait_candidate(CandidateSource::ParamEnv(ParamEnvSource::NonGlobal))


TODO: this incorrect marks param_env candidates as global :<

lcnr · 2025-07-21T14:10:23Z

tests/ui/traits/next-solver/normalize/normalize-param-env-4.rs

-//@[next] compile-flags: -Znext-solver
-//@[next] known-bug: #92505
-//@[current] check-pass
+//@ check-pass


TODO: I don't fully get why this one compiles now 🤔

we've got the proof tree

T: Trait

via env: AliasRelate(T::Assoc, T) no solution

NormalizesTo(T::Assoc)

compute proven_via for T: Trait

via AliasRelate(T::Assoc, T) unproductive cycle

via impl -> T

we currently never try to actually normalize T::Assoc outside of applying the param env candidate. Doing this has the following proof tree

NormalizesTo(T::Assoc)

compute proven_via for T: Trait

via AliasRelate(T::Assoc, T) err

NormalizesTo(T::Assoc) unproductive cycle no solution

via impl -> T

cycle rerun (provisional result: normalizes-to T)

compute proven_via for T: Trait

via AliasRelate(T::Assoc, T) ok

NormalizesTo(T::Assoc) cycle provisional result T

alias is rigid

cycle rerun (provisional result: normalizes-to T::Assoc)

compute proven_via for T: Trait

via AliasRelate(T::Assoc, T) err

NormalizesTo(T::Assoc) cycle provisional result T::Assoc

via impl -> T

...fail to reach fixpoint

This should start failing again with #140558 and changes it's behavior as this PR simply causes checking whether the <T as Trait>::Assoc: Trait to use the fast path instead of normalizing the self type

lcnr · 2025-07-21T14:12:50Z

@bors2 try

`compute_trait_goal` structural equality fast path Fixes the hang in rayon and rust-lang/trait-system-refactor-initiative#210. This changes means we now have a far stronger reliance on structural equality making rust-lang/trait-system-refactor-initiative#127 a significantly larger problem. However, I've come to believe that fast paths based on structural equality are just too desirable. Fixes #139409. r? `@compiler-errors`

rust-bors · 2025-07-21T14:12:54Z

⌛ Trying commit 8c737ce with merge 2f05b1f…

To cancel the try build, run the command @bors2 try cancel.

rust-bors · 2025-07-21T16:28:20Z

☀️ Try build successful (CI)
Build commit: 2f05b1f (2f05b1f742a5c897ad5434184c44474f4107be5e, parent: 67819923ac8ea353aaa775303f4c3aacbf41d010)

rust-timer · 2025-07-21T18:07:59Z

Finished benchmarking commit (2f05b1f): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-5.5%, -0.3%]	8
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -3.9%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[2.3%, 2.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.9%	[-11.4%, -3.1%]	3
All ❌✅ (primary)	-	-	0

Cycles

Results (primary -2.2%, secondary -3.3%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.7%	[1.7%, 1.7%]	1
Improvements ✅ (primary)	-2.2%	[-2.2%, -2.2%]	1
Improvements ✅ (secondary)	-4.3%	[-5.7%, -2.4%]	5
All ❌✅ (primary)	-2.2%	[-2.2%, -2.2%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 465.321s -> 462.652s (-0.57%)
Artifact size: 374.50 MiB -> 374.44 MiB (-0.02%)

uniquify root goals during HIR typeck We need to rely on region identity to deal with hangs such as rust-lang/trait-system-refactor-initiative#210 and to keep the current behavior of `fn try_merge_responses`. This is a problem as borrowck starts by replacing each *occurrence* of a region with a unique inference variable. This frequently splits a single region during HIR typeck into multiple distinct regions. As we assume goals to always succeed during borrowck, relying on two occurances of a region being identical during HIR typeck causes ICE. See the now fixed examples in rust-lang/trait-system-refactor-initiative#27 and #139409. We've previously tried to avoid this issue by always *uniquifying* regions when canonicalizing goals. This prevents caching subtrees during canonicalization which resulted in hangs for very large types. People rely on such types in practice, which caused us to revert our attempt to reinstate `#[type_length_limit]` in #127670. The complete list of changes here: - #107981 - #110180 - #114117 - #130821 After more consideration, all occurrences of such large types need to happen outside of typeck/borrowck. We know this as we already walk over all types in the MIR body when replacing their regions with nll vars. This PR therefore enables us to rely on region identity inside of the trait solver by exclusively **uniquifying root goals during HIR typeck**. These are the only goals we assume to hold during borrowck. This is insufficient as type inference variables may "hide" regions we later uniquify. Because of this, we now stash proven goals which depend on inference variables in HIR typeck and reprove them after writeback. This closes rust-lang/trait-system-refactor-initiative#127. This was originally part of #144258 but I've moved it into a separate PR. While I believe we need to rely on region identity to fix the performance issues in some way, I don't know whether #144258 is the best approach to actually do so. Regardless of how we deal with the hangs however, this change is necessary and desirable regardless. r? `@compiler-errors` or `@BoxyUwU`

uniquify root goals during HIR typeck We need to rely on region identity to deal with hangs such as rust-lang/trait-system-refactor-initiative#210 and to keep the current behavior of `fn try_merge_responses`. This is a problem as borrowck starts by replacing each *occurrence* of a region with a unique inference variable. This frequently splits a single region during HIR typeck into multiple distinct regions. As we assume goals to always succeed during borrowck, relying on two occurances of a region being identical during HIR typeck causes ICE. See the now fixed examples in rust-lang/trait-system-refactor-initiative#27 and rust-lang/rust#139409. We've previously tried to avoid this issue by always *uniquifying* regions when canonicalizing goals. This prevents caching subtrees during canonicalization which resulted in hangs for very large types. People rely on such types in practice, which caused us to revert our attempt to reinstate `#[type_length_limit]` in rust-lang/rust#127670. The complete list of changes here: - rust-lang/rust#107981 - rust-lang/rust#110180 - rust-lang/rust#114117 - rust-lang/rust#130821 After more consideration, all occurrences of such large types need to happen outside of typeck/borrowck. We know this as we already walk over all types in the MIR body when replacing their regions with nll vars. This PR therefore enables us to rely on region identity inside of the trait solver by exclusively **uniquifying root goals during HIR typeck**. These are the only goals we assume to hold during borrowck. This is insufficient as type inference variables may "hide" regions we later uniquify. Because of this, we now stash proven goals which depend on inference variables in HIR typeck and reprove them after writeback. This closes rust-lang/trait-system-refactor-initiative#127. This was originally part of rust-lang/rust#144258 but I've moved it into a separate PR. While I believe we need to rely on region identity to fix the performance issues in some way, I don't know whether rust-lang/rust#144258 is the best approach to actually do so. Regardless of how we deal with the hangs however, this change is necessary and desirable regardless. r? `@compiler-errors` or `@BoxyUwU`

bors · 2025-07-31T05:17:59Z

☔ The latest upstream changes (presumably #144405) made this pull request unmergeable. Please resolve the merge conflicts.

uniquify root goals during HIR typeck We need to rely on region identity to deal with hangs such as rust-lang/trait-system-refactor-initiative#210 and to keep the current behavior of `fn try_merge_responses`. This is a problem as borrowck starts by replacing each *occurrence* of a region with a unique inference variable. This frequently splits a single region during HIR typeck into multiple distinct regions. As we assume goals to always succeed during borrowck, relying on two occurances of a region being identical during HIR typeck causes ICE. See the now fixed examples in rust-lang/trait-system-refactor-initiative#27 and rust-lang/rust#139409. We've previously tried to avoid this issue by always *uniquifying* regions when canonicalizing goals. This prevents caching subtrees during canonicalization which resulted in hangs for very large types. People rely on such types in practice, which caused us to revert our attempt to reinstate `#[type_length_limit]` in rust-lang/rust#127670. The complete list of changes here: - rust-lang/rust#107981 - rust-lang/rust#110180 - rust-lang/rust#114117 - rust-lang/rust#130821 After more consideration, all occurrences of such large types need to happen outside of typeck/borrowck. We know this as we already walk over all types in the MIR body when replacing their regions with nll vars. This PR therefore enables us to rely on region identity inside of the trait solver by exclusively **uniquifying root goals during HIR typeck**. These are the only goals we assume to hold during borrowck. This is insufficient as type inference variables may "hide" regions we later uniquify. Because of this, we now stash proven goals which depend on inference variables in HIR typeck and reprove them after writeback. This closes rust-lang/trait-system-refactor-initiative#127. This was originally part of rust-lang/rust#144258 but I've moved it into a separate PR. While I believe we need to rely on region identity to fix the performance issues in some way, I don't know whether rust-lang/rust#144258 is the best approach to actually do so. Regardless of how we deal with the hangs however, this change is necessary and desirable regardless. r? `@compiler-errors` or `@BoxyUwU`

yikes

e8f0b17

rustbot assigned compiler-errors Jul 21, 2025

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 Jul 21, 2025

rarw

8c737ce

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 21, 2025

lcnr commented Jul 21, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 21, 2025

lcnr mentioned this pull request Jul 24, 2025

uniquify root goals during HIR typeck #144405

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`compute_trait_goal` structural equality fast path #144258

`compute_trait_goal` structural equality fast path #144258

lcnr commented Jul 21, 2025 •

edited

Loading

Uh oh!

rustbot commented Jul 21, 2025

Uh oh!

lcnr commented Jul 21, 2025

Uh oh!

This comment has been minimized.

lcnr Jul 21, 2025

Uh oh!

lcnr Jul 21, 2025

Uh oh!

lcnr Jul 22, 2025

Uh oh!

lcnr commented Jul 21, 2025

Uh oh!

rust-bors bot commented Jul 21, 2025

Uh oh!

rust-bors bot commented Jul 21, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 21, 2025

Uh oh!

bors commented Jul 31, 2025

Uh oh!

Uh oh!

compute_trait_goal structural equality fast path #144258

Are you sure you want to change the base?

compute_trait_goal structural equality fast path #144258

Conversation

lcnr commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 21, 2025

Uh oh!

lcnr commented Jul 21, 2025

Uh oh!

This comment has been minimized.

lcnr Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

lcnr Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

lcnr Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

lcnr commented Jul 21, 2025

Uh oh!

rust-bors bot commented Jul 21, 2025

Uh oh!

rust-bors bot commented Jul 21, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 21, 2025

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

bors commented Jul 31, 2025

Uh oh!

Uh oh!

`compute_trait_goal` structural equality fast path #144258

`compute_trait_goal` structural equality fast path #144258

lcnr commented Jul 21, 2025 •

edited

Loading