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

Eq + Hash rule is broken when mixing OsStr and Path #55319

Open
mzabaluev opened this issue Oct 24, 2018 · 2 comments
Open

Eq + Hash rule is broken when mixing OsStr and Path #55319

mzabaluev opened this issue Oct 24, 2018 · 2 comments
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@mzabaluev
Copy link
Contributor

mzabaluev commented Oct 24, 2018

This program surprisingly fails:

use std::{
    ffi::OsStr,
    path::Path,
    collections::hash_map::DefaultHasher,
    hash::{Hash, Hasher},
};

fn hash<T: ?Sized + Hash>(v: &T) -> u64 {
    let mut sh = DefaultHasher::new();
    v.hash(&mut sh);
    sh.finish()
}

fn main() {
    let s = OsStr::new("/dev/null");
    let p = Path::new("/dev/null");
    assert_eq!(s, p);
    assert!(
        hash(s) == hash(p),
        "Hashes differ for values that compare as equal"
    );
}

The problem is that the provided impls of PartialEq, while convenient, cross the data domains and break the intuition on matching behavior of Hash and ==. This does not affect HashMap due to lack of a cross-type Borrow impl, but the inconsistency can bite the unwary.

@mzabaluev
Copy link
Contributor Author

There is another problem with Hash and Eq behavior for Path: #29008

@jonas-schievink jonas-schievink added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Mar 23, 2019
@Enselic Enselic added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 20, 2023
@Enselic Enselic changed the title impl PartialEq<OsStr> for Path and similar impls violate Hash consistency Eq + Hash rule is broken when mixing OsStr and Path Nov 20, 2023
@Enselic
Copy link
Member

Enselic commented Nov 20, 2023

Triage: I think the rule is written with a single type in mind, but this example of cross-type rule violation seems unnecessarily inconsistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants