fix: Port a bunch of stuff from rustc and fix a bunch of type mismatches/diagnostics #20664

ChayimFriedman2 · 2025-09-15T02:32:19Z

This started from porting coercion, but ended with porting much more.

As I shared on Zulip, type mismatches go to zero, and diagnostics go to 1.

This is a giant PR. However, all stuff under hir-ty/src/next_solver is basically copied straight from rustc, so could use a skim review (although something is probably warranted). To my detriment I discovered, after porting plenty of code, that some of the modules were already there - so I had to cut them. I chose to preserve the ones that were more loyal to the origin, even if it means porting more code.

Some themes of the PR:

It introduces ObligationCtxt (parallel to rustc) and starts to use it. This is an opening to a future where, like rustc and correctly (and unlike what we do currently), we don't stash all obligations on the InferenceTable, rather we evaluate them separately and only put them there if needed.
It heavily prefers to use Obligation over Predicate. Existing APIs using Predicate remain, but new code uses Obligation (and existing APIs will also be migrated eventually). The reason is foresight. Obligation is basically meant to be a root Predicate, and the most important thing about it is its ability to carry origin information (ObligationCause, currently empty), which will be used when we start reporting trait solving errors. Better prepare for that day.
Some things ported/rewritten:
- Coercion, of course.
- Closure inference.
- Trait selection.
- Deep normalization, which already existed (discovered too late, as I said), and I kept my version because it was more complete.
- Autoderef.
- And probably few tiny more I don't rememberr.
Unfortunately, this PR also changes a lot of tests. Some are regressions I didn't want to fix because this PR is already huge, some are problem in the tests. I will comment on the tests separately.

ChayimFriedman2 · 2025-09-15T02:34:23Z

crates/hir-ty/src/infer/closure/analysis.rs

@@ -0,0 +1,1298 @@
+//! Post-inference closure analysis: captures and closure kind.


This file was just moved from crates/hir-ty/src/infer/closure.rs, to clean things up and also to not mix new solver and Chalk imports.

ChayimFriedman2 · 2025-09-15T02:35:28Z

crates/hir-ty/src/mir/eval/tests.rs

-                    Some(x)
-                } else {
-                    None
+    salsa::attach(db, || {


This file needed salsa::attach() in few places.

ChayimFriedman2 · 2025-09-15T02:36:18Z

It'd be really nice if we could share rustc_infer with rustc; we're basically duplicating it all over. Need to ask T-types and T-compiler for that.

ChayimFriedman2 · 2025-09-15T02:39:53Z

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

    let x = match 1 {
        1 => t as *mut i32,
-           //^^^^^^^^^^^^^ adjustments: Pointer(MutToConstPointer)
+           //^ adjustments: Deref(None), Borrow(RawPtr(Mut))


A theme that repeats here and in other tests is variance. Look at the comment I added to variances_of() for the detail, but in rough lines, bivariance causes problems with the new solver, and our tests had it all over the place. I fixed most of them before becoming upset and defaulting to Invariant in variance.rs; I still think there is value in fixing the test so I left them as is.

On an unrelated note, the change in adjustments from MutToConstPointer to Deref, Borrow is intentional - I believe rustc does the same.

ChayimFriedman2 · 2025-09-15T02:41:36Z

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

            73..94 'spam!(...am!())': {unknown}
            100..119 'for _ ...!() {}': fn into_iter<isize>(isize) -> <isize as IntoIterator>::IntoIter
-            100..119 'for _ ...!() {}': {unknown}
+            100..119 'for _ ...!() {}': <isize as IntoIterator>::IntoIter


This test has again associated type normalization and {unknown} accuracy in fluff; we're used to that with the new solver, I'm not sure under what condition that happens.

ChayimFriedman2 · 2025-09-15T02:43:40Z

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

            !0..13 '"helloworld!"': &'static str
            65..121 '{     ...")); }': ()
-            75..76 'x': &'static str
+            75..76 'x': &'? str


This is another theme repeating in the tests: we lost almost all of our lifetimes. Due to the significance of this I checked this, and this is kinda intentional: rustc has a region check pass (well, not exactly a pass) (https://rustc-dev-guide.rust-lang.org/type-inference.html#region-constraints) and without it our regions won't be resolved. We could add that pass later, or we could instead wait until we properly support lifetimes.

ChayimFriedman2 · 2025-09-15T02:46:55Z

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

 }

+// FIXME(next-solver): The never type fallback implemented in r-a no longer works properly because of
+// `Coerce` predicates. We should reimplement fallback like rustc.


To expand a bit on this: the never type fallback relies on being able to conclude from the diverging-ness of one infer var to another. It does this, simply, by resolving the root variable. Unfortunately, now this isn't enough: coercion between variables, also an element that needs to be taken into account in the fallback, can be delayed and stored in the form of Coerce predicates in the fulfillment context. Worse, unlike root vars, Coerce predicates form a graph and not even necessarily a DAG: therefore, some more sophisticated tracking is needed. Instead of complicating this PR even more, I opted to regress never types a bit and leave that as a future work.

ChayimFriedman2 · 2025-09-15T02:47:41Z

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

-            38..39 'y': {unknown}
-            41..43 '&y': &'? {unknown}
-            42..43 'y': {unknown}
+            20..21 'y': &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? &'? {unknown}


I have no idea what caused the blowup here, but it doesn't seem so bad: the test just tests we don't crash or something AFAIK.

What a relief that we don't have infinite recursion here 😌 (Maybe it encountered some recursion limit here?)

It's probably the solver (or something else) hitting its overflow check.

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

ShoyuVanilla · 2025-09-15T15:18:14Z

crates/hir-ty/src/infer/coerce.rs

+            // FIXME: We need to have an `upvars_mentioned()` query:
+            // At this point we haven't done capture analysis, which means
+            // that the ClosureArgs just contains an inference variable instead
+            // of tuple of captured types.


I guess at this point InferenceContext.result.closure_info would have been filled up for given closure_def_id_a?

Basically yes, closure analysis at the end of inference is equivalent to this query and should resolve the infer var. I didn't do this because I didn't want to overload this PR, but it should be pretty easy.

Leaving this as a follow-up sounds reasonable to me, as this is already about 10K loc PR 😆

ShoyuVanilla · 2025-09-15T15:19:15Z

crates/hir-ty/src/infer/coerce.rs

+            let is_capturing_closure = |_ty: Ty<'db>| {
+                // FIXME:
+                // if let TyKind::Closure(closure_def_id, _args) = ty.kind() {
+                //     self.db.upvars_mentioned(closure_def_id.expect_local()).is_some()


And might this be like the above one? (I might be wrong 😅 )

Seems like you are not 😅

ShoyuVanilla · 2025-09-15T15:31:12Z

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

+            }
+        }
+    }
+}


Finally we got some human-readable debug impl here 👍

That wasn't intended in fact, I just did that for my own convenience and left it here, but well, instead of everyone doing the same for their convenience then reverting it let's save their time 😅

ShoyuVanilla · 2025-09-15T15:35:18Z

crates/hir-ty/src/next_solver/infer/relate/generalize.rs

        //
        // cc trait-system-refactor-initiative#110
-        if self.infcx.next_trait_solver() && !alias.has_escaping_bound_vars() && !self.in_alias {
+        if !alias.has_escaping_bound_vars() && !self.in_alias {


Seems like Jack forgot to remove this 😄

crates/hir-ty/src/next_solver/fulfill/errors.rs

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

…iagnostics This started from porting coercion, but ended with porting much more.

ShoyuVanilla

Looks great overall! I couldn’t spot no more then just few questionable lines, which is impressive given how large this change is. Fantastic work. It’s hard to imagine how much effort went into pulling this off in just a few days!

I think it's totally fine to merge this now but would you like to wait for reviews from other rust-analyzer or types team members?

ChayimFriedman2 · 2025-09-15T16:16:38Z

I think we don't need another full review, but given the size of the PR I'll wait for at least one more approval: CC @Veykril @lnicola @davidbarsky @flodiebold.

I don't expect type team members to review (or have the capacity to review) this.

ChayimFriedman2 · 2025-09-18T00:19:19Z

I'm merging this based on the above @ShoyuVanilla review and @jackh726's vibe approval on Zulip.

lnicola · 2025-10-14T15:23:42Z

changelog fixup #20329

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

ChayimFriedman2 commented Sep 15, 2025

View reviewed changes

ChayimFriedman2 force-pushed the coerce-ns branch from 616c9d3 to badc11f Compare September 15, 2025 02:52

ChayimFriedman2 mentioned this pull request Sep 15, 2025

analysis-stats panics on rust-std #20648

Closed

ShoyuVanilla reviewed Sep 15, 2025

View reviewed changes

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

ShoyuVanilla reviewed Sep 15, 2025

View reviewed changes

crates/hir-ty/src/next_solver/fulfill/errors.rs Show resolved Hide resolved

ShoyuVanilla reviewed Sep 15, 2025

View reviewed changes

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

Port a bunch of stuff from rustc and fix a bunch of type mismatches/d…

7d18608

…iagnostics This started from porting coercion, but ended with porting much more.

ChayimFriedman2 force-pushed the coerce-ns branch from badc11f to 7d18608 Compare September 15, 2025 15:56

ShoyuVanilla approved these changes Sep 15, 2025

View reviewed changes

ShoyuVanilla mentioned this pull request Sep 17, 2025

rust-analyzer subtree update rust-lang/rust#146238

Closed

ChayimFriedman2 mentioned this pull request Sep 17, 2025

False positive: Non-primitive type cast error if the function has a constrained lifetime #20466

Open

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

Merged via the queue into rust-lang:master with commit cd31e11 Sep 18, 2025
15 checks passed

ChayimFriedman2 deleted the coerce-ns branch September 18, 2025 00:30

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

ChayimFriedman2 mentioned this pull request Sep 21, 2025

Lifetime-extended variable incorrectly reported as 'static #20510

Closed

		@@ -0,0 +1,1298 @@
		//! Post-inference closure analysis: captures and closure kind.

fix: Port a bunch of stuff from rustc and fix a bunch of type mismatches/diagnostics #20664

fix: Port a bunch of stuff from rustc and fix a bunch of type mismatches/diagnostics #20664

Uh oh!

Conversation

ChayimFriedman2 commented Sep 15, 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!

ChayimFriedman2 commented Sep 15, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShoyuVanilla Sep 15, 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!

Choose a reason for hiding this comment

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!

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!

Uh oh!

Uh oh!

ShoyuVanilla left a comment

Choose a reason for hiding this comment

Uh oh!

ChayimFriedman2 commented Sep 15, 2025

Uh oh!

ChayimFriedman2 commented Sep 18, 2025

Uh oh!

Uh oh!

lnicola commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ChayimFriedman2 commented Sep 15, 2025 •

edited

Loading

ShoyuVanilla Sep 15, 2025 •

edited

Loading