Make `rustc_with_all_queries!` pass query modifiers as named values by Zalathar · Pull Request #153326 · rust-lang/rust

Zalathar · 2026-03-03T03:42:10Z

This PR is a bold overhaul of how the proc-macro in rustc_macros::query passes query modifiers to the callback macros in rustc_middle and rustc_query_impl.

The existing approach passes modifiers as a list that looks like [(arena_cache), (no_hash)]. That style requires a family of helper macros (if_arena_cache!, if_no_hash!) to check for modifiers when consuming the query list.

This PR changes the proc-macro to instead pass modifiers like this:

{
    anon: false,
    arena_cache: true,
    cache_on_disk: false,
    ...
}

This style allows each of the callback macros to deconstruct the modifier list in a relatively straightforward way, by binding the true/false literals to variables like $arena_cache:literal.

One of the big advantages of this style is that we can write things like #[cfg($arena_cache)] and #[cfg(not($arena_cache))] to select blocks of code, eliminating the need for the if_arena_cache! family of helper macros.

In follow-up PRs, we can also try to take advantage of the new modifier style to pass richer information for some modifiers, such as desc or cache_on_disk_if. That could potentially make it more reasonable to get rid of the _description_fns and _cache_on_disk_if_fns modules, as proposed in #153065.

r? nnethercote

Zalathar · 2026-03-03T03:43:11Z

The named-modifier style is based on a suggestion from @Zoxc.

Zalathar · 2026-03-03T03:44:20Z

I don't anticipate perf effects, but for something like this it's good to check.

@bors try @rust-timer queue

Make `rustc_with_all_queries!` pass query modifiers as named values

Zalathar · 2026-03-03T03:57:59Z

At first I wasn't sure whether the named-values approach was worthwhile, but it was the ability to start using #[cfg(..)] that really sold me.

nnethercote · 2026-03-03T04:01:40Z

The named-modifier style is based on a suggestion from @Zoxc.

Who got it from Claude Opus!

Zalathar · 2026-03-03T04:04:58Z

compiler/rustc_query_impl/src/dep_kind_vtables.rs

+                    $is_anon,
+                    $is_cache_on_disk,
+                    $is_eval_always,


If we change these to use named constants defined by one of the other callback macros, we can remove the modifier-parsing block from dep_kind_vtables.

I currently haven't made that change because I'm not sure whether it's worthwhile.

Sorry, I don't understand. Can you expand?

If we make the big macro in rustc_query_impl::plumbing define constants like:

pub(crate) const ANON: bool = $anon;

Then instead of writing $anon here, we would write $crate::query_impl::$name::ANON.

The benefit of doing so would be that the macro in dep_kind_vtables would no longer need to parse the modifier list at all, reducing the number of places that need to be kept in sync with the modifier list syntax.

But that benefit disappears if this code ends up needing to parse the modifier list for other reasons anyway.

Zalathar · 2026-03-03T04:17:37Z

One of the downsides of #[cfg(..)] is that it's harder to select one of two different expressions, which is relevant when initializing QueryVTable fields. I ended up duplicating the entire field-assignment in several places, because my attempts to select an expression in a single block ended up looking messy due to weaker type inference.

nnethercote

I like the approach on the modifier passing. Verbose in places, but simple and explicit. Eliminating all those if_* macros is really nice. A few comments below.

I honestly don't like the first commit. More qualifiers, more use items, more pub(crate) visibilities, and now the query code is spread across two files which makes navigability worse. All just to move ~100 lines out of a ~500 line module. We clearly have different senses of taste on this. (Genuine question: if you introduced the module but as a mod modifiers { ... } within query.rs, does that satisfy you? I'm trying to understand the motivation.) I won't block the PR on this but I will ask you to think twice about whether it's worthwhile.

View changes since this review

compiler/rustc_macros/src/query/modifiers.rs

compiler/rustc_middle/src/query/inner.rs

compiler/rustc_middle/src/query/plumbing.rs

nnethercote · 2026-03-03T03:59:45Z

compiler/rustc_middle/src/query/plumbing.rs

+                pub type ProvidedValue<'tcx> =
+                    <Value<'tcx> as $crate::query::arena_cached::ArenaCached<'tcx>>::Provided;
+                /// Type returned from query providers and loaded from disk-cache.
+                #[cfg(not($is_arena_cache))]


The duplication of the doc comments and the cfg attribute name is unfortunate, but I don't see how to avoid it.

I guess the attribute name repetition could be avoided with the cfg_if crate, but it's probably not worth it.

compiler/rustc_middle/src/query/plumbing.rs

Zalathar · 2026-03-03T05:04:18Z

Addressed all of the feedback so far: (diff).

nnethercote · 2026-03-03T05:16:03Z

Now I'm second-guessing the return type alias change. (Sorry!) A little bit of duplication might be worth it to avoid the unnecessary args. What do you think?

nnethercote · 2026-03-03T05:17:02Z

BTW, the use of tidy-alphabetical to order the modifiers is great.

Zalathar · 2026-03-03T05:28:26Z

Now I'm second-guessing the return type alias change. (Sorry!) A little bit of duplication might be worth it to avoid the unnecessary args. What do you think?

I’m not sure which one I ultimately prefer, but right now I’m leaning towards the type alias because it’s closer to the status quo.

This PR lays the foundation for some interesting follow-ups, so it’s fine to leave some potential improvements for future work in other PRs.

rust-bors · 2026-03-03T05:53:33Z

☀️ Try build successful (CI)
Build commit: a360f53 (a360f5337f07d909bc5a859cd85ee0bf1e07bf77, parent: ec818fda361ca216eb186f5cf45131bd9c776bb4)

rust-timer · 2026-03-03T06:33:39Z

Finished benchmarking commit (a360f53): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.1%, 0.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.6%	[-3.1%, -2.2%]	2
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -6.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-6.7%	[-6.7%, -6.7%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 480.002s -> 478.954s (-0.22%)
Artifact size: 394.97 MiB -> 396.99 MiB (0.51%)

nnethercote · 2026-03-03T08:06:52Z

compiler/rustc_macros/src/query.rs

    block: Block,
 }

+/// See `rustc_middle::query::modifiers` for documentation of each query modifier.


I had been thinking about this very change myself, thanks for doing it.

Though there is still the comments at the top of compiler/rustc_middle/src/queries.rs.

nnethercote · 2026-03-03T08:10:10Z

compiler/rustc_macros/src/query.rs

-            [#modifiers_stream]
-            fn #name(#key_ty) #return_ty,
+            fn #name(#key_ty) #return_ty
+            { #modifiers_stream }


Was there any particular motivation behind moving the modifiers after the fn signature?

It's closer to the actual syntax used in rustc_middle::queries (and processed by the proc-macro), which is hopefully a bit more intuitive for people who don't know all the horrible details of how these macros work

It puts the signature near the top, which feels a bit more natural when looking at pattern at the pattern at the top of the callback macros

nnethercote · 2026-03-03T08:12:58Z

@bors r+

rust-bors · 2026-03-03T08:13:01Z

📌 Commit 083b5db has been approved by nnethercote

It is now in the queue for this repository.

JonathanBrouwer · 2026-03-03T08:21:13Z

@bors rollup
I think this can be rolled up, perf results are noise

…rcote Make `rustc_with_all_queries!` pass query modifiers as named values This PR is a bold overhaul of how the proc-macro in `rustc_macros::query` passes query modifiers to the callback macros in `rustc_middle` and `rustc_query_impl`. The existing approach passes modifiers as a list that looks like `[(arena_cache), (no_hash)]`. That style requires a family of helper macros (`if_arena_cache!`, `if_no_hash!`) to check for modifiers when consuming the query list. This PR changes the proc-macro to instead pass modifiers like this: ```text { anon: false, arena_cache: true, cache_on_disk: false, ... } ``` This style allows each of the callback macros to deconstruct the modifier list in a relatively straightforward way, by binding the true/false literals to variables like `$arena_cache:literal`. One of the big advantages of this style is that we can write things like `#[cfg($arena_cache)]` and `#[cfg(not($arena_cache))]` to select blocks of code, eliminating the need for the `if_arena_cache!` family of helper macros. In follow-up PRs, we can also try to take advantage of the new modifier style to pass richer information for some modifiers, such as `desc` or `cache_on_disk_if`. That could potentially make it more reasonable to get rid of the `_description_fns` and `_cache_on_disk_if_fns` modules, as proposed in rust-lang#153065. r? nnethercote

petrochenkov · 2026-03-03T10:58:35Z

One of the downsides of #[cfg(..)] is that it's harder to select one of two different expressions, which is relevant when initializing QueryVTable fields. I ended up duplicating the entire field-assignment in several places, because my attempts to select an expression in a single block ended up looking messy due to weaker type inference.

Maybe using cfg! instead of #[cfg] will help.
(I usually prefer it in general, unless #[cfg] is strictly required to satisfy name resolution, or type checking, or similar.)

Zalathar · 2026-03-03T11:07:49Z

Maybe using cfg! instead of #[cfg] will help. (I usually prefer it in general, unless #[cfg] is strictly required to satisfy name resolution, or type checking, or similar.)

In this context, it’s often the case that one of the arms wouldn’t type-check, which is why we need to be able to eliminate it entirely before that happens.

But I do agree that avoiding conditional compilation is preferable when reasonably possible.

…rcote Make `rustc_with_all_queries!` pass query modifiers as named values This PR is a bold overhaul of how the proc-macro in `rustc_macros::query` passes query modifiers to the callback macros in `rustc_middle` and `rustc_query_impl`. The existing approach passes modifiers as a list that looks like `[(arena_cache), (no_hash)]`. That style requires a family of helper macros (`if_arena_cache!`, `if_no_hash!`) to check for modifiers when consuming the query list. This PR changes the proc-macro to instead pass modifiers like this: ```text { anon: false, arena_cache: true, cache_on_disk: false, ... } ``` This style allows each of the callback macros to deconstruct the modifier list in a relatively straightforward way, by binding the true/false literals to variables like `$arena_cache:literal`. One of the big advantages of this style is that we can write things like `#[cfg($arena_cache)]` and `#[cfg(not($arena_cache))]` to select blocks of code, eliminating the need for the `if_arena_cache!` family of helper macros. In follow-up PRs, we can also try to take advantage of the new modifier style to pass richer information for some modifiers, such as `desc` or `cache_on_disk_if`. That could potentially make it more reasonable to get rid of the `_description_fns` and `_cache_on_disk_if_fns` modules, as proposed in rust-lang#153065. r? nnethercote

…uwer Rollup of 3 pull requests Successful merges: - #153336 (stdarch subtree update) - #153285 (Update call-llvm-intrinsics test for Rust 1.94.0 IR) - #153326 (Make `rustc_with_all_queries!` pass query modifiers as named values)

…rcote Make `rustc_with_all_queries!` pass query modifiers as named values This PR is a bold overhaul of how the proc-macro in `rustc_macros::query` passes query modifiers to the callback macros in `rustc_middle` and `rustc_query_impl`. The existing approach passes modifiers as a list that looks like `[(arena_cache), (no_hash)]`. That style requires a family of helper macros (`if_arena_cache!`, `if_no_hash!`) to check for modifiers when consuming the query list. This PR changes the proc-macro to instead pass modifiers like this: ```text { anon: false, arena_cache: true, cache_on_disk: false, ... } ``` This style allows each of the callback macros to deconstruct the modifier list in a relatively straightforward way, by binding the true/false literals to variables like `$arena_cache:literal`. One of the big advantages of this style is that we can write things like `#[cfg($arena_cache)]` and `#[cfg(not($arena_cache))]` to select blocks of code, eliminating the need for the `if_arena_cache!` family of helper macros. In follow-up PRs, we can also try to take advantage of the new modifier style to pass richer information for some modifiers, such as `desc` or `cache_on_disk_if`. That could potentially make it more reasonable to get rid of the `_description_fns` and `_cache_on_disk_if_fns` modules, as proposed in rust-lang#153065. r? nnethercote

…uwer Rollup of 6 pull requests Successful merges: - #153336 (stdarch subtree update) - #152943 (Parse `impl` restrictions) - #153184 (Replace CodegenResults with CompiledModules) - #153285 (Update call-llvm-intrinsics test for Rust 1.94.0 IR) - #153319 (Comments and docs: add missing periods to "ie.") - #153326 (Make `rustc_with_all_queries!` pass query modifiers as named values)

rustbot assigned nnethercote Mar 3, 2026

This comment has been minimized.

Sign in to view

rust-bors bot pushed a commit that referenced this pull request Mar 3, 2026

Auto merge of #153326 - Zalathar:query-modifiers, r=<try>

a360f53

Make `rustc_with_all_queries!` pass query modifiers as named values

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 3, 2026

Zalathar commented Mar 3, 2026

View reviewed changes

nnethercote reviewed Mar 3, 2026

View reviewed changes

nnethercote mentioned this pull request Mar 3, 2026

Remove impl QueryVTable #153323

Open

Zalathar force-pushed the query-modifiers branch from ba86d8a to bb45c0c Compare March 3, 2026 04:57

Zalathar force-pushed the query-modifiers branch from bb45c0c to 0b96d85 Compare March 3, 2026 05:05

Make rustc_with_all_queries! pass query modifiers as named values

083b5db

Zalathar force-pushed the query-modifiers branch from 0b96d85 to 083b5db Compare March 3, 2026 05:44

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 3, 2026

nnethercote reviewed Mar 3, 2026

View reviewed changes

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 Mar 3, 2026

JonathanBrouwer mentioned this pull request Mar 3, 2026

Rollup of 2 pull requests #153339

Closed

JonathanBrouwer mentioned this pull request Mar 3, 2026

Rollup of 3 pull requests #153342

Closed

JonathanBrouwer mentioned this pull request Mar 3, 2026

Rollup of 6 pull requests #153344

Open

Uh oh!

Conversation

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

Uh oh!

Uh oh!

Zalathar commented Mar 3, 2026

Uh oh!

Zalathar commented Mar 3, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Zalathar commented Mar 3, 2026

Uh oh!

nnethercote commented Mar 3, 2026

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!

Zalathar commented Mar 3, 2026

Uh oh!

nnethercote left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Uh oh!

Zalathar commented Mar 3, 2026

Uh oh!

nnethercote commented Mar 3, 2026

Uh oh!

nnethercote commented Mar 3, 2026

Uh oh!

Zalathar commented Mar 3, 2026

Uh oh!

rust-bors bot commented Mar 3, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Mar 3, 2026

Overall result: ❌✅ regressions and improvements - no action needed

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!

nnethercote commented Mar 3, 2026

Uh oh!

rust-bors bot commented Mar 3, 2026

Uh oh!

JonathanBrouwer commented Mar 3, 2026

Uh oh!

petrochenkov commented Mar 3, 2026

Uh oh!

Zalathar commented Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Zalathar commented Mar 3, 2026 •

edited

Loading

nnethercote left a comment •

edited by rustbot

Loading