feat: Switch from Chalk to the next trait solver by jackh726 · Pull Request #20329 · rust-lang/rust-analyzer

jackh726 · 2025-07-28T14:07:21Z

At this point, opening up for general review, though I don't think this is fully ready to merge.

Some checklist (non-exhaustive) of things to do before/after merge (feel free to edit):

Before:

Fix the tidy lints (currently, I see that we need to add doc tests to the new files)
Squash (or just rework) commits
Review all the changed tests, open an issue to track the "real" regressions
Go through the todo!()s and FIXMEs and decide which are needed/useful to fix prior to merge
Decide what (if anything) should get upstreamed in a separate PR(s) first
Switch with_db calls to ScopedDb::set_db (or decide on a better API)

After (before stable):

Test across different projects, find things that are "blocking" (probably after merge)
Figure out where memory opts can be made

After (any time):

Transition things to from chalk_ir to rustc_type_ir
- Inference table blocks a lot of things (e.g. normalization)
- Some queries are "easy" like layout_of_ty

Mixed:

General cleanup of things
- Reduce/remove allow(unused)

ChayimFriedman2 · 2025-07-31T15:40:42Z

@@ -48,7 +48,8 @@ impl ToTokens for TrackedQuery {
                quote!(#(#options),*)
            })
            .into_iter()
-            .chain(self.lru.map(|lru| quote!(lru = #lru)));
+            .chain(self.lru.map(|lru| quote!(lru = #lru)))
+            .chain(Some(quote!(unsafe(non_update_return_type))));


That should be changed before merge, right?

Sure, if someone can point me to how to fix it.

ChayimFriedman2

First round of review, I think that's what I will be able to do today.

ChayimFriedman2 · 2025-07-31T15:53:33Z

 mod inhabitedness;
 mod interner;
 mod lower;
+mod lower_nextsolver;


Are all those X_nextsolver modules supposed to replace their original before merge?

I think that's pretty much impossible if we want this to land anytime soon. There is so much that relies on the non-next solver modules that goes beyond trait solving.

ChayimFriedman2 · 2025-07-31T15:58:51Z

    #[salsa::cycle(cycle_result = crate::drop::has_drop_glue_cycle_result)]
    fn has_drop_glue(&self, ty: Ty, env: Arc<TraitEnvironment>) -> DropGlue;
+
+    #[salsa::invoke(crate::layout_nextsolver::layout_of_adt_query)]


The fully qualified names everywhere (crate::X_nextsolver::X) make the code harder to read, but I suppose we'll get rid of that once we remove the Chalk types.

Yeah, if there are specific places that make sense to remove them, let me know. But unfortunately a lot of them are kind of necessary.

jackh726 · 2025-08-01T15:43:29Z

Squashed and rebased version here: jackh726@5fa8436

Will switch this PR after a bit of review, just so people can see commits if they need.

Veykril

Just a quick glance

Veykril · 2025-08-08T14:07:41Z

Regarding the remaining todos on the issue tracker itself, I think squashing commits would be good (or just cleaning them up a bit), upstreaming things I feel like can be done after merge as well so not that important. Tests I'll plan on looking at this weekend. And then I think we are mostly ready? (object if not, I have not been catching up with things here and on zulip).

So one more thing we ought to do is check if there is any important unrelated bug fix we wanna get out on the last stable before this merge.

Veykril · 2025-08-09T10:07:43Z

            ex Bar
+            ex Bar::foo()
            ex bar()


Not really a regression by the PR and so not relevant, but all 3 of these are wrong, we shouldn't show expression completions in this position

Veykril · 2025-08-09T10:10:16Z

@@ -108,7 +108,7 @@ pub(crate) fn goto_definition(
            }

            Some(
-                IdentClass::classify_node(sema, &parent)?
+                salsa::attach(sema.db, || IdentClass::classify_node(sema, &parent))?


We should probably have the various Analysis methods just attach the salsa db instead, that is in Analysis::with_db. That should allow us to remove all those intermediate salsa::attach calls in the ide crates (except for tests setups maybe)

I did think about this and tried to change with_db to

let snap = &snap; salsa::attach(snap, || { Cancelled::catch(|| f(snap)) })

which results in Cannot change database mid-query.

That is odd, and sounds kind of worrying ... I can take a look at that later maybe.

Ah okay at least part of this is due to syntax highlighting building a new db for comments, so probably not that problematic. A change for later then.

jackh726 · 2025-08-09T16:27:38Z

I've squashed the commits.

Snowiiii · 2025-08-10T09:24:40Z

What benefits does this bring ?

ChayimFriedman2 · 2025-08-10T09:32:31Z

@Snowiiii

What benefits does this bring ?

A lot!

Improved performance now, and even more in the future as the next trait solver is optimized more (e.g. Jack reported analysis-stats took half the time!)
Improved accuracy. Most of our false positive errors come from bugs in Chalk. Switching to the new trait solver may finally enable marking type mismatch diagnostics as no longer experimental.
General maintenance, as Chalk is no longer maintained.

ShoyuVanilla · 2025-08-10T13:16:59Z

Yes, without this, type inference failures and bugs would accumulate over time as Rust continues to evolve.

ShoyuVanilla · 2025-08-10T15:02:29Z

I’ve opened #20422 to track the test regressions. It looks like we only have a few to fix. 😄 I might be wrong on some of them or have missed something, so feel free to edit

davidbarsky · 2025-08-12T16:11:10Z

@Snowiiii

What benefits does this bring ?

A lot!

Improved performance now, and even more in the future as the next trait solver is optimized more (e.g. Jack reported analysis-stats took half the time!)

Improved accuracy. Most of our false positive errors come from bugs in Chalk. Switching to the new trait solver may finally enable marking type mismatch diagnostics as no longer experimental.

General maintenance, as Chalk is no longer maintained.

To expand on "improved performance", the work that analysis-stats mostly measures correspond to things like rust-analyzer's native diagnostics and autocomplete. Features like the initial indexing at startup or code navigation are not impacted by the new trait solver.

ChayimFriedman2 · 2025-08-12T16:31:11Z

Indeed it's not startup, but it basically impacts everything else.

Veykril · 2025-08-13T06:10:42Z

Alright let's give this a spin!

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

ChayimFriedman2 reviewed Jul 31, 2025

View reviewed changes

rust-cloud-vms Bot force-pushed the next-trait-solver-querify branch from 0c5be1c to 57c7b7d Compare July 31, 2025 21:36

Veykril reviewed Aug 1, 2025

View reviewed changes

rust-cloud-vms Bot force-pushed the next-trait-solver-querify branch 2 times, most recently from eb38567 to ab8258d Compare August 2, 2025 01:18

Veykril force-pushed the next-trait-solver-querify branch from 93688bb to fbf45f7 Compare August 2, 2025 07:23

ShoyuVanilla mentioned this pull request Aug 5, 2025

internal: merge Trait and TraitAlias handling #20376

Merged

ShoyuVanilla reviewed Aug 6, 2025

View reviewed changes

Comment thread crates/hir-ty/src/layout_nextsolver.rs Outdated

ShoyuVanilla reviewed Aug 6, 2025

View reviewed changes

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

ChayimFriedman2 reviewed Aug 7, 2025

View reviewed changes

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

rust-cloud-vms Bot force-pushed the next-trait-solver-querify branch from 1e93992 to 84377be Compare August 8, 2025 21:18

Veykril reviewed Aug 9, 2025

View reviewed changes

Implement next trait solver

9418a3f

rust-cloud-vms Bot force-pushed the next-trait-solver-querify branch from e3f8237 to 9418a3f Compare August 9, 2025 16:10

ShoyuVanilla mentioned this pull request Aug 10, 2025

Tracking Issue: Test regressions in next-generation trait solver migration #20422

Open

9 tasks

Veykril added this pull request to the merge queue Aug 13, 2025

Merged via the queue into rust-lang:master with commit a9450eb Aug 13, 2025
15 checks passed

Uh oh!

Uh oh!

Conversation

jackh726 commented Jul 28, 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 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jackh726 commented Aug 1, 2025

Uh oh!

Veykril left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Veykril commented Aug 8, 2025

Uh oh!

Veykril Aug 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!

Veykril Aug 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!

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!

jackh726 commented Aug 9, 2025

Uh oh!

Snowiiii commented Aug 10, 2025

Uh oh!

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

Uh oh!

Uh oh!

ShoyuVanilla commented Aug 10, 2025

Uh oh!

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

Uh oh!

Uh oh!

davidbarsky commented Aug 12, 2025

Uh oh!

ChayimFriedman2 commented Aug 12, 2025

Uh oh!

Veykril commented Aug 13, 2025

Uh oh!

Uh oh!

Reviewers

jackh726 commented Jul 28, 2025 •

edited

Loading

Veykril Aug 9, 2025 •

edited

Loading

Veykril Aug 9, 2025 •

edited

Loading

ChayimFriedman2 commented Aug 10, 2025 •

edited

Loading

ShoyuVanilla commented Aug 10, 2025 •

edited

Loading