Skip to content

HashMap docs don't mention {Eq, Hash} consistency #23320

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

Closed
apasel422 opened this issue Mar 12, 2015 · 2 comments · Fixed by #23814
Closed

HashMap docs don't mention {Eq, Hash} consistency #23320

apasel422 opened this issue Mar 12, 2015 · 2 comments · Fixed by #23814

Comments

@apasel422
Copy link
Contributor

While it may not be possible to enforce this at the type level, the docs for HashMap (and HashSet) should specify that its key type's {Eq, Hash} implementations must be consistent for the map to have reasonable behavior. The exact requirement is

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

This is always the case for derived implementations, but the following example is nonsensical if Foo is used as a HashMap key:

#[derive(Hash)]
struct Foo(u32, u32);

impl PartialEq for Foo {
    fn eq(&self, other: &Foo) -> bool { self.0 == other.0 }
}

impl Eq for Foo {}

There could additionally be some kind of debug-level assertion of this behavior, or possibly a lint that checks to make sure that the fields that are referenced in PartialEq::eq and Hash::hash are consistent. The lint wouldn't be perfect, but could be useful.

@ghost
Copy link

ghost commented Mar 12, 2015

This belongs to the class of properties that are great to check for with quickcheck.

use std::fmt::Debug;
use std::hash::{hash, Hash, SipHasher};
use quickcheck::{Arbitrary, quickcheck};

fn prop_sane_hash<T: Arbitrary + Debug + Hash + PartialEq>(a: T, b: T) -> bool {
    a != b || hash::<_, SipHasher>(&a) == hash::<_, SipHasher>(&b)
}
fn main() {
    quickcheck(prop_sane_hash as fn(Foo, Foo) -> bool);
}

@kennytm
Copy link
Member

kennytm commented Mar 13, 2015

I think this is a generic property that should be mentioned in Hash, not just HashMap/HashSet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants