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

derive_hash_xor_eq deny deriving PartialEq when implementing Hash manually #2627

Closed
Alvenix opened this issue Apr 4, 2018 · 4 comments · Fixed by #10184
Closed

derive_hash_xor_eq deny deriving PartialEq when implementing Hash manually #2627

Alvenix opened this issue Apr 4, 2018 · 4 comments · Fixed by #10184
Labels
A-documentation Area: Adding or improving documentation good-first-issue These issues are a good way to get started with Clippy

Comments

@Alvenix
Copy link

Alvenix commented Apr 4, 2018

According to here

derive_hash_xor_eq "Checks for deriving Hash but implementing PartialEq explicitly." however even doing the reverse is denied too.

I have tried the minimal example below and still faced this problem:

use std::hash::{Hash, Hasher};

#[derive(PartialEq)]
struct Duck {}

impl Hash for Duck {
    fn hash<H: Hasher>(&self, _: &mut H) {}
}
fn main() {}

Where running clippy results in this:


error: you are implementing `Hash` explicitly but have derived `PartialEq`
 --> src/main.rs:6:1
  |
6 | / impl Hash for Duck {
7 | |     fn hash<H: Hasher>(&self, _: &mut H) {}
8 | | }
  | |_^
  |
  = note: #[deny(derive_hash_xor_eq)] on by default
note: `PartialEq` implemented here
 --> src/main.rs:3:10
  |
3 | #[derive(PartialEq)]
  |          ^^^^^^^^^
  = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.191/index.html#derive_hash_xor_eq

error: aborting due to previous error

error: Could not compile `hashequal`.

I am using the latest rust nightly and the latest clippy. (mac os)

I have tried to look at the code and this seems intentional -well even the name seems to imply so- so maybe only the documentation should change?

@Alvenix Alvenix changed the title derive_hash_xor_eq deny deriving equality when implement Hash manually derive_hash_xor_eq deny deriving PartialEq when implementing Hash manually Apr 4, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Apr 4, 2018

Jup, just a docs issue. Good eye

@oli-obk oli-obk added good-first-issue These issues are a good way to get started with Clippy A-documentation Area: Adding or improving documentation labels Apr 4, 2018
@gilescope
Copy link

I can't think of a good example where overriding the Hash while having a derived PartialEq would cause a problem.
The other way around I can easily see, but this way round? If you're hashing any subset of the fields that would be fine. Wouldn't you have to try pretty hard to violate the agreement (E.g. referencing external mutable state or returning a random number)?

It's important that clippy stays as focused as possible. False positives detract.

@robertbastian
Copy link
Contributor

"Checks for deriving Hash but implementing PartialEq explicitly." is a valuable check, but I agree that the way this is currently implemented where it denies a custom Hash with a derived PartialEq doesn't make sense. There is a valid use case for expensive-to-hash fields for excluding them from the hash.

@Manishearth Manishearth reopened this Jul 11, 2022
@ahmed-masud
Copy link

Another use case for this is a situation where we have a subset of fields that define equality and there should only be exactly one of those instances (in a HashSet e.g.) despite other non-important fields being unequal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Adding or improving documentation good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants