Skip to content

Clean up query macros for cache_on_disk_if#151943

Open
Zalathar wants to merge 1 commit intorust-lang:mainfrom
Zalathar:cache-on-disk
Open

Clean up query macros for cache_on_disk_if#151943
Zalathar wants to merge 1 commit intorust-lang:mainfrom
Zalathar:cache-on-disk

Conversation

@Zalathar
Copy link
Member

@Zalathar Zalathar commented Feb 1, 2026

This PR aims to make the macros for dealing with cache_on_disk_if a bit easier to read and work with.

There should be no change to compiler behaviour.

@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 1, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 1, 2026

r? @tiif

rustbot has assigned @tiif.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

#[derive(Default)]
struct HelperTokenStreams {
descs_stream: proc_macro2::TokenStream,
cache_on_disk_if_fns_stream: proc_macro2::TokenStream,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there multiple functions/descriptions, or one function/description?
There's some naming inconsistency, query_description_stream below says that it's one, but "descs" says that there are multiple.

Copy link
Member Author

Choose a reason for hiding this comment

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

Each stream contains multiple descriptions or multiple functions (0-1 of each per query).

The existing name query_description_stream is misleading; I'll update this PR to change it.

@rustbot

This comment has been minimized.

@Zalathar Zalathar force-pushed the cache-on-disk branch 2 times, most recently from 3820de3 to d24c1a0 Compare February 3, 2026 01:26

// FIXME(Zalathar): Instead of declaring these functions directly, can
// we put them in a macro and then expand that macro downstream in
// `rustc_query_impl`, where the functions are actually used?
Copy link
Contributor

Choose a reason for hiding this comment

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

We kind of want to keep rustc_query_impl small (for compile-time reasons), so having it point to functions (which won't be inlined) in rustc_middle isn't terrible.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking that we also want to keep rustc_middle small (since it's a bottleneck for most other compiler crates), though I don't have a strong sense of how shuffling things between the two crates affects overall build times (especially with metadata pipelining).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but there's other way to reduce rustc_middle (particularly if other crates can also define queries). rustc_query_impl will mostly keep growing as we add queries.

Copy link
Member Author

Choose a reason for hiding this comment

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

The primary purpose of rustc_query_impl is to allow rustc_middle to be smaller.

If we wanted to improve query-impl build times by enabling more parallelism, we wouldn't do so by moving code into middle; we'd move it into an intermediate crate between the two.

So I don't think keeping query-impl small is a reason to not move code from middle to query-impl.

@rust-bors

This comment has been minimized.

@rustbot

This comment has been minimized.

@rust-bors

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Feb 5, 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.

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.

5 participants