Skip to content

Conversation

@nnethercote
Copy link
Contributor

Some readability improvements.

r? @Zalathar

It's just an unnecessary level of additional nesting. rust-lang#151893 did
the same thing for the `define_callbacks` macro.
Specifically `define_callbacks` and `define_queries`. These are big,
complicated macros. If the top-level `$(..)*` repetitions are obvious it
helps enormously with readability.
It's a unit type used for trait stuff, it doesn't need a lifetime.
(Unless there's some weird variance thing I'm unaware of, but it
compiles fine, so I think it should be ok.)
@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 10, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 10, 2026

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@Zalathar
Copy link
Member

I find that the query_impl module serves two useful purposes:

  • It groups the per-query submodules so that they aren’t expanded directly into the crate root.
  • It provides a named “anchor” that indicates when code in one part of the macro is using definitions from another part of the macro; seeing query_impl::$name in a path is much more helpful than just seeing $name.

In #151893 it’s less necessary because $crate::queries ends up serving roughly those same roles.

@Zalathar
Copy link
Member

I agree with giving $( … )* its own indentation level in the general case; it’s nicer than having them squished away and invisible.

The exception I sometimes make is for relatively simple struct field declarations and struct/slice initialisation. If the whole thing can clearly and comfortably fit on one line, then I tend to prefer this more compact style:

Struct {
    $( $name: FieldType<$name::Thing>, )*
}

(Take this as a suggestion rather than a request for changes; use it if you agree it’s an improvement in particular places, but otherwise the fully-indented style is fine.)

pub(crate) struct QueryType<'tcx> {
data: PhantomData<&'tcx ()>
}
pub(crate) struct QueryType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: I intend to dismantle and remove QueryDispatcherUnerased (and therefore QueryType) at some point, but no harm in simplifying this now.

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 10, 2026

☔ The latest upstream changes (presumably #152437) made this pull request unmergeable. Please resolve the merge conflicts.

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) 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants