Clean up query macros for cache_on_disk_if
#151943
Conversation
rustbot
added
A-query-system
| #[derive(Default)] | ||
| struct HelperTokenStreams { | ||
| descs_stream: proc_macro2::TokenStream, | ||
| cache_on_disk_if_fns_stream: proc_macro2::TokenStream, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Zalathar
force-pushed
the
cache-on-disk
branch
from
b2d5760 to
6b38801
Compare
This comment has been minimized.
This comment has been minimized.
Zalathar
force-pushed
the
cache-on-disk
branch
2 times, most recently
from
3820de3 to
d24c1a0
Compare
|
|
||
| // 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? |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
This comment has been minimized.
This comment has been minimized.
Zalathar
force-pushed
the
cache-on-disk
branch
from
d24c1a0 to
c59d7f4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Zalathar
force-pushed
the
cache-on-disk
branch
from
c59d7f4 to
892665b
Compare
|
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. |
|
Hi, sorry for taking a while to get to this. I took a while to read this and while I am fairly confident that the logic hasn't changed much, I am not familiar enough with this part of the compiler to approve the change. @rustbot reroll |
6 participants
This PR aims to make the macros for dealing with
cache_on_disk_ifa bit easier to read and work with.There should be no change to compiler behaviour.