Move and expand the big `rustc_query_impl` macro into a physical `query_impl.rs` by Zalathar · Pull Request #153760 · rust-lang/rust

Zalathar · 2026-03-12T07:39:01Z

While looking through #153588, I came up with a related but different change that I think resolves a lot of tension in the current module arrangement.

The core idea is that if we both define and expand the big macro in the same physical module rustc_query_impl::query_impl, then we no longer need to worry about where mod query_impl should be declared, or where its imports should go, because those questions now have simple and obvious answers.

The second commit follows up with some more changes inspired by #153588. Those particular follow-ups are not essential to the main idea of this PR.

r? nnethercote

Zalathar · 2026-03-12T07:40:46Z

cc @Zoxc @zetanumbers on this structural change to rustc_query_impl.

The idea of splitting off the big macro has some overlap with the ideas in #151977, but I think the key insight of this PR is that there's a lot of benefit from defining and expanding the macro in the same module.

nnethercote

It is good to define and call define_queries in the same file.

Are you planning to do the same for define_callbacks? I like when it and define_queries are as similar as possible. That would require moving define_callbacks into compiler/rustc_middle/src/queries.rs.

View changes since this review

nnethercote · 2026-03-13T03:38:56Z

compiler/rustc_query_impl/src/query_impl.rs

@@ -0,0 +1,263 @@
+use rustc_middle::query::plumbing::QueryVTable;


Aside: feels weird that QueryVTable isn't obtainable via rustc_middle::query::plumbing.

You mean it not being re-exported from rustc_middle::query? Yeah, we should fix that at some point.

compiler/rustc_query_impl/src/query_impl.rs

nnethercote · 2026-03-13T03:41:42Z

compiler/rustc_query_impl/src/query_impl.rs

+                    mode: QueryMode,
+                ) -> Option<Erased<queries::$name::Value<'tcx>>> {
+                    #[cfg(debug_assertions)]
+                    let _guard = tracing::span!(tracing::Level::TRACE, stringify!($name), ?key).entered();


Aside: I wonder if this is necessary. The non-incr function doesn't have it.

nnethercote · 2026-03-13T03:55:57Z

compiler/rustc_query_impl/src/query_impl.rs


            pub(crate) mod execute_query_incr {
                use super::*;
+                use rustc_middle::queries::$name::{Key, Value};


You import Key and Value multiple times, but don't import Cache. Can the repeated imports be avoided by importing once at the top of mod $name, next to where you import Erased?

To me, the important thing about these local imports is that they're local, being physically quite close to the code that relies on them.

We could import more things into mod $name, but I think that would make it harder to mentally resolve imports, which is a big concern in macro-generated code.

If anything, I'm tempted to duplicate the import of erase/Erased into more local imports, closer to the places that use them.

Actually, I realised that I can hoist the imports of erase/Erased out to the top of the module, so they're no longer in macros at all.

nnethercote · 2026-03-13T03:56:55Z

compiler/rustc_query_impl/src/query_impl.rs

                    description_fn: $crate::queries::_description_fns::$name,
                    execute_query_fn: if incremental {
-                        query_impl::$name::execute_query_incr::__rust_end_short_backtrace
+                        crate::query_impl::$name::execute_query_incr::__rust_end_short_backtrace


crate::query_impl::$name can be shortened to just $name, I think? Here and a few cases below.

I think it happens to work because of the use super::*; import, but generally I'd prefer not to see $name outside of a fully-qualified path (or stringify!).

In the context of a macro, it's so much easier to know how $name is being used if you can see the fully-qualified path directly in the source code.

Zalathar · 2026-03-13T05:06:17Z

It is good to define and call define_queries in the same file.

Are you planning to do the same for define_callbacks? I like when it and define_queries are as similar as possible. That would require moving define_callbacks into compiler/rustc_middle/src/queries.rs.

In the case of rustc_middle, things are a bit trickier.

I'm pretty confident that we don't want to move the big plumbing macro into the same source file as the list of query declarations; that would be far worse than any inconsistency between rustc_middle and rustc_query_impl.

The expansion of the big rustc_middle macro also really wants to have access to the plain imports in rustc_middle::queries, either directly or by glob-import in a submodule.

Currently I can think of two plausible resolutions:

Define and call the big rustc_middle macro in a submodule of rustc_middle::queries, for maximum consistency with rustc_query_impl.
Define the big rustc_middle macro in a submodule of rustc_middle::queries, but continue to expand it into queries, for a compromise between consistency and local considerations.

nnethercote

r=me but I will wait until #153707 merges before approving this because the two PRs will conflict.

View changes since this review

nnethercote · 2026-03-13T22:08:03Z

#153707 has merged. r=me after rebasing.

@bors delegate=Zalathar

rust-bors · 2026-03-13T22:08:07Z

Invalid command: Invalid delegation type Zalathar. Possible values are try/review. Run @bors help to see available commands.

Moving the macro and its expansion into the same physical file resolves a lot of tension in the current module arrangement. Code in the macro is now free to use plain imports in the same file, and there is no longer any question of whether `mod query_impl` should be declared inside the macro, or surrounding a separate expansion site.

rustbot · 2026-03-14T04:44:33Z

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.

Zalathar · 2026-03-14T04:45:20Z

I rebased, but I'm going to spend some extra time checking that I didn't accidentally break anything subtle.

Zalathar · 2026-03-14T04:56:01Z

I used git diff --color-moved --color-moved-ws=allow-indentation-change head^^ head^ to verify that the moved code is still faithful to the original after the rebase (except for some minor intended changes), so this all looks good to go once again.

@bors r=nnethercote

Zalathar · 2026-03-14T04:57:01Z

@bors r=nnethercote

rust-bors · 2026-03-14T04:57:04Z

📌 Commit 69ea0c5 has been approved by nnethercote

It is now in the queue for this repository.

Move and expand the big `rustc_query_impl` macro into a physical `query_impl.rs` While looking through rust-lang#153588, I came up with a related but different change that I think resolves a lot of tension in the current module arrangement. The core idea is that if we both define and expand the big macro in the same physical module `rustc_query_impl::query_impl`, then we no longer need to worry about where `mod query_impl` should be declared, or where its imports should go, because those questions now have simple and obvious answers. The second commit follows up with some more changes inspired by rust-lang#153588. Those particular follow-ups are not essential to the main idea of this PR. r? nnethercote

Rollup of 7 pull requests Successful merges: - #152621 (LinkedGraph: support adding nodes and edges in arbitrary order) - #153376 (Replace `visit_waiters` with `abstracted_waiters_of`) - #153760 (Move and expand the big `rustc_query_impl` macro into a physical `query_impl.rs`) - #153818 (Reimplement const closures) - #153774 (Fix std doctest build for SGX target.) - #153786 (Remove `value_from_cycle_error` specializations) - #153819 (delete some duplicated tests)

rustbot assigned nnethercote Mar 12, 2026

Zalathar mentioned this pull request Mar 12, 2026

define_queries! module tweaks #153588

Closed

Zalathar force-pushed the query-impl branch from 9642fba to 9d17f2c Compare March 12, 2026 07:46