[Foundation] Notification: Add note on == not being reflexive and stabilize hashing

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

Author: lorentey

stdlib/public/Darwin/Foundation/Notification.swift

@@ -41,13 +41,17 @@ public struct Notification : ReferenceConvertible, Equatable, Hashable {
 
     public func hash(into hasher: inout Hasher) {
         hasher.combine(name)
-        hasher.combine(ObjectIdentifier(object as AnyObject))
-        // `userInfo` is not directly hashable, but == compares it by bridging
-        // it to NSDictionary, so its contents should be hashed here.  However,
-        // to prevent potential issues with values fail to correctly implement
-        // `NSObject.hash`, we only hash the keys here.
+
+        // FIXME: We should feed `object` to the hasher, too. However, this
+        // cannot be safely done if its value happens not to be Hashable.
+
+        // FIXME: == compares `userInfo` by bridging it to NSDictionary, so its
+        // contents should be fully hashed here.  However, to prevent stability
+        // issues with userInfo dictionaries that include non-Hashable values,
+        // we only feed the keys to the hasher here.
         //
-        // FIXME: This makes for weak hashes. We really should hash everything.
+        // FIXME: This makes for weak hashes. We really should hash everything
+        // that's compared in ==.
         //
         // The algorithm below is the same as the one used by Dictionary.
         var commutativeKeyHash = 0
@@ -79,6 +83,10 @@ public struct Notification : ReferenceConvertible, Equatable, Hashable {
         }
         if let lhsObj = lhs.object {
             if let rhsObj = rhs.object {
+                // FIXME: Converting an arbitrary value to AnyObject won't use a
+                // stable address when the value needs to be boxed.  Therefore,
+                // this comparison makes == non-reflexive, violating Equatable
+                // requirements. (rdar://problem/49797185)
                 if lhsObj as AnyObject !== rhsObj as AnyObject {
                     return false
                 }
@@ -91,6 +99,9 @@ public struct Notification : ReferenceConvertible, Equatable, Hashable {
         if lhs.userInfo != nil {
             if rhs.userInfo != nil {
                 // user info must be compared in the object form since the userInfo in swift is not comparable
+
+                // FIXME: This violates reflexivity when userInfo contains any
+                // non-Hashable values, for the same reason as described above.
                 return lhs._bridgeToObjectiveC() == rhs._bridgeToObjectiveC()
             } else {
                 return false

test/stdlib/TestNotification.swift

@@ -32,11 +32,64 @@ class TestNotification : TestNotificationSuper {
     func test_unconditionallyBridgeFromObjectiveC() {
         expectEqual(Notification(name: Notification.Name("")), Notification._unconditionallyBridgeFromObjectiveC(nil))
     }
+
+    func test_hashing() {
+        let o1 = NSObject()
+        let o2 = NSObject()
+        let values: [Notification] = [
+            /* 0 */ Notification(name: .init("a"), object: o1, userInfo: nil),
+            /* 1 */ Notification(name: .init("b"), object: o1, userInfo: nil),
+            /* 2 */ Notification(name: .init("a"), object: o2, userInfo: nil),
+            /* 3 */ Notification(name: .init("a"), object: o1, userInfo: ["Foo": 1]),
+            /* 4 */ Notification(name: .init("a"), object: o1, userInfo: ["Foo": 2]),
+            /* 5 */ Notification(name: .init("a"), object: o1, userInfo: ["Bar": 1]),
+            /* 6 */ Notification(name: .init("a"), object: o1, userInfo: ["Foo": 1, "Bar": 2]),
+        ]
+
+        let hashException: Set<Int> = [3, 4]
+
+        checkHashable(
+            values,
+            equalityOracle: { $0 == $1 },
+            hashEqualityOracle: {
+                // FIXME: Unfortunately cases 3 and 4 above currently hash the
+                // same way, even though they compare different.
+                $0 == $1 || (hashException.contains($0) && hashException.contains($1))
+            })
+    }
 }
 
 
 #if !FOUNDATION_XCTEST
 var NotificationTests = TestSuite("TestNotification")
 NotificationTests.test("test_unconditionallyBridgeFromObjectiveC") { TestNotification().test_unconditionallyBridgeFromObjectiveC() }
+NotificationTests.test("test_hashing") { TestNotification().test_hashing() }
+
+private struct NonHashableValueType: Equatable {
+    let value: Int
+    init(_ value: Int) {
+        self.value = value
+    }
+}
+
+NotificationTests.test("test_reflexivity_violation")
+  .xfail(
+    .custom({ true },
+        reason: "<rdar://problem/49797185> Foundation.Notification's equality relation isn't reflexive"))
+  .code {
+    let name = Notification.Name("name")
+    let a = NonHashableValueType(1)
+    let b = NonHashableValueType(2)
+    // Currently none of these values compare equal to themselves:
+    let values: [Notification] = [
+        Notification(name: name, object: a, userInfo: nil),
+        Notification(name: name, object: b, userInfo: nil),
+        Notification(name: name, object: nil, userInfo: ["foo": a]),
+        Notification(name: name, object: nil, userInfo: ["foo": b]),
+    ]
+    checkHashable(values)
+}
+
+
 runAllTests()
 #endif