Skip to content

Commit

Permalink
std: Base Hash for Path on its iterator
Browse files Browse the repository at this point in the history
Almost all operations on Path are based on the components iterator in one form
or another to handle equivalent paths. The `Hash` implementations, however,
mistakenly just went straight to the underlying `OsStr`, causing these
equivalent paths to not get merged together.

This commit updates the `Hash` implementation to also be based on the iterator
which should ensure that if two paths are equal they hash to the same thing.

cc rust-lang#29008, but doesn't close it
  • Loading branch information
alexcrichton committed Nov 2, 2015
1 parent 01fd4d6 commit d2dd700
Showing 1 changed file with 37 additions and 3 deletions.
40 changes: 37 additions & 3 deletions src/libstd/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ use borrow::{Borrow, IntoCow, ToOwned, Cow};
use cmp;
use fmt;
use fs;
use hash::{Hash, Hasher};
use io;
use iter;
use mem;
Expand Down Expand Up @@ -446,7 +447,7 @@ enum State {
///
/// Does not occur on Unix.
#[stable(feature = "rust1", since = "1.0.0")]
#[derive(Copy, Clone, Eq, Hash, Debug)]
#[derive(Copy, Clone, Eq, Debug)]
pub struct PrefixComponent<'a> {
/// The prefix as an unparsed `OsStr` slice.
raw: &'a OsStr,
Expand Down Expand Up @@ -490,6 +491,13 @@ impl<'a> cmp::Ord for PrefixComponent<'a> {
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<'a> Hash for PrefixComponent<'a> {
fn hash<H: Hasher>(&self, h: &mut H) {
self.parsed.hash(h);
}
}

/// A single component of a path.
///
/// See the module documentation for an in-depth explanation of components and
Expand Down Expand Up @@ -932,7 +940,7 @@ impl<'a> cmp::Ord for Components<'a> {
/// path.push("system32");
/// path.set_extension("dll");
/// ```
#[derive(Clone, Hash)]
#[derive(Clone)]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct PathBuf {
inner: OsString
Expand Down Expand Up @@ -1171,6 +1179,13 @@ impl cmp::PartialEq for PathBuf {
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Hash for PathBuf {
fn hash<H: Hasher>(&self, h: &mut H) {
self.as_path().hash(h)
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl cmp::Eq for PathBuf {}

Expand Down Expand Up @@ -1224,7 +1239,6 @@ impl Into<OsString> for PathBuf {
/// let parent_dir = path.parent();
/// ```
///
#[derive(Hash)]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Path {
inner: OsStr
Expand Down Expand Up @@ -1809,6 +1823,15 @@ impl cmp::PartialEq for Path {
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Hash for Path {
fn hash<H: Hasher>(&self, h: &mut H) {
for component in self.components() {
component.hash(h);
}
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl cmp::Eq for Path {}

Expand Down Expand Up @@ -3035,6 +3058,14 @@ mod tests {

#[test]
pub fn test_compare() {
use hash::{Hash, Hasher, SipHasher};

fn hash<T: Hash>(t: T) -> u64 {
let mut s = SipHasher::new_with_keys(0, 0);
t.hash(&mut s);
s.finish()
}

macro_rules! tc(
($path1:expr, $path2:expr, eq: $eq:expr,
starts_with: $starts_with:expr, ends_with: $ends_with:expr,
Expand All @@ -3045,6 +3076,9 @@ mod tests {
let eq = path1 == path2;
assert!(eq == $eq, "{:?} == {:?}, expected {:?}, got {:?}",
$path1, $path2, $eq, eq);
assert!($eq == (hash(path1) == hash(path2)),
"{:?} == {:?}, expected {:?}, got {} and {}",
$path1, $path2, $eq, hash(path1), hash(path2));

let starts_with = path1.starts_with(path2);
assert!(starts_with == $starts_with,
Expand Down

0 comments on commit d2dd700

Please sign in to comment.