Skip to content

[5.1][stdlib] RawRepresentable: revert to default _rawHashValue(seed:) #25928

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion stdlib/public/core/CompilerProtocols.swift
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,21 @@ extension RawRepresentable where RawValue: Hashable, Self: Hashable {

@inlinable // trivial
public func _rawHashValue(seed: Int) -> Int {
return rawValue._rawHashValue(seed: seed)
// In 5.0, this used to return rawValue._rawHashValue(seed: seed). This was
// slightly faster, but it interfered with conforming types' ability to
// customize their hashing. The current definition is equivalent to the
// default implementation; however, we need to keep the definition to remain
// ABI compatible with code compiled on 5.0.
//
// Note that unless a type provides a custom hash(into:) implementation,
// this new version returns the same values as the original 5.0 definition,
// so code that used to work in 5.0 remains working whether or not the
// original definition was inlined.
//
// See https://bugs.swift.org/browse/SR-10734
var hasher = Hasher(_seed: seed)
self.hash(into: &hasher)
return hasher._finalize()
}
}

Expand Down
96 changes: 62 additions & 34 deletions test/stdlib/RawRepresentable-tricky-hashing.swift
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
// RUN: %target-run-simple-swift | %FileCheck %s
// RUN: %target-run-simple-swift
// REQUIRES: executable_test

// RawRepresentable is not Equatable itself, but it does provide a generic
// implementation of == based on rawValues. This gets picked up as the
// implementation of Equatable.== when a concrete RawRepresentable type conforms
// to Equatable without providing its own implementation.
//
// However, RawRepresentable used to not provide equivalent implementations for
// hashing, allowing the compiler to synthesized hashing as usual, based on the
// actual contents of the type rather than its rawValue. Thus, the definitions
// of equality and hashing may not actually match, leading to broken hashes.
//
// The difference between rawValue and the actual contents is subtle, and it
// only causes problems in custom RawRepresentable implementations where the
// rawValue isn't actually the storage representation, like the weird struct
// below.
//
// rdar://problem/45308741
import StdlibUnittest

let suite = TestSuite("RawRepresentable")

extension Hasher {
static func hash<H: Hashable>(_ value: H) -> Int {
var hasher = Hasher()
hasher.combine(value)
return hasher.finalize()
}
}



struct TrickyRawRepresentable: RawRepresentable, Hashable {
var value: [Unicode.Scalar]
Expand All @@ -30,27 +27,58 @@ struct TrickyRawRepresentable: RawRepresentable, Hashable {
}
}

let s1 = TrickyRawRepresentable(rawValue: "café")!
let s2 = TrickyRawRepresentable(rawValue: "cafe\u{301}")!
suite.test("Tricky hashing") {
// RawRepresentable is not Equatable itself, but it does provide a generic
// implementation of == based on rawValue. This gets picked up as the
// implementation of Equatable.== when a concrete RawRepresentable type
// conforms to Equatable without providing its own implementation.
//
// However, RawRepresentable used to not provide equivalent implementations
// for hashing, allowing the compiler to synthesize hashing as usual, based on
// the actual contents of the type rather than its rawValue. Thus, the
// definitions of equality and hashing did not actually match in some cases,
// leading to broken behavior.
//
// The difference between rawValue and the actual contents is subtle, and it
// only causes problems in custom RawRepresentable implementations where the
// rawValue isn't actually the storage representation, like the weird struct
// above.
//
// rdar://problem/45308741

// CHECK: s1 == s2: true
print("s1 == s2: \(s1 == s2)")
let s1 = TrickyRawRepresentable(rawValue: "café")!
let s2 = TrickyRawRepresentable(rawValue: "cafe\u{301}")!

// CHECK: hashValue matches: true
print("hashValue matches: \(s1.hashValue == s2.hashValue)")
expectEqual(s1, s2)
expectEqual(s1.hashValue, s2.hashValue)
expectEqual(Hasher.hash(s1), Hasher.hash(s2))
expectEqual(s1._rawHashValue(seed: 42), s2._rawHashValue(seed: 42))
}

extension Hasher {
static func hash<H: Hashable>(_ value: H) -> Int {
var hasher = Hasher()
hasher.combine(value)
return hasher.finalize()
struct CustomRawRepresentable: RawRepresentable, Hashable {
var rawValue: Int

init?(rawValue: Int) {
self.rawValue = rawValue
}

func hash(into hasher: inout Hasher) {
hasher.combine(23)
}
}

// CHECK: hash(into:) matches: true
print("hash(into:) matches: \(Hasher.hash(s1) == Hasher.hash(s2))")
suite.test("Custom hashing") {
// In 5.0, RawRepresentable had a bogus default implementation for
// _rawHashValue(seed:) that interfered with custom hashing for
// RawRepresentable types. Adding a custom hash(into:) implementation should
// always be enough to customize hashing.
//
// See https://bugs.swift.org/browse/SR-10734

if #available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) {
let r = CustomRawRepresentable(rawValue: 42)!
expectEqual(Hasher.hash(r), Hasher.hash(23))
}
}

// CHECK: _rawHashValue(seed:) matches: true
let r1 = s1._rawHashValue(seed: 42)
let r2 = s2._rawHashValue(seed: 42)
print("_rawHashValue(seed:) matches: \(r1 == r2)")
runAllTests()