Skip to content

Clean up QueryVTable::hash_result into hash_value_fn#153209

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Zalathar:hash-value-fn
Mar 1, 2026
Merged

Clean up QueryVTable::hash_result into hash_value_fn#153209
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Zalathar:hash-value-fn

Conversation

@Zalathar
Copy link
Member

@Zalathar Zalathar commented Feb 28, 2026

This PR:

  • Renames the query vtable field hash_result to hash_value_fn
  • Removes the unhelpful HashResult type alias, which was hiding an important layer of Option
  • Replaces the cryptic hash_result! helper macro with a more straightforward if_no_hash! helper, in line with other modifier-checking macros
  • Renames a few identifiers to refer to a query's return value as value

There should be no change to compiler behaviour.

r? nnethercote (or compiler)

@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 28, 2026
[$($modifiers)*]
None
(Some($crate::plumbing::hash_erased_value::<queries::$name::Value<'tcx>>))
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just use an expr block here and avoid creating hash_erased_value (and its complicated signature). Less code and easier to read, IMO, because you don't need to look elsewhere.

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 think there are advantages to having the separate function with its explicit signature, but it's a close call and the inline closure is reasonable, so I've changed to the inline closure.

@nnethercote
Copy link
Contributor

The commit message seems out of date, it needs some more details.

r=me with that fixed and the nits fixed.

@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 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.

This commit:
- Renames the query vtable field `hash_result` to `hash_value_fn`
- Removes the unhelpful `HashResult` type alias, which was hiding an
  important layer of `Option`
- Replaces the cryptic `hash_result!` helper macro with a more straightforward
  `if_no_hash!` helper, in line with other modifier-checking macros
- Renames a few identifiers to refer to a query's return value as `value`
@Zalathar
Copy link
Member Author

Zalathar commented Mar 1, 2026

The commit message seems out of date, it needs some more details.

I don't understand this request; do you mean that I should copy the PR description into the (previously empty) commit message? If so, I've just done that.

@nnethercote
Copy link
Contributor

Thanks!

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 1, 2026

📌 Commit 2c057bb has been approved by nnethercote

It is now in the queue for this repository.

@rust-bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2026
rust-bors bot pushed a commit that referenced this pull request Mar 1, 2026
Rollup of 5 pull requests

Successful merges:

 - #152042 (Suggest async block instead of async closure when possible)
 - #152949 (Introduce --ci flag in tidy)
 - #152655 (Disable debug_assert_not_in_new_nodes for multiple threads)
 - #153209 (Clean up `QueryVTable::hash_result` into `hash_value_fn`)
 - #153229 (rustfmt: add test for field representing type builtin syntax)
rust-bors bot pushed a commit that referenced this pull request Mar 1, 2026
Rollup of 5 pull requests

Successful merges:

 - #152042 (Suggest async block instead of async closure when possible)
 - #152949 (Introduce --ci flag in tidy)
 - #152655 (Disable debug_assert_not_in_new_nodes for multiple threads)
 - #153209 (Clean up `QueryVTable::hash_result` into `hash_value_fn`)
 - #153229 (rustfmt: add test for field representing type builtin syntax)
@rust-bors rust-bors bot merged commit 27503c4 into rust-lang:main Mar 1, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 1, 2026
rust-timer added a commit that referenced this pull request Mar 1, 2026
Rollup merge of #153209 - Zalathar:hash-value-fn, r=nnethercote

Clean up `QueryVTable::hash_result` into `hash_value_fn`

This PR:
- Renames the query vtable field `hash_result` to `hash_value_fn`
- Removes the unhelpful `HashResult` type alias, which was hiding an important layer of `Option`
- Replaces the cryptic `hash_result!` helper macro with a more straightforward `if_no_hash!` helper, in line with other modifier-checking macros
- Renames a few identifiers to refer to a query's return value as `value`

There should be no change to compiler behaviour.

r? nnethercote (or compiler)
@Zalathar Zalathar deleted the hash-value-fn branch March 1, 2026 22:41
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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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