Skip to content

std: Base Hash for Path on its iterator #29456

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

Merged
merged 1 commit into from
Nov 2, 2015

Conversation

alexcrichton
Copy link
Member

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 #29008, but doesn't close it

@rust-highfive
Copy link
Contributor

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@aturon
Copy link
Member

aturon commented Nov 2, 2015

A definite pitfall of derive! I wonder if we could lint for cases where you've given a custom implementation of PartialEq or Hash but derived the other.

@bors: r+

@bors
Copy link
Collaborator

bors commented Nov 2, 2015

📌 Commit 3a07518 has been approved by aturon

@bors
Copy link
Collaborator

bors commented Nov 2, 2015

⌛ Testing commit 3a07518 with merge 7216ea7...

@bors
Copy link
Collaborator

bors commented Nov 2, 2015

💔 Test failed - auto-win-gnu-64-opt

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
@alexcrichton
Copy link
Member Author

@bors: r=aturon d2dd700

@bors
Copy link
Collaborator

bors commented Nov 2, 2015

⌛ Testing commit d2dd700 with merge a1fd944...

bors added a commit that referenced this pull request Nov 2, 2015
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 #29008, but doesn't close it
@bors bors merged commit d2dd700 into rust-lang:master Nov 2, 2015
@alexcrichton alexcrichton deleted the path-hash branch January 21, 2016 01:31
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 this pull request may close these issues.

5 participants