Skip to content

overflow -> ambig: trigger recursion_depth_exceeding_limit - #160632

Closed
lcnr wants to merge 3 commits into
rust-lang:mainfrom
lcnr:overflow-to-ambig
Closed

overflow -> ambig: trigger recursion_depth_exceeding_limit#160632
lcnr wants to merge 3 commits into
rust-lang:mainfrom
lcnr:overflow-to-ambig

Conversation

@lcnr

@lcnr lcnr commented Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

cc https://rust-lang.zulipchat.com/#narrow/channel/364551-t-types.2Ftrait-system-refactor/topic/overflow.20hard.20error.20.60calimero-store.60/with/614961692

the fact that this lint triggers multiple times for the same goal is pretty bad. We should somehow change this to only be emitted once per obligation or maybe even less. Looking at crates which trigger this lint, they sometimes emit it thousands of times. That seems pretty bad from a UX perspective.

cc #159224 #159228

r? @adwinwhite

@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 Aug 6, 2026
@lcnr

lcnr commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 6, 2026
@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Aug 6, 2026
@bors
Auto merge of #160632 - lcnr:overflow-to-ambig, r=<try>
f81a509 
overflow -> ambig: trigger `recursion_depth_exceeding_limit`
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lcnr
lcnr force-pushed the overflow-to-ambig branch from 9dcb449 to 940cc6c Compare August 6, 2026 11:51
@rust-bors

rust-bors Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: f81a509 (f81a509f84288e35b6be72a05dee373179700839)
Base parent: 887804d (887804d5686c3bf9a090b5bbc23e3dcdfd9e316a)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer

rust-timer commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (f81a509): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@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)		 -0.6% [-0.6%, -0.6%] 3
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -2.1%)

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

mean range count
Regressions ❌
(primary)		 2.2% [2.2%, 2.2%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -6.4% [-6.4%, -6.4%] 1
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -2.1% [-6.4%, 2.2%] 2

Cycles

Results (primary 2.2%, secondary 3.1%)

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

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

Binary size

Results (primary -0.0%, secondary -0.0%)

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)		 - - 0
Improvements ✅
(primary)		 -0.0% [-0.1%, -0.0%] 19
Improvements ✅
(secondary)		 -0.0% [-0.1%, -0.0%] 5
All ❌✅ (primary) -0.0% [-0.1%, -0.0%] 19

Bootstrap: 458.35s -> 466.079s (1.69%)
Artifact size: 398.64 MiB -> 398.58 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 6, 2026
@adwinwhite

adwinwhite commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Similar to #160254, the current perf collection won't be affected by this as they don't overflow(maybe except typenum)?

@adwinwhite

adwinwhite commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Should this be blocked on UX improvement? Or just land this fix first?

@lcnr

lcnr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

I'd like to deduplicate the errors at least somewhat 🤔 unsure how to do so rn though 😁

@lcnr
lcnr force-pushed the overflow-to-ambig branch from efe2806 to 07b0490 Compare August 13, 2026 10:50
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot

rustbot commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to the core trait solver

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

adwinwhite
adwinwhite approved these changes Aug 14, 2026
View reviewed changes

@adwinwhite adwinwhite left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

r=me after rebasing

View changes since this review

Comment thread compiler/rustc_errors/src/lib.rs
emitted_diagnostics: FxHashSet<Hash128>,

/// We only want to emit `recursion_depth_exceeding_limit` once per
/// crate. Otherwise crates like `calimero-store` emit more than

@adwinwhite adwinwhite Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of referencing calimero-store which is "fixed" by the new version of generic-array, we quickly explain why we would have tons of warnings?

@lcnr

lcnr commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

closing in favor of #161341 ::>

@lcnr lcnr closed this Aug 21, 2026
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 21, 2026
@lcnr lcnr mentioned this pull request Aug 22, 2026
Merged
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 22, 2026
@JonathanBrouwer
Rollup merge of rust-lang#161341 - adwinwhite:overflow-fcw-chain, r=l…
ac5567e 
…cnr,adwinwhite

be more permissive wrt overflow and and improve diagnostics

This builds on rust-lang#160632

Previously we only showed a single root goal for the FCW. It was difficult to find out how the goal overflowed.
We display a proving chain now which should help users identify relevant types or auto traits.

This will affect perf for crates emitting the FCW. E.g. `calimero-store` goes from 4.7s -> 5.7s in local testing since it emits thousands of FCWs internally. The FCW is a mitigation of future hard error and authors are expected to resolve it so it's probably acceptable. It doesn't affect crates without the FCW.

r? lcnr
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 22, 2026
@jhpratt
Rollup merge of rust-lang#161341 - adwinwhite:overflow-fcw-chain, r=l…
be3ada1 
…cnr,adwinwhite

be more permissive wrt overflow and and improve diagnostics

This builds on rust-lang#160632

Previously we only showed a single root goal for the FCW. It was difficult to find out how the goal overflowed.
We display a proving chain now which should help users identify relevant types or auto traits.

This will affect perf for crates emitting the FCW. E.g. `calimero-store` goes from 4.7s -> 5.7s in local testing since it emits thousands of FCWs internally. The FCW is a mitigation of future hard error and authors are expected to resolve it so it's probably acceptable. It doesn't affect crates without the FCW.

r? lcnr
rust-bors Bot pushed a commit that referenced this pull request Aug 23, 2026
@bors
Unrolled build for #161341 in rollup 161566
448505c 
Rollup merge of #161341 - adwinwhite:overflow-fcw-chain, r=lcnr,adwinwhite

be more permissive wrt overflow and and improve diagnostics

This builds on #160632

Previously we only showed a single root goal for the FCW. It was difficult to find out how the goal overflowed.
We display a proving chain now which should help users identify relevant types or auto traits.

This will affect perf for crates emitting the FCW. E.g. `calimero-store` goes from 4.7s -> 5.7s in local testing since it emits thousands of FCWs internally. The FCW is a mitigation of future hard error and authors are expected to resolve it so it's probably acceptable. It doesn't affect crates without the FCW.

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

Reviewers

@adwinwhite adwinwhite adwinwhite approved these changes

Assignees

@adwinwhite adwinwhite

Labels

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lcnr @rust-timer @rust-log-analyzer @adwinwhite @rustbot