Navigation Menu

Skip to content
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

Allow implementing Hash with derived PartialEq (derive_hash_xor_eq) #10184

Merged
merged 3 commits into from Jan 12, 2023

Conversation

robertbastian
Copy link
Contributor

@robertbastian robertbastian commented Jan 9, 2023

This is a common pattern and is totally allowed by the Hash trait.

Fixes #2627

changelog: Move: Renamed derive_hash_xor_eq to [derived_hash_with_manual_eq]
#10184
changelog: Enhancement: [derived_hash_with_manual_eq]: Now allows #[derive(PartialEq)] with custom Hash implementations
#10184

@rustbot
Copy link
Collaborator

rustbot commented Jan 9, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @xFrednet (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 9, 2023
@robertbastian robertbastian changed the title Allow implementingHash with derived PartialEq (derive_hash_xor_eq Allow implementing Hash with derived PartialEq (derive_hash_xor_eq Jan 9, 2023
@Jarcho
Copy link
Contributor

Jarcho commented Jan 10, 2023

The lint name should be changed while you're at it. Something like derived_hash_with_manual_eq. cargo dev rename_lint can be used to make the change.

@robertbastian
Copy link
Contributor Author

Done

Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

The logic looks good to me. Two small nits and then this should be ready for master.

@Jarcho Thank you for the suggestion to rename the lint, I probably would have missed that :)

tests/ui/derived_hash_with_manual_eq.rs Show resolved Hide resolved
tests/ui/rename.rs Show resolved Hide resolved
@xFrednet
Copy link
Member

Looks good to me, thank you for the update :)

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 12, 2023

📌 Commit 9206332 has been approved by xFrednet

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jan 12, 2023

⌛ Testing commit 9206332 with merge decaba9...

@bors
Copy link
Collaborator

bors commented Jan 12, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing decaba9 to master...

@bors bors merged commit decaba9 into rust-lang:master Jan 12, 2023
@robertbastian robertbastian changed the title Allow implementing Hash with derived PartialEq (derive_hash_xor_eq Allow implementing Hash with derived PartialEq (derive_hash_xor_eq) Jan 12, 2023
a4lg added a commit to a4lg/ffuzzy that referenced this pull request May 8, 2023
`clippy::derive_hash_xor_eq` is the problem.  While it was default error
on our current MSRV (1.56), it is now renamed and the meaning of the lint
has changed (so that our code is no longer affected by the renamed lint).

Related (rust-clippy):
Allow implementing `Hash` with derived `PartialEq` (`derive_hash_xor_eq`)
<rust-lang/rust-clippy#10184>

As a workaround, it disables `renamed_and_removed_lints` lint unless in
the maintaince mode.
a4lg added a commit to a4lg/ffuzzy that referenced this pull request May 9, 2023
`clippy::derive_hash_xor_eq` is the problem.  While it was default error
on our current MSRV (1.56), it is now renamed and the meaning of the lint
has changed (so that our code is no longer affected by the renamed lint).

Related (rust-clippy):
Allow implementing `Hash` with derived `PartialEq` (`derive_hash_xor_eq`)
<rust-lang/rust-clippy#10184>

As a workaround, it disables `renamed_and_removed_lints` lint unless in
the maintaince mode.

This is a backport from the version 0.2.x development branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

derive_hash_xor_eq deny deriving PartialEq when implementing Hash manually
5 participants