Query key cleanups by nnethercote · Pull Request #153112 · rust-lang/rust

nnethercote · 2026-02-25T23:53:36Z

The first three commits are simple. The last two are a bit more opinionated, see what you think.

r? @Zalathar

rustbot · 2026-02-25T23:53:40Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustbot · 2026-02-25T23:53:43Z

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

Zalathar · 2026-02-26T03:07:16Z

compiler/rustc_middle/src/query/plumbing.rs

                }

-                pub type Storage<'tcx> =
+                pub type QueryCache<'tcx> =


I definitely agree with renaming Storage.

Because this is macro-heavy code, I place a very high priority on having distinct names for things, both to make searching easier and less noisy, and to make it easier to keep things distinct in my mind without IDE-hover assistance.

For that reason I would prefer something unique, even if it's less elegant. Thankfully we only need to see it in a few places.

Perhaps QueryCacheImpl? I don't love it, but it's unique, and feels suitably connected to the QueryCache trait without actually colliding.

Zalathar · 2026-02-26T03:09:17Z

compiler/rustc_middle/src/query/keys.rs

 #[derive(Copy, Clone, Debug)]
 pub struct LocalCrate;

+pub trait QueryKeyBounds = Copy + Debug + Eq + Hash + for<'a> HashStable<StableHashingContext<'a>>;


Remark: This is a nice little trick for repetitive bounds.

Zalathar · 2026-02-26T03:16:30Z

I like all of this, including the last two.

My only quibble is wanting to find a unique replacement for Storage→QueryCache, because having a macro-defined type alias collide with a relevant trait name seems like it's going to be a real source of confusion.

nnethercote · 2026-02-26T04:23:19Z

We could just use Cache for the type name to replace Storage? That aligns with the very short local typedefs Key and Value. There is the QueryKey::Cache associated type but I don't think that would cause confusion in the same way as having the same name as a trait?

Zalathar · 2026-02-26T04:49:10Z

We could just use Cache for the type name to replace Storage? That aligns with the very short local typedefs Key and Value. There is the QueryKey::Cache associated type but I don't think that would cause confusion in the same way as having the same name as a trait?

Yeah, that sounds like a reasonable choice.

It's less searchable than QueryCacheImpl, but it's good enough to resolve my objection to QueryCache, and it's better than Storage. If I hypothetically regret the choice, it's easy to change again later.

r=me with Cache.

nnethercote · 2026-02-26T05:22:42Z

I changed to Cache. I don't think this needs a perf run, there are no changes that should change any generated code.

@bors r=Zalathar rollup

rust-bors · 2026-02-26T05:22:45Z

📌 Commit ef7aa68 has been approved by Zalathar

It is now in the queue for this repository.

It's currently in `dep_node.rs`, along with a blanket impl. Specific impls are in `dep_node_key.rs`. This commit moves the trait and the blanket impl into `dep_node_key.rs`, so everything is in one place.

Because `Key` is extremely generic and hard to search for. Also rename `LocalKey` and `AsLocalKey` similarly, for consistency.

We have two traits governing query keys, for no particular reason. This commit combines them.

Because `Storage` is a vague name that I've never liked.

rustbot · 2026-02-26T08:26:43Z

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 · 2026-02-26T08:27:10Z

I rebased.

@bors r+

rust-bors · 2026-02-26T08:27:13Z

📌 Commit 9ba101f has been approved by nnethercote

It is now in the queue for this repository.

@Zalathar

…thercote Query key cleanups The first three commits are simple. The last two are a bit more opinionated, see what you think. r? @Zalathar

…uwer Rollup of 7 pull requests Successful merges: - #153079 (Revert "Move aarch64-apple dist builder to dynamic llvm linking") - #152651 (Avoid duplicate `requirement` diag args in `RegionOriginNote`) - #152978 (Port `#[rustc_autodiff]` to the attribute parser infrastructure) - #153091 (Migration of `LintDiagnostic` - part 4) - #153112 (Query key cleanups) - #153118 (mailmap: add redddy) - #153120 (Clean up some code related to `QueryVTable::execute_query_fn`)

@Zalathar

Rollup merge of #153112 - nnethercote:query-key-stuff, r=nnethercote Query key cleanups The first three commits are simple. The last two are a bit more opinionated, see what you think. r? @Zalathar

JonathanBrouwer · 2026-02-26T21:35:13Z

@rust-timer build f07a639

rust-timer · 2026-02-27T02:02:37Z

Finished benchmarking commit (f07a639): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 0.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.0%	[2.8%, 3.2%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.8%	[-3.8%, -3.8%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[-3.8%, 3.2%]	3

Cycles

Results (secondary 3.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.1%	[3.1%, 3.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 480.132s -> 481.144s (0.21%)
Artifact size: 395.80 MiB -> 395.79 MiB (-0.00%)

rustbot assigned Zalathar Feb 25, 2026

Zalathar reviewed Feb 26, 2026

View reviewed changes

nnethercote force-pushed the query-key-stuff branch from eb743fd to ef7aa68 Compare February 26, 2026 05:21

This comment has been minimized.

Sign in to view

rust-bors bot removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 26, 2026

nnethercote added 5 commits February 26, 2026 19:09

Move trait DepNodeKey.

6ba5b1a

It's currently in `dep_node.rs`, along with a blanket impl. Specific impls are in `dep_node_key.rs`. This commit moves the trait and the blanket impl into `dep_node_key.rs`, so everything is in one place.

Rename trait Key as trait QueryKey`

fbea6dd

Because `Key` is extremely generic and hard to search for. Also rename `LocalKey` and `AsLocalKey` similarly, for consistency.

Remove unnecessary Clone bounds.

b284b04

Merge trait QueryCacheKey into trait QueryKey.

4499407

We have two traits governing query keys, for no particular reason. This commit combines them.

Rename Storage as Cache.

9ba101f

Because `Storage` is a vague name that I've never liked.

nnethercote force-pushed the query-key-stuff branch from ef7aa68 to 9ba101f Compare February 26, 2026 08:26

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 26, 2026

GuillaumeGomez mentioned this pull request Feb 26, 2026

Rollup of 5 pull requests #153123

Closed

JonathanBrouwer mentioned this pull request Feb 26, 2026

Rollup of 7 pull requests #153124

Merged

rust-bors bot merged commit 8386f94 into rust-lang:main Feb 26, 2026
11 checks passed

rustbot added this to the 1.95.0 milestone Feb 26, 2026

nnethercote deleted the query-key-stuff branch February 26, 2026 20:07

This comment has been minimized.

Sign in to view

Uh oh!

Conversation

nnethercote commented Feb 25, 2026

Uh oh!

rustbot commented Feb 25, 2026

Uh oh!

rustbot commented Feb 25, 2026

Uh oh!

Zalathar Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zalathar Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Zalathar commented Feb 26, 2026

Uh oh!

nnethercote commented Feb 26, 2026

Uh oh!

Zalathar commented Feb 26, 2026

Uh oh!

nnethercote commented Feb 26, 2026

Uh oh!

rust-bors bot commented Feb 26, 2026

Uh oh!

This comment has been minimized.

rustbot commented Feb 26, 2026

Uh oh!

nnethercote commented Feb 26, 2026

Uh oh!

rust-bors bot commented Feb 26, 2026

Uh oh!

Uh oh!

JonathanBrouwer commented Feb 26, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Feb 27, 2026

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Zalathar Feb 26, 2026 •

edited

Loading