Skip to content

define_queries! module tweaks#153588

Closed
nnethercote wants to merge 3 commits intorust-lang:mainfrom
nnethercote:define_queries-module-tweaks
Closed

define_queries! module tweaks#153588
nnethercote wants to merge 3 commits intorust-lang:mainfrom
nnethercote:define_queries-module-tweaks

Conversation

@nnethercote
Copy link
Contributor

@nnethercote nnethercote commented Mar 9, 2026

Minor module structure tweaks, trying to make define_queries! nicer and more like define_callbacks!. Details in individual commits.

r? @Zalathar

@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. labels Mar 9, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 9, 2026

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2026
@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 9, 2026

@Zalathar: not sure if you'll like these, but I figured it's worth a try.

Zalathar
Zalathar reviewed Mar 9, 2026
View reviewed changes
@nnethercote nnethercote force-pushed the define_queries-module-tweaks branch from b045589 to ca85c9b Compare March 9, 2026 05:49
@rust-bors

This comment has been minimized.

Sign in to view
@nnethercote nnethercote force-pushed the define_queries-module-tweaks branch from ca85c9b to 0b115aa Compare March 11, 2026 10:28
@rustbot

This comment has been minimized.

Sign in to view
@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 11, 2026

I rebased.

@Zalathar
Copy link
Member

Zalathar commented Mar 11, 2026

I was a little uneasy about defining other items in the same module as per-query submodules, but #153685 will end up removing all of those other items except make_query_vtables (and for_each_query_vtable!), so maybe that's fine.

@rust-bors

This comment has been minimized.

Sign in to view
@nnethercote
Remove use rustc_middle::queries::self from rustc_query_impl.
8abfc7c 
This use item lets `rustc_query_impl` use `queries::foo` to refer to
things from `rustc_middle`, which is confusing, especially within
`define_queries!`.

This commit removes it, requiring more explicit qualification of
imported things. It also removes some unnecessary leading `::`
qualifiers from `rustc_middle` mentions for consistency, conciseness,
and to spare the poor reader's eyes.
@nnethercote
Put more things into mod query_impl.
286acc2 
`define_queries!` currently outputs some things (including one `mod
$name` per query) into a module `query_impl`, and some things into the
crate root. This commit moves the latter things into `query_impl`. This
is (a) nicer, and (b) more closely matches what `define_callbacks!`
does.
@nnethercote
Move location of mod query_impl.
5d0eea5 
This:
- reduces the amount of stuff inside `define_queries!`
- makes `define_queries!` look more like `define_callbacks!`

Also, I want to improve the formatting of `define_queries!` and this
will avoid one level of indentation.
@nnethercote nnethercote force-pushed the define_queries-module-tweaks branch from 0b115aa to 5d0eea5 Compare March 12, 2026 02:15
@rustbot
Copy link
Collaborator

rustbot commented Mar 12, 2026

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
Copy link
Contributor Author

nnethercote commented Mar 12, 2026

I have rebased over #153685, see what you think now.

@Zalathar
Copy link
Member

Zalathar commented Mar 12, 2026 •
edited
Loading

Something I found while playing around with this branch (and also on main) is that messing up the imports in these macro-generated modules can sometimes result in the bootstrap compiler spinning for a very long time before failing, on the order of multiple minutes instead of the usual ~20 second successful compile time.

That's not directly actionable, but it does make me want to find a style of imports that is less vulnerable to that failure mode.

It also makes me wonder whether the common imports needed by the query_impl module should be declared out in lib.rs (where they can be auto-sorted), or in plumbing.rs (where they are closer to the macro-generated code that uses them). Or perhaps we should be using even more fully-qualified names in the macro, to minimize or eliminate the need for blanket imports.

@Zalathar Zalathar mentioned this pull request Mar 12, 2026
Merged
@Zalathar
Copy link
Member

Zalathar commented Mar 12, 2026

While thinking about the implications of my previous comment, I came up with the idea for #153760, which I think is a promising alternative to some of the changes in this PR.

@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 13, 2026

Superseded by #153760.

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 13, 2026
Zalathar added a commit to Zalathar/rust that referenced this pull request Mar 14, 2026
@Zalathar
Rollup merge of rust-lang#153760 - Zalathar:query-impl, r=nnethercote
6d7ea71 
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
Zalathar added a commit to Zalathar/rust that referenced this pull request Mar 14, 2026
@Zalathar
Rollup merge of rust-lang#153760 - Zalathar:query-impl, r=nnethercote
e0bb94e 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Zalathar Zalathar Zalathar left review comments

Assignees

@Zalathar Zalathar

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nnethercote @rustbot @Zalathar