Start cutting down `rustc_query_system` by nnethercote · Pull Request #152160 · rust-lang/rust

nnethercote · 2026-02-05T09:20:08Z

The query system is implemented in rustc_query_system, rustc_middle, and rustc_query_impl. rustc_query_system is hamstrung by not having access to TyCtxt, and there seems to be consensus to eliminate it. It's contents can be moved into the other two crates. Moving as much stuff as possible to rustc_query_impl is preferred, because rustc_middle is already so big.

This PR starts this process. It moves one small function to rustc_middle and a good chunk of code to rustc_query_impl.

Once rustc_query_system is gone (or at least shrunk down a lot more) some of the traits like DepContext, QueryContext, and QueryDispatcher will be removable.

r? @Zalathar

nnethercote · 2026-02-05T09:20:24Z

cc @cjgillot @Zoxc

Zoxc · 2026-02-05T10:04:41Z

I'd prefer to apply the rustc_query_system::query::plumbing -> execution renaming in #151977 and then probably move the entire file, instead of growing rustc_query_impl's plumbing module.

nnethercote · 2026-02-05T10:16:03Z

You can't move the entire file. Some parts of it are used by rustc_middle, some aren't, so it needs to be split up accordingly.

Zoxc · 2026-02-05T10:26:12Z

Not enough moved for git to pick up a rename? It could be moved to a execution module in rustc_query_impl at least.

Zalathar · 2026-02-05T10:29:03Z

Adding a few hundred more lines to rustc_query_impl::plumbing is a bit unfortunate.

I'm not sure whether this will work out in practice, but here's a potential approach:

Take the parts of rustc_query_system::query::plumbing that are not moving, and extract them into a separate module to remain in rustc_query_system.
Move and rename rustc_query_system::query::plumbing, making it a submodule of rustc_query_impl::plumbing to minimize visibility headaches.

That would produce an end result similar to the current state of this PR, but with the migrated code still in its own separate file, and preserving as much line history as possible.

nnethercote · 2026-02-05T10:48:51Z

Adding a few hundred more lines to rustc_query_impl::plumbing is a bit unfortunate.

Why? The same few hundred lines are removed from rustc_query_system.

That would produce an end result similar to the current state of this PR, but with the migrated code still in its own separate file, and preserving as much line history as possible.

It would also result in an odd module structure that reflects historical evolution rather than what makes sense today. rustc_query_system is 5,000 lines of code. If we split it across two other modules there's going to be some churn. I am a firm believer that the current version of the code is the most important, and good code structure shouldn't be held hostage by concerns about version control history.

Zalathar · 2026-02-05T11:17:31Z

I tried prototyping my proposal, and while I still like it on paper, I came away with the distinct impression that it's probably not better enough to justify the extra fiddling and delay, compared to just moving forward with this PR.

Ultimately, what's most important is unlocking the sorts of simplification and improvement that are impossible with the current module split.

That's the only place it's used, so it no longer needs to be `pub`.

Instead of writing it by hand.

…umbing`. We are in the process of eliminating `rustc_query_system`. Chunks of it are unused by `rustc_middle`, and so can be moved into `rustc_query_info`. This commit does some of that. Mostly it's just moving code from one file to another. There are a couple of non-trivial changes. - `QueryState` and `ActiveKeyStatus` must remain in `rustc_query_system` because they are used by `rustc_middle`. But their inherent methods are not used by `rustc_middle`. So these methods are moved and converted to free functions. - The visibility of some things must increase. This includes `DepGraphData` and some of its methods, which are now used in `rustc_query_impl`. This is a bit annoying but seems hard to avoid. What little is left behind in `compiler/rustc_query_system/src/query/plumbing.rs` will be able to moved into `rustc_query_impl` or `rustc_middle` in the future.

The previous commit moved some code from `rustc_query_system`, which doesn't have access to `TyCtxt` and `QueryCtxt`, to `rustc_query_impl`, which does. We can now remove quite a bit of indirection. - Three methods in `trait QueryContext` are no longer needed (`next_job_id`, `current_query_job`, `start_query`). As a result, `QueryCtxt`'s trait impls of these methods are changed to inherent methods. - `qcx: Q::Qcx` parameters are simplified to `qcx: QueryCtxt<'tcx>`. - `*qcx.dep_context()` occurrences are simplified to `qcx.tcx`, and things like `qcx.dep_context().profiler()` become `qcx.tcx.prof`. - `DepGraphData<<Q::Qcx as HasDepContext>::Deps>` becomes `DepGraphData<DepsType>`. In short, various layers of indirection and abstraction are cut away. The resulting code is simpler, more concrete, and easier to understand. It's a good demonstration of the benefits of eliminating `rustc_query_system`, and there will be more to come.

Zalathar · 2026-02-06T03:25:49Z

It would also result in an odd module structure that reflects historical evolution rather than what makes sense today. rustc_query_system is 5,000 lines of code. If we split it across two other modules there's going to be some churn. I am a firm believer that the current version of the code is the most important, and good code structure shouldn't be held hostage by concerns about version control history.

I do want to push back on this a bit.

Taking ~500 lines of code from system/plumbing and shoving it in impl/pluming is useful in the sense that it unlocks within-crate simplifications, but it's most certainly not a good module structure.

One of the first things I'd want to do is take big chunks of that code and extract it into one or more separate modules, so that it isn't making the impl/plumbing macros significantly harder to deal with.

And because the code has been isolated in a separate crate up to this point, it does represent a fairly “natural” boundary for the code that currently exists. IIRC, the main point of interaction between impl/plumbing and system/plumbing is through the get_query_incr and get_query_non_incr functions, with most of the rest of system/plumbing being internal details of those two public functions.

(For comparison, look at middle/plumbing, where I made a point of taking some functions that exist to be called by macro-generated functions, and moved them out to a separate inner submodule.)

So my main motivation for wanting to move the code over in a separate file is not to preserve history (though that's nice if we can get it), but because I think it's genuinely a better module structure for the current state of the code.

rustbot · 2026-02-06T04:54:28Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

nnethercote · 2026-02-06T04:57:07Z

My motivation is to get rid of all the different layers of indirection. I personally find that kind of thing much more of a barrier to understanding than module structure. I've added a new commit that starts to remove the indirection. It's nice!

Zalathar · 2026-02-06T05:07:33Z

The cleanups are nice, and it will be nice to unlock even more, so let’s try to get this into the next big rollup when the tree reopens.

@bors r+

rust-bors · 2026-02-06T05:07:36Z

📌 Commit d6218b6 has been approved by Zalathar

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

rustbot assigned Zalathar Feb 5, 2026

This comment has been minimized.

Sign in to view

nnethercote added 4 commits February 6, 2026 09:23

Move try_get_cached from rustc_query_system to rustc_middle.

6355d33

That's the only place it's used, so it no longer needs to be `pub`.

Derive Clone for QueryJob.

2a691ca

Instead of writing it by hand.

nnethercote force-pushed the start-cutting-down-rustc_query_system branch from 4f0dd45 to d6218b6 Compare February 6, 2026 04:54

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 6, 2026

Uh oh!

Conversation

nnethercote commented Feb 5, 2026

Uh oh!

nnethercote commented Feb 5, 2026

Uh oh!

Zoxc commented Feb 5, 2026

Uh oh!

nnethercote commented Feb 5, 2026

Uh oh!

Zoxc commented Feb 5, 2026

Uh oh!

Zalathar commented Feb 5, 2026

Uh oh!

This comment has been minimized.

nnethercote commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

Zalathar commented Feb 5, 2026

Uh oh!

Zalathar commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 6, 2026

Uh oh!

nnethercote commented Feb 6, 2026

Uh oh!

Zalathar commented Feb 6, 2026

Uh oh!

rust-bors bot commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nnethercote commented Feb 5, 2026 •

edited

Loading

Zalathar commented Feb 6, 2026 •

edited

Loading