Skip to content

[5.1][Foundation] Modernize hashing in Foundation's Swift-only types #24228

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 32 commits into from
Apr 26, 2019

Conversation

lorentey
Copy link
Member

This is cherry picked from #23832; see discussion there.

rdar://problem/43394032

lorentey added 30 commits April 23, 2019 15:27
…*all* components

The new definition diverges from the existing hashValue: it feeds all path components to the hasher, rather than just a limited subset.

Hashing all the bits that are compared in == is necessary to ensure proper operation of the hash table.
This replaces the compiler-generated variant (based on hashValue) with a better implementation.
Note: URL does not implement Hashable.
I’m not sure why I was so convinced it wasn’t, but this certainly simplifies things.
…bilize hashing

The `ObjectIdentifier(object as AnyObject)` is not necessarily stable; this breaks reflexivity for ==, and it makes the hash encoding nondeterministic.
@lorentey
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 8cd901f1409070c9234a6d6cfc7c150820544250

@lorentey lorentey force-pushed the foundation-hashing-5.1 branch from 8cd901f to c894cbb Compare April 24, 2019 06:05
@lorentey
Copy link
Member Author

@swift-ci please test

@swift-ci

This comment has been minimized.

@swift-ci

This comment has been minimized.

@lorentey
Copy link
Member Author

@swift-ci please test

@swift-ci

This comment has been minimized.

@lorentey lorentey merged commit 18c281b into swiftlang:swift-5.1-branch Apr 26, 2019
@lorentey lorentey deleted the foundation-hashing-5.1 branch April 26, 2019 22:07
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.

2 participants