fix: Infinite loop while elaborting predicates #20654

ShoyuVanilla · 2025-09-11T17:15:34Z

Fixes #20582

Disclaimer: this code can effectively hang or stack-overflow your rust-analyzer!

trait DimMax<Other: Dimension> {
    type Output: Dimension;
}
trait Dimension: DimMax<<Self as Dimension>:: Smaller, Output = Self> {
    type Smaller: Dimension;
}

If we gonna normalize projection types related to the above traits, the current implementation endlessly elaborates implied bounds like

<T as DimMax>::Output: Dimension and we have Dimension: DimMax
<<T as DimMax>::Output as DimMax>::Output: DimMax
<<<T as DimMax>::Output as DimMax>::Output as DimMax>::Output: DimMax
... 😵

Though this is not the best I can, I partially ported some implementation details from rustc to fix this infinite loop.

I think we should move overall lowering logic closer to that of rustc's, especially for those ones in here:
https://github.com/rust-lang/rust/blob/76c5ed2847cdb26ef2822a3a165d710f6b772217/compiler/rustc_hir_analysis/src/collect/predicates_of.rs

but maybe after migrating remaining things to next-solver.

ShoyuVanilla · 2025-09-11T17:16:50Z

crates/hir-ty/src/tests/incremental.rs

    );
 }

-// FIXME(next-solver): does this test make sense with fast path?


Removing this in accordance with #20422 (comment)

I’d keep this around and add an “expects a panic”. i think queries might be too expensive, but… i’d consider keeping it.

Instead of adding #[should_panic] I just updated the test output, as what was changed is just a single query name 😄

ShoyuVanilla · 2025-09-11T17:18:58Z

crates/hir-ty/src/dyn_compatibility.rs


    let interner = DbInterner::new_with(db, Some(krate), None);
+    // FIXME: We should use `explicit_predicates_of` here, which hasn't been implemented to
+    // rust-analyzer yet


This causes a regression in the following test, though I think it's not a big deal for now, since having <Self> in type bounds is also reported as dyn-incompatible due to SelfRenferential but we should fix this eventually

ShoyuVanilla · 2025-09-11T17:20:52Z

crates/hir-ty/src/tests/regression/new_solver.rs

+    U: Dimension,
+{
+    let t: <T as DimMax<U>>::Output = loop {};
+}


Again, this will blow up your rust-analyzer like in #20504 😨

I feel getting better on minimizing hangs/panics on real world projects 😄
These were actually monstrously large traits like in https://docs.rs/ndarray/0.16.1/ndarray/trait.Dimension.html

davidbarsky

approving, modulo a nit.

davidbarsky · 2025-09-12T16:42:09Z

crates/hir-ty/src/tests/incremental.rs

    );
 }

-// FIXME(next-solver): does this test make sense with fast path?


I’d keep this around and add an “expects a panic”. i think queries might be too expensive, but… i’d consider keeping it.

ShoyuVanilla · 2025-09-14T09:19:46Z

Merging this with David’s approval — without it, the issue might crop up quite often. I don’t think this should interfere with the ongoing coercion rewrite PR, but if it does, I’ll be ready to revert 😄

ChayimFriedman2 · 2025-09-14T09:28:47Z

I actually rediscovered this issue from scratch in my coercion work just recently 😅

However, I stopped the merge due to one question - can't we just filter on generic_predicates_ns() in explicit_super_predicates_of(), no new query? I don't think lowering itself should be problematic.

ShoyuVanilla · 2025-09-14T09:33:35Z

That sounds reasonable as more queries means more memory usage 😅 I think if we gonna move this closer to rustc, we might need separated queries as we need more control over recursive lowering but given the current simple implementation, I'll give it a try

crates/hir-ty/src/next_solver/interner.rs

ChayimFriedman2

Sorry for nudging you!

ShoyuVanilla · 2025-09-14T11:37:31Z

No worries at all

lnicola · 2025-09-14T15:02:37Z

changelog fixup #20329

ChayimFriedman2 · 2025-09-14T19:12:07Z

crates/hir-ty/src/next_solver/interner.rs

            .iter()
+            .filter(|p| match p.kind().skip_binder() {
+                rustc_type_ir::ClauseKind::Trait(tr) => is_self_or_assoc(tr.self_ty()),
+                _ => true,


Wait, that doesn't seem correct? E.g. if we got an AliasRelate we must filter it out if it's not constraining Self, otherwise trouble will happen.

Oh, I just found your comment by now. Yes, seems that I missed it.
To be clear, <T as Trait>::Assoc = Ty should be filtered out conditionally when T is not Self for both explicit_super_predicates_of and explicit_implied_predicates_of, unlike <T as Trait>::Assoc: Ty, which is always filtered out for explicit_super_predicates_of
https://github.com/rust-lang/rust/blob/52618eb338609df44978b0ca4451ab7941fd1c7a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs#L525-L608

I'll open a followup for this once I'm back home from work 😅

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 11, 2025

ShoyuVanilla commented Sep 11, 2025

View reviewed changes

ShoyuVanilla mentioned this pull request Sep 11, 2025

Frequent OOMs in rust-analyzer #20634

Closed

davidbarsky approved these changes Sep 12, 2025

View reviewed changes

ShoyuVanilla force-pushed the predicates branch from 6d28a79 to 45e0f30 Compare September 13, 2025 04:41

ShoyuVanilla added this pull request to the merge queue Sep 14, 2025

ChayimFriedman2 removed this pull request from the merge queue due to a manual request Sep 14, 2025

ShoyuVanilla force-pushed the predicates branch 2 times, most recently from 3dd559b to 7cabe02 Compare September 14, 2025 10:42

ChayimFriedman2 approved these changes Sep 14, 2025

View reviewed changes

crates/hir-ty/src/next_solver/interner.rs Outdated Show resolved Hide resolved

fix: Infinite loop while elaborting predicates

26b8b79

ShoyuVanilla force-pushed the predicates branch from 7cabe02 to 26b8b79 Compare September 14, 2025 11:08

ChayimFriedman2 approved these changes Sep 14, 2025

View reviewed changes

ChayimFriedman2 enabled auto-merge September 14, 2025 11:11

ChayimFriedman2 added this pull request to the merge queue Sep 14, 2025

Merged via the queue into rust-lang:master with commit 760d378 Sep 14, 2025
15 checks passed

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 14, 2025

ShoyuVanilla deleted the predicates branch September 14, 2025 11:37

ChayimFriedman2 reviewed Sep 14, 2025

View reviewed changes

ChayimFriedman2 mentioned this pull request Sep 15, 2025

Rust-analyzer shows an error for code that compiles #20662

Closed

fix: Infinite loop while elaborting predicates #20654

fix: Infinite loop while elaborting predicates #20654

Uh oh!

Conversation

ShoyuVanilla commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShoyuVanilla Sep 11, 2025 • 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!

davidbarsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShoyuVanilla commented Sep 14, 2025

Uh oh!

Uh oh!

ChayimFriedman2 commented Sep 14, 2025

Uh oh!

ShoyuVanilla commented Sep 14, 2025

Uh oh!

Uh oh!

ChayimFriedman2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShoyuVanilla commented Sep 14, 2025

Uh oh!

lnicola commented Sep 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ShoyuVanilla commented Sep 11, 2025 •

edited

Loading

ShoyuVanilla Sep 11, 2025 •

edited

Loading