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

[4.2][stdlib] Make sure _SwiftNewtypeWrapper hashes the same way as its RawValue #15991

Merged

Conversation

lorentey
Copy link
Member

_SwiftNewtypeWrapper forwarded hashValue to its rawValue, but it failed to do the same for _hash(into:), which resulted in the wrapper struct having subtly different hashing than the raw value.

This violated an implicit invariant when dictionaries/sets of such types were bridged to Objective-C through AnyHashable, leading to nondeterministic but frequent crashes.

rdar://problem/39398060

(cherry picked from commit 6c5a6a6)

…ts RawValue

_SwiftNewtypeWrapper forwarded hashValue to its rawValue, but it failed to do the same for _hash(into:), which resulted in the wrapper struct having subtly different hashing than the raw value.

This violated an implicit invariant when dictionaries/sets of such types were bridged to Objective-C through AnyHashable, leading to nondeterministic but frequent crashes.

rdar://problem/39398060

(cherry picked from commit 6c5a6a6)
@lorentey
Copy link
Member Author

@swift-ci please smoke test

@lorentey lorentey requested a review from DougGregor April 17, 2018 21:25
@lorentey
Copy link
Member Author

@DougGregor could you take a quick look? Note that I had to add the new Self: Hashable constraint on the extension; otherwise the _hash(into:) definition clashed with its default implementation.

@lorentey lorentey requested a review from airspeedswift April 17, 2018 21:36
Copy link
Member

@airspeedswift airspeedswift left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Can you add a test for the issue it resolves?

@lorentey
Copy link
Member Author

@airspeedswift PR #15939 includes an update to the test suite's checkHashable to also verify hash(into:) equivalency (among other things). This update will extend the scope of an existing test so that it will cover this issue. I expect to land that very soon.

@lorentey
Copy link
Member Author

@swift-ci please test and merge

@swift-ci swift-ci merged commit ed1b8f3 into swiftlang:swift-4.2-branch Apr 18, 2018
@lorentey lorentey deleted the newtype-vs-anyhashable-4.2 branch April 18, 2018 13:05
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.

3 participants