Skip to content

Comments

Match query key types as $K:tt instead of $($K:tt)*#152833

Closed
Zalathar wants to merge 2 commits intorust-lang:mainfrom
Zalathar:into-query-param
Closed

Match query key types as $K:tt instead of $($K:tt)*#152833
Zalathar wants to merge 2 commits intorust-lang:mainfrom
Zalathar:into-query-param

Conversation

@Zalathar
Copy link
Member

When query-plumbing macros match on a query key type, they currently have to perform a clunky match on $($K:tt)*, instead of the more obvious $K:ty.

Switching to $K:ty is currently not feasible, because that would prevent us from inspecting the specific key type in a helper macro that decides whether to replace $K with impl IntoQueryParam<$K>, if the key type happens to be DefId or LocalDefId.1

However, we can switch from $($K:tt)* to the simpler $K:tt if we modify the proc-macro to always emit key types surrounded in parentheses, guaranteeing that they always form a legal type that is also a single token tree.

(In some of the helper macros, this PR does switch to $K:ty because the inner token structure is not needed there.)

Background on impl IntoQueryParam<K>

Of the ~300 queries currently used by the compiler, about 100 have a key type of DefId or LocalDefId. For those two key types, the query plumbing macros have special logic to declare the corresponding query methods as taking impl IntoQueryParam<DefId> or impl IntoQueryParam<LocalDefId> instead. That allows callers to pass IDs that are actually strongly-typed wrappers for a subset of DefId/LocalDefId2, without having to explicitly convert at the call site.

There are arguments for or against doing that kind of trait-based argument conversion,3 but there are enough call sites currently relying on it that stopping would be a pretty substantial change.

Meanwhile, there are good reasons to not want to extend that practice to the other ~200 queries that don't need it. Auto-converting to DefId is common enough to arguably be worth it, whereas cluttering the real signatures of query methods that don't need auto-conversion adds real friction4 for almost no benefit.


The first commit is a general cleanup of rustc_macros::query that makes the subsequent changes more readable. It could be merged on its own if desired.


r? nnethercote

Footnotes

  1. In macro-rules macros, any input matched by something that isn't tt is subsequently treated as an opaque unit, whose sub-tokens cannot be inspected.

  2. Example conversions include LocalDefIdDefId, ModDefIdDefId, and LocalModDefIdLocalDefId.

  3. E.g. callers could just write .into() explicitly instead.

  4. Parameters involving conversion traits have less helpful type inference, less helpful error messages, and complicate otherwise-straightforward signatures as seen in documentation or IDE hover tips.

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 19, 2026
@rust-log-analyzer

This comment has been minimized.

The trick to making this work is to have the proc-macro surround every key type
with parentheses, so that it remains a valid type but also matches as a single
token tree.

Due to how macro fragment specifiers work, matching as `$K:ty` would prevent us
from inspecting the key type in `maybe_into_query_param!`.
@nnethercote
Copy link
Contributor

The current code implements the query-key-conversion stuff partly in the define_callbacks and query_helper_param_ty macros, and partly in the type system (IntoQueryParam).

#152779 eliminates the macro parts, so it is fully in the type system. And we end up using $K:ty, like you'd expect, and was my original goal when I started looking into this.

This PR spreads the complexity more: partly in the proc macro, partly in define_callbacks and maybe_into_query_param macros, and partly in the type system. All this and we end up with $K:tt, which is not as nice as $K:ty. Not an improvement, IMO.

--

About #152779's, you said "it clutters the signature of every single query method in a way that adds real friction." I don't understand what that "friction" is.

@Zalathar
Copy link
Member Author

About #152779's, you said "it clutters the signature of every single query method in a way that adds real friction." I don't understand what that "friction" is.

It wrecks type inference, it wrecks error messages, and it wrecks documentation.

That's not something I want to inflict on contributors for hundreds of methods that don't need it, especially if the only payoff is being able to write $K:ty instead of $K:tt in some query plumbing macros.

@nnethercote
Copy link
Contributor

The first commit is good. Just yesterday I was puzzling over the difference between key and arg, so I like the renaming and the explanation of the query anatomy.

The second commit increases coupling between the proc macro and the declarative macro in a way I don't like. I guess we'll just have to live with $($K:tt)*.

@Zalathar
Copy link
Member Author

@Zalathar Zalathar closed this Feb 19, 2026
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 19, 2026
@Zalathar Zalathar deleted the into-query-param branch February 19, 2026 11:58
jhpratt added a commit to jhpratt/rust that referenced this pull request Feb 20, 2026
…ercote

Clarify some variable names in the query proc-macro

These are some cleanups to the `rustc_macros::query`, extracted from rust-lang#152833.

r? nnethercote
jhpratt added a commit to jhpratt/rust that referenced this pull request Feb 20, 2026
…ercote

Clarify some variable names in the query proc-macro

These are some cleanups to the `rustc_macros::query`, extracted from rust-lang#152833.

r? nnethercote
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 20, 2026
…ercote

Clarify some variable names in the query proc-macro

These are some cleanups to the `rustc_macros::query`, extracted from rust-lang#152833.

r? nnethercote
rust-timer added a commit that referenced this pull request Feb 20, 2026
Rollup merge of #152846 - Zalathar:query-proc-macro, r=nnethercote

Clarify some variable names in the query proc-macro

These are some cleanups to the `rustc_macros::query`, extracted from #152833.

r? nnethercote
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants