Migrate `InferenceTable` into next-solver #20578

ShoyuVanilla · 2025-08-30T18:47:12Z

Still work in progress and I have 4 remaining failing tests

failures:
    consteval::tests::closure_and_impl_fn
    mir::eval::tests::closure_layout_in_rpit
    mir::lower::tests::dyn_projection_with_auto_traits_regression_next_solver
    tests::traits::builtin_binop_expectation_works_on_single_reference

The last one seems that we are not applying some relates between int inference vars and the other three are related to rust-analyzer's PlaceholderTy crimes: We are interning its parent generic and local idx and then using that intern id into Placeholder's index, quite an abuse.

So, next-gen solver is panicking while trying to instantiate canonicals containing that PlaceholderTy in such a manner like trying to access the tens of thousands index of original values.
I'm trying to mitigate it utilizing BoundTykind to discern those "abused" placeholders and normal placeholders but it still failing with something like:

ra-ap-rustc_next_trait_solver-0.123.0/src/solve/eval_ctxt/canonical.rs:422:29:
index out of bounds: the len is 1 but the index is 1

Still a problem but much better than the len is 2 but the index is 42002 🙃

I'm drafting this PR as I'm going on a vacation for next three days and cannot work on this so much for that period 😅 So fill free to use this as coarse ingredients if we should do the migration in a rush 😄

ChayimFriedman2 · 2025-09-01T18:13:17Z

That seems especially grievous given that unlike Chalk, the next solver gives us precise control on what is in Placeholder and Param, so no transmute is needed 🫨

I'll try to see if we still need the info, and if yes create a new PR to switch to them, to not overload this PR.

ShoyuVanilla · 2025-09-04T05:14:06Z

The same tests are still failing, but things improved significantly after #20586. One failure is related to InferenceVar relations (same as before). The other three have mitigated from index-out-of-bound errors to method resolution failures, since they now encounter unnormalized opaque types during MIR lowering. Normalizing them in the right place should resolve the issues.

I’ll work on finishing this once I’m back from work today 😄

ChayimFriedman2 · 2025-09-04T08:51:03Z

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


 pub trait ChalkToNextSolver<'db, Out> {
    fn to_nextsolver(&self, interner: DbInterner<'db>) -> Out;
+    fn from_nextsolver(out: Out, interner: DbInterner<'db>) -> Self;


These things already exist, just under the name convert_X_from_result. Maybe a method in the trait make sense (but I guess then ChalkToNextSolver isn't an appropriate name?) but no need to recreate them.

Yes, I'm thinking of separating this into two traits, as after a while, it has panned out that some from_nextsolver method aren't called at all. Maybe I could do something like

enum Never {} trait ChalkToNextSolver<'db, Out> { type FromNextSolver = Self; fn from_nextsolver(..) -> Self::FromNextSolver; ... } impl<'db> ChalkToNextSolver<'db, Bar> for Foo { type FromNextSolver = Never; ... }

but it would not save for me not so many keystrokes 😅

Yes, so, I just moved the things from convert_X_for_result to trait methods (no need to keep the free functions around).

A separate trait or an associated type makes sense - I had done this because there were a lot of things that I had to convert back and forth compared to previously and this seemed better than creating a bunch of free functions.

A separate trait will allow us to use method call syntax, which is nice.

Separated this into new NextSolverToChalk

ChayimFriedman2 · 2025-09-04T08:56:42Z

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

-    /// temporary allocations.
-    resolve_obligations_buffer: Vec<Canonicalized<InEnvironment<Goal>>>,
+    pub(crate) infer_ctxt: InferCtxt<'a>,
+    diverging_tys: FxHashSet<Ty>,


This whole "diverging" flag on an infer var is probably incorrect, that's not how rustc does it AFAIK and it also leads to incorrect results (at least on edition 2024, see #20460 (comment)). But it's probably better to leave it out of this PR, just maybe leave a FIXME.

Okay, I'll check this

I think it would be better to defer resolving this as a follow up, as it would be a bit complicated

ChayimFriedman2 · 2025-09-04T09:25:05Z

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


-    #[tracing::instrument(level = "debug", skip(self))]
-    fn register_obligation_in_env(&mut self, goal: InEnvironment<Goal>) {
+    // If this goal is an `AliasEq` for an opaque type, just unify instead of trying to solve (since the next-solver is lazy)


Is it the correct thing to do? I don't think we should deviate from rustc here, rather follow it and change the callsites, otherwise we will have bugs.

If this is just a temporary workaround to shorten this PR then fine, though.

Actually, most lines of this PR are cherry-picked from Jack's branch and I haven't looked into this close enough yet 😅
But IIRC, next solver is quite lazy on unifying opaque types in general. I'll take relevant lines from rustc soon.

I just found what was on my mind.

So the following is the (sparse) call stacks made while coercing opaque/projection type in next-solver

https://github.com/rust-lang/rust/blob/79bdc627562db90599edcf16cb166a532919a0a8/compiler/rustc_infer/src/infer/at.rs#L221

https://github.com/rust-lang/rust/blob/79bdc627562db90599edcf16cb166a532919a0a8/compiler/rustc_type_ir/src/relate/solver_relating.rs#L48

https://github.com/rust-lang/rust/blob/79bdc627562db90599edcf16cb166a532919a0a8/compiler/rustc_type_ir/src/relate/solver_relating.rs#L243

https://github.com/rust-lang/rust/blob/79bdc627562db90599edcf16cb166a532919a0a8/compiler/rustc_type_ir/src/relate/combine.rs#L118-L120

At the last link, it just registers delayed obligation and returns Ok

The new solver uses lazy normalization, sure, but that means we should not deviate from rustc and eagerly normalize, rather we should adjust the callsites that require a normalized type (probably those inspecting the TyKind) to normalize themselves, and leave the rest doing lazy normalization, like rustc.

Ah, I see. So you’re suggesting that a more structured approach to lazy normalization would be preferable to a quick fix like this?

Yes. As I said, if you prefer this hack for this PR that's also fine, as long as eventually we will do that.

Okay. I'll revisit this once all tests turn green (still at my office 😅) Maybe we could reuse those infra from rustc_type_ir

I don't even remember what I was doing here, lol. At some point, when other things around start to fall into place, I think this should fairly easily be able to be removed. But for now, idk.

I don't even remember what I was doing here, lol. At some point, when other things around start to fall into place, I think this should fairly easily be able to be removed. But for now, idk.

Simply removing it causes more failures in tests. Will check how rustc handles it 😄

Removed this and managed to pass tests

ShoyuVanilla · 2025-09-07T21:04:27Z

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

            self.unify(&builtin_ret, &ret_ty);
+            builtin_ret
+        } else {
+            ret_ty


This corresponds with https://github.com/rust-lang/rust/blob/23718020b12d7e5a54f82a19910e8356ee719667/compiler/rustc_hir_typeck/src/op.rs#L150-L152 and is quite crucial for type inferences around builtin binops

ShoyuVanilla · 2025-09-08T07:52:29Z

crates/hir-ty/src/mir/monomorphization.rs

+                opaque_ty_id: id,
+                substitution: subst,
+            }))
+            | TyKind::OpaqueType(id, subst) => {


rustc_type_ir::TyKind doesn't have TyKind::OpaqueType and we are converting chalk_ir::TyKind::OpaqueType into rustc_type_ir::TyKind::Alias(Opaque) and back into chalk_ir::TyKind::Alias(Opaque) so we need to handle these cases as well 🥲

ShoyuVanilla · 2025-09-08T07:52:58Z

crates/hir-ty/src/method_resolution.rs

            }
            TyKind::AssociatedType(_, _)
            | TyKind::OpaqueType(_, _)
+            | TyKind::Alias(AliasTy::Opaque(_))


Same as https://github.com/rust-lang/rust-analyzer/pull/20578/files#r2329433975

ShoyuVanilla · 2025-09-08T08:04:55Z

crates/hir-ty/src/chalk_ext.rs

-                    ImplTraitId::AsyncBlockTypeImplTrait(..) => unreachable!(),
+                    ImplTraitId::AsyncBlockTypeImplTrait(def, _) => {
+                        return handle_async_block_type_impl_trait(def);
+                    }


Same as https://github.com/rust-lang/rust-analyzer/pull/20578/files#r2329433975

ShoyuVanilla · 2025-09-08T08:06:38Z

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

In general, we are inserting less inference vars in next-solver inference table so iv indices has been changed

ShoyuVanilla · 2025-09-08T08:09:36Z

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

I'm not sure these are regressions or improvements but seems that we are now leaving projection types of unknown selves as it is, rather than creating new unknown for them

Probably because of lazy normalization of the new solver, we only normalize at the end and to the same projection (can't tell why it's different from Chalk though). Not a problem I believe.

ShoyuVanilla · 2025-09-08T08:10:25Z

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

            56..58 '{}': ()
            72..171 '{     ... x); }': ()
-            78..81 'foo': fn foo<&'? (i32, &'? str), i32, impl FnOnce(&'? (i32, &'? str)) -> i32>(&'? (i32, &'? str), impl FnOnce(&'? (i32, &'? str)) -> i32) -> i32
+            78..81 'foo': fn foo<&'? (i32, &'static str), i32, impl FnOnce(&'? (i32, &'static str)) -> i32>(&'? (i32, &'static str), impl FnOnce(&'? (i32, &'static str)) -> i32) -> i32


Lifetime labels has been improved in some places and regressed in other places 🤔

ShoyuVanilla · 2025-09-08T08:13:24Z

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


-fn test<T>(t: T) where T: UnificationStoreMut {
+fn test<T>(mut t: T) where T: UnificationStoreMut {
    let x;


This and the above changes are fixing compilation errors on the test code and are necessary to make the type inference work. Next-solver (or this migration) is less forgiving to some errors 😅

That's quite unfortunate but I don't think that's something we can change.

Although... How can the mutness of a local impact inference?

Oh, mut wasn't necessary, but the above type param usize was. Seems that I just mindlessly fixed all compilation errors on this 😅

ShoyuVanilla · 2025-09-08T08:29:30Z

crates/ide-diagnostics/src/handlers/type_mismatch.rs

            fn main() {
-                run(f()) // FIXME: remove this error
-                  //^^^ error: expected Rate<5>, found Rate<_>
+                run(f())


Ideally, we should emit an error on const param N mismatch above while emitting no error here but the former should be done in wfcheck we are currently lacking of in rust-analyzer

Why is it a well-formedness check? It seems to be a normal type mismatch. Anyway we should not emit an error here, no idea why we did with Chalk.

I mean, wfcheck should emit an error on L523 and of course we should not emit an error here as fixed in this PR. I'm quite surprised that chalk has been emitting an error here 😅

ChayimFriedman2 · 2025-09-08T17:15:19Z

You seem to be using structurally_resolve_type() as a replacement for resolve_ty_shallow(), but this method deeply resolve vars, so it seem to not be a correct replacement. Why are you using it?

ShoyuVanilla · 2025-09-08T18:14:44Z

You seem to be using structurally_resolve_type() as a replacement for resolve_ty_shallow(), but this method deeply resolve vars, so it seem to not be a correct replacement. Why are you using it?

Because rustc does so in myriad places, especially with next-solver, like
https://github.com/rust-lang/rust/blob/68baa87ba6f03f8b6af2a368690161f1601e4040/compiler/rustc_hir_typeck/src/coercion.rs#L1039
https://github.com/search?q=repo%3Arust-lang%2Frust%20structurally_resolve_ty&type=code

As I remember, last time I looked into rustc_hir_typeck before it used mostly shallow_resolve_ty but I was quite surprised that most of them has been replaced by this when I looked into it again to get hints for fixing type inference failures caused by removing the above hack on #20578 (comment)

ChayimFriedman2

I have a few comments, but otherwise LGTM.

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

ChayimFriedman2 · 2025-09-08T18:30:05Z

crates/hir-ty/src/next_solver/infer/mod.rs

+    }
+
    pub fn next_ty_var_with_origin(&self, origin: TypeVariableOrigin) -> Ty<'db> {
        let vid = self.inner.borrow_mut().type_variables().new_var(self.universe(), origin);


Suggested change

let vid = self.inner.borrow_mut().type_variables().new_var(self.universe(), origin);

let vid = self.next_ty_vid();

Oh, wait. We should pass origin here.

ChayimFriedman2 · 2025-09-08T18:30:28Z

crates/hir-ty/src/next_solver/infer/mod.rs

+    }
+
    pub fn next_const_var_with_origin(&self, origin: ConstVariableOrigin) -> Const<'db> {
        let vid = self


Here's origin too

crates/hir-ty/src/next_solver/infer/mod.rs

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

ChayimFriedman2 · 2025-09-08T19:03:05Z

crates/ide-diagnostics/src/handlers/type_mismatch.rs

            fn main() {
-                run(f()) // FIXME: remove this error
-                  //^^^ error: expected Rate<5>, found Rate<_>
+                run(f())


Why is it a well-formedness check? It seems to be a normal type mismatch. Anyway we should not emit an error here, no idea why we did with Chalk.

crates/ide/src/doc_links.rs

crates/hir-ty/src/next_solver/infer/canonical/canonicalizer.rs

ChayimFriedman2 · 2025-09-08T19:11:16Z

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

 pub(crate) struct InferenceTable<'a> {
    pub(crate) db: &'a dyn HirDatabase,
+    pub(crate) interner: DbInterner<'a>,
    pub(crate) trait_env: Arc<TraitEnvironment>,


We will eventually need to replace TraitEnvironment with ParamEnv, but not in this PR.

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

ShoyuVanilla · 2025-09-08T19:58:43Z

I have a few comments, but otherwise LGTM.

Thanks for the thorough review! I’ll take care of the fixes tomorrow once I’m back from work.

ChayimFriedman2

Extremely excited for this!

ShoyuVanilla · 2025-09-09T16:54:46Z

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

    pub(super) fn clauses_for_self_ty(
        &mut self,
        self_ty: InferenceVar,
    ) -> SmallVec<[WhereClause; 4]> {


It would be better to return rustc_type_ir's predicates here but the callsite applies elaboration(in our codebase) to the output and I'm suspecting that current Elaboratable implementation in our next-solver that called from rustc_type_ir is a bit buggy and that might be the cause of #20582. Will fix it after this PR

lnicola · 2025-10-14T12:49:39Z

changelog fixup #20329

ShoyuVanilla force-pushed the inftbl branch from 768965b to f2c0406 Compare August 30, 2025 18:48

ChayimFriedman2 mentioned this pull request Sep 2, 2025

fix: Make sense of the mess that were (are) different kind of generics in the solver #20586

Merged

ShoyuVanilla force-pushed the inftbl branch from f2c0406 to 0933850 Compare September 3, 2025 14:12

ChayimFriedman2 reviewed Sep 4, 2025

View reviewed changes

ChayimFriedman2 mentioned this pull request Sep 4, 2025

internal: Upgrade rustc crates #20609

Merged

ShoyuVanilla force-pushed the inftbl branch 3 times, most recently from ae0cbc7 to 089ef7e Compare September 7, 2025 20:46

ShoyuVanilla marked this pull request as ready for review September 7, 2025 20:47

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

ShoyuVanilla force-pushed the inftbl branch from 089ef7e to fdff671 Compare September 7, 2025 20:59

ShoyuVanilla commented Sep 7, 2025

View reviewed changes

ShoyuVanilla force-pushed the inftbl branch from fdff671 to b52cd0b Compare September 7, 2025 21:11

ShoyuVanilla commented Sep 8, 2025

View reviewed changes

ChayimFriedman2 reviewed Sep 8, 2025

View reviewed changes

ShoyuVanilla mentioned this pull request Sep 9, 2025

[next-trait] expected &dyn T found &Y #20487

Open

jackh726 and others added 2 commits September 9, 2025 22:45

WIP switch inference table to next-solver

d24e8c1

Fix failing tests and fill-in missing details

4a8bc8d

ShoyuVanilla force-pushed the inftbl branch from b52cd0b to 4a8bc8d Compare September 9, 2025 16:43

ChayimFriedman2 approved these changes Sep 9, 2025

View reviewed changes

ChayimFriedman2 enabled auto-merge September 9, 2025 16:54

ShoyuVanilla commented Sep 9, 2025

View reviewed changes

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

Merged via the queue into rust-lang:master with commit 412932e Sep 9, 2025
15 checks passed

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

ShoyuVanilla deleted the inftbl branch September 9, 2025 17:09

nicolas-guichard mentioned this pull request Sep 9, 2025

Type inference failure on let foo: [_; _] = [false] as _; #20644

Closed

ShoyuVanilla mentioned this pull request Sep 10, 2025

internal: Upgrade rustc crates #20645

Merged

	let vid = self.inner.borrow_mut().type_variables().new_var(self.universe(), origin);
	let vid = self.next_ty_vid();

Migrate InferenceTable into next-solver #20578

Migrate InferenceTable into next-solver #20578

Uh oh!

Conversation

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

Uh oh!

Uh oh!

ChayimFriedman2 commented Sep 1, 2025

Uh oh!

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

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!

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 7, 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!

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

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!

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

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

Uh oh!

Choose a reason for hiding this comment

Migrate `InferenceTable` into next-solver #20578

Migrate `InferenceTable` into next-solver #20578

ShoyuVanilla commented Aug 30, 2025 •

edited

Loading

ShoyuVanilla Sep 7, 2025 •

edited

Loading

ShoyuVanilla Sep 8, 2025 •

edited

Loading

ShoyuVanilla Sep 9, 2025 •

edited

Loading

ShoyuVanilla Sep 8, 2025 •

edited

Loading