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

Derived Hash is incorrect if PartialEq is not derived #29758

Closed
untitaker opened this issue Nov 10, 2015 · 6 comments
Closed

Derived Hash is incorrect if PartialEq is not derived #29758

untitaker opened this issue Nov 10, 2015 · 6 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.

Comments

@untitaker
Copy link
Contributor

This should not work:

#[derive(Hash)]
struct Foo { bogus: usize };

impl PartialEq for Foo {
    fn eq(&self, _: &Foo) -> bool { true }
}

impl Eq for Foo {};

playpen

We have this in the docs of std::hash::Hash:

k1 == k2 -> hash(k1) == hash(k2)

Since all Foos are equal, their hashes must always be the same too, but the derived hash impl doesn't behave that way (see playpen).

One way to avoid this issue is to give up on deriving Hash when PartialEq isn't automatically derived.

@Gankra
Copy link
Contributor

Gankra commented Nov 10, 2015

Enforcing this rigorously seems unreasonable, but one could lint against this. See https://github.com/Manishearth/rust-clippy/issues/430

@untitaker
Copy link
Contributor Author

I don't know, rust-clippy mostly seems to be about warning about either ugly or noop/inefficent code, while deriving Hash without deriving PartialEq leads to actual bugs.

@apasel422
Copy link
Contributor

I think a lint with a warning here would be appropriate.

@steveklabnik steveklabnik added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Nov 11, 2015
@steveklabnik
Copy link
Member

Since new lints have a big impact on users of rustc, the policy is that they should go through the RFC process like other user-facing changes. As such, I'm going to give this one a close, but if anyone comes across this ticket and wants this lint, consider adding it to clippy and/or writing up an RFC. Thanks!

@untitaker
Copy link
Contributor Author

Would it be appropriate to just refile this issue against rust-lang/rfcs for now?

@untitaker
Copy link
Contributor Author

whatever, continued at rust-lang/rfcs#1499

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.
Projects
None yet
Development

No branches or pull requests

4 participants