Merge pull request #25928 from lorentey/RawRepresentable-hashing-5.1

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

Author: lorentey

stdlib/public/core/CompilerProtocols.swift

@@ -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()
   }
 }
 

test/stdlib/RawRepresentable-tricky-hashing.swift

@@ -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]
@@ -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()