Skip to content

Allow implementing Hash with derived PartialEq (derive_hash_xor_eq)#10184

Merged
bors merged 3 commits into
rust-lang:masterfrom
robertbastian:master
Jan 12, 2023
Merged

Allow implementing Hash with derived PartialEq (derive_hash_xor_eq)#10184
bors merged 3 commits into
rust-lang:masterfrom
robertbastian:master

Conversation

@robertbastian

@robertbastian robertbastian commented Jan 9, 2023

Copy link
Copy Markdown
Contributor

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

rustbot commented Jan 9, 2023

Copy link
Copy Markdown
Collaborator

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

Jarcho commented Jan 10, 2023

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

Done

@xFrednet xFrednet left a comment

Copy link
Copy Markdown
Contributor

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 :)

Comment thread tests/ui/derived_hash_with_manual_eq.rs
Comment thread tests/ui/rename.rs
@xFrednet

Copy link
Copy Markdown
Contributor

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

@bors r+

@bors

bors commented Jan 12, 2023

Copy link
Copy Markdown
Contributor

📌 Commit 9206332 has been approved by xFrednet

It is now in the queue for this repository.

@bors

bors commented Jan 12, 2023

Copy link
Copy Markdown
Contributor

⌛ Testing commit 9206332 with merge decaba9...

@bors

bors commented Jan 12, 2023

Copy link
Copy Markdown
Contributor

☀️ 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