Skip to content

Derive Hash for rustc_public types #145018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 8, 2025

Conversation

AlexanderPortland
Copy link
Contributor

This derives Hash for the rustc_public versions of Rvalue, Place, Span, and all the types they contain. As far as I can tell, the internal versions of all these types already implement Hash, so it would be helpful if the public versions implemented it too!

@rustbot
Copy link
Collaborator

rustbot commented Aug 6, 2025

r? @scottmcm

rustbot has assigned @scottmcm.
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

@rustbot rustbot added 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 Aug 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 6, 2025

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a

@@ -255,7 +255,17 @@ pub struct Placeholder<T> {
pub bound: T,
}

#[derive(Clone, Copy, PartialEq, Eq, Serialize)]
impl<T: std::hash::Hash> std::hash::Hash for Placeholder<T> {
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this also derived? If it's deriving Eq and PartialEq, it should almost certainly be deriving Hash too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, my bad! I didn't realize the derive macro could handle generics. Just fixed this now

@scottmcm scottmcm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2025
@AlexanderPortland
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 7, 2025
@scottmcm
Copy link
Member

scottmcm commented Aug 7, 2025

Sounds good to me; these things that are already derive(PartialEq, Eq) might as well derive(Hash) too.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 7, 2025

📌 Commit b9e6bd7 has been approved by scottmcm

It is now in the queue for this repository.

@bors bors 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 Aug 7, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 7, 2025
…h, r=scottmcm

Derive `Hash` for rustc_public types

This derives `Hash` for the rustc_public versions of `Rvalue`, `Place`, `Span`, and all the types they contain. As far as I can tell, the internal versions of all these types already implement `Hash`, so it would be helpful if the public versions implemented it too!
bors added a commit that referenced this pull request Aug 7, 2025
Rollup of 8 pull requests

Successful merges:

 - #144705 (compiler-builtins: plumb LSE support for aarch64 on linux/gnu when optimized-compiler-builtins not enabled)
 - #144857 (Port `#[allow_internal_unsafe]` to the new attribute system)
 - #144900 (Stabilize `unsigned_signed_diff` feature)
 - #144903 (Rename `begin_panic_handler` to `panic_handler`)
 - #144974 (compiler-builtins subtree update)
 - #145004 (Couple of minor cleanups)
 - #145007 (Fix build/doc/test of error index generator)
 - #145018 (Derive `Hash` for rustc_public types)

r? `@ghost`
`@rustbot` modify labels: rollup
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 7, 2025
…h, r=scottmcm

Derive `Hash` for rustc_public types

This derives `Hash` for the rustc_public versions of `Rvalue`, `Place`, `Span`, and all the types they contain. As far as I can tell, the internal versions of all these types already implement `Hash`, so it would be helpful if the public versions implemented it too!
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 8, 2025
…h, r=scottmcm

Derive `Hash` for rustc_public types

This derives `Hash` for the rustc_public versions of `Rvalue`, `Place`, `Span`, and all the types they contain. As far as I can tell, the internal versions of all these types already implement `Hash`, so it would be helpful if the public versions implemented it too!
bors added a commit that referenced this pull request Aug 8, 2025
Rollup of 9 pull requests

Successful merges:

 - #144705 (compiler-builtins: plumb LSE support for aarch64 on linux/gnu when optimized-compiler-builtins not enabled)
 - #144857 (Port `#[allow_internal_unsafe]` to the new attribute system)
 - #144900 (Stabilize `unsigned_signed_diff` feature)
 - #144903 (Rename `begin_panic_handler` to `panic_handler`)
 - #144974 (compiler-builtins subtree update)
 - #145007 (Fix build/doc/test of error index generator)
 - #145018 (Derive `Hash` for rustc_public types)
 - #145045 (doc(library): Fix Markdown in `Iterator::by_ref`)
 - #145046 (Fix doc comment of File::try_lock and File::try_lock_shared)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 41b6da1 into rust-lang:master Aug 8, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 8, 2025
rust-timer added a commit that referenced this pull request Aug 8, 2025
Rollup merge of #145018 - AlexanderPortland:rustc-public-hash, r=scottmcm

Derive `Hash` for rustc_public types

This derives `Hash` for the rustc_public versions of `Rvalue`, `Place`, `Span`, and all the types they contain. As far as I can tell, the internal versions of all these types already implement `Hash`, so it would be helpful if the public versions implemented it too!
github-actions bot pushed a commit to rust-lang/compiler-builtins that referenced this pull request Aug 9, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang/rust#144705 (compiler-builtins: plumb LSE support for aarch64 on linux/gnu when optimized-compiler-builtins not enabled)
 - rust-lang/rust#144857 (Port `#[allow_internal_unsafe]` to the new attribute system)
 - rust-lang/rust#144900 (Stabilize `unsigned_signed_diff` feature)
 - rust-lang/rust#144903 (Rename `begin_panic_handler` to `panic_handler`)
 - rust-lang/rust#144974 (compiler-builtins subtree update)
 - rust-lang/rust#145007 (Fix build/doc/test of error index generator)
 - rust-lang/rust#145018 (Derive `Hash` for rustc_public types)
 - rust-lang/rust#145045 (doc(library): Fix Markdown in `Iterator::by_ref`)
 - rust-lang/rust#145046 (Fix doc comment of File::try_lock and File::try_lock_shared)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Aug 9, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang/rust#144705 (compiler-builtins: plumb LSE support for aarch64 on linux/gnu when optimized-compiler-builtins not enabled)
 - rust-lang/rust#144857 (Port `#[allow_internal_unsafe]` to the new attribute system)
 - rust-lang/rust#144900 (Stabilize `unsigned_signed_diff` feature)
 - rust-lang/rust#144903 (Rename `begin_panic_handler` to `panic_handler`)
 - rust-lang/rust#144974 (compiler-builtins subtree update)
 - rust-lang/rust#145007 (Fix build/doc/test of error index generator)
 - rust-lang/rust#145018 (Derive `Hash` for rustc_public types)
 - rust-lang/rust#145045 (doc(library): Fix Markdown in `Iterator::by_ref`)
 - rust-lang/rust#145046 (Fix doc comment of File::try_lock and File::try_lock_shared)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants