Merge pull request #14331 from lorentey/rdar/35995647-4.1

[4.1][Foundation] Coalesce duplicate String keys in bridged NSDictionary and NSSet

Author: tkremenek

stdlib/public/SDK/Foundation/NSDictionary.swift

@@ -80,6 +80,23 @@ extension Dictionary : _ObjectiveCBridgeable {
       return
     }
 
+    if Key.self == String.self {
+      // String and NSString have different concepts of equality, so
+      // string-keyed NSDictionaries may generate key collisions when bridged
+      // over to Swift. See rdar://problem/35995647
+      var dict = Dictionary(minimumCapacity: d.count)
+      d.enumerateKeysAndObjects({ (anyKey: Any, anyValue: Any, _) in
+        let key = Swift._forceBridgeFromObjectiveC(
+          anyKey as AnyObject, Key.self)
+        let value = Swift._forceBridgeFromObjectiveC(
+          anyValue as AnyObject, Value.self)
+        // FIXME: Log a warning if `dict` already had a value for `key`
+        dict[key] = value
+      })
+      result = dict
+      return
+    }
+
     // `Dictionary<Key, Value>` where either `Key` or `Value` is a value type
     // may not be backed by an NSDictionary.
     var builder = _DictionaryBuilder<Key, Value>(count: d.count)
@@ -115,26 +132,9 @@ extension Dictionary : _ObjectiveCBridgeable {
     // dictionary; map it to an empty dictionary.
     if _slowPath(d == nil) { return Dictionary() }
 
-    if let native = [Key : Value]._bridgeFromObjectiveCAdoptingNativeStorageOf(
-        d! as AnyObject) {
-      return native
-    }
-
-    if _isBridgedVerbatimToObjectiveC(Key.self) &&
-       _isBridgedVerbatimToObjectiveC(Value.self) {
-      return [Key : Value](
-        _cocoaDictionary: unsafeBitCast(d! as AnyObject, to: _NSDictionary.self))
-    }
-
-    // `Dictionary<Key, Value>` where either `Key` or `Value` is a value type
-    // may not be backed by an NSDictionary.
-    var builder = _DictionaryBuilder<Key, Value>(count: d!.count)
-    d!.enumerateKeysAndObjects({ (anyKey: Any, anyValue: Any, _) in
-      builder.add(
-          key: Swift._forceBridgeFromObjectiveC(anyKey as AnyObject, Key.self),
-          value: Swift._forceBridgeFromObjectiveC(anyValue as AnyObject, Value.self))
-    })
-    return builder.take()
+    var result: Dictionary? = nil
+    _forceBridgeFromObjectiveC(d!, result: &result)
+    return result!
   }
 }
 

stdlib/public/SDK/Foundation/NSSet.swift

@@ -73,6 +73,21 @@ extension Set : _ObjectiveCBridgeable {
       return
     }
 
+    if Element.self == String.self {
+      // String and NSString have different concepts of equality, so
+      // string-keyed NSSets may generate key collisions when bridged over to
+      // Swift. See rdar://problem/35995647
+      var set = Set(minimumCapacity: s.count)
+      s.enumerateObjects({ (anyMember: Any, _) in
+        let member = Swift._forceBridgeFromObjectiveC(
+          anyMember as AnyObject, Element.self)
+        // FIXME: Log a warning if `member` is already in the set.
+        set.insert(member)
+      })
+      result = set
+      return
+    }
+
     // `Set<Element>` where `Element` is a value type may not be backed by
     // an NSSet.
     var builder = _SetBuilder<Element>(count: s.count)
@@ -101,25 +116,9 @@ extension Set : _ObjectiveCBridgeable {
     // set; map it to an empty set.
     if _slowPath(s == nil) { return Set() }
 
-    if let native =
-      Set<Element>._bridgeFromObjectiveCAdoptingNativeStorageOf(s! as AnyObject) {
-
-      return native
-    }
-
-    if _isBridgedVerbatimToObjectiveC(Element.self) {
-      return Set<Element>(_cocoaSet: unsafeBitCast(s! as AnyObject,
-                                                   to: _NSSet.self))
-    }
-
-    // `Set<Element>` where `Element` is a value type may not be backed by
-    // an NSSet.
-    var builder = _SetBuilder<Element>(count: s!.count)
-    s!.enumerateObjects({ (anyMember: Any, _) in
-      builder.add(member: Swift._forceBridgeFromObjectiveC(
-        anyMember as AnyObject, Element.self))
-    })
-    return builder.take()
+    var result: Set? = nil
+    Set<Element>._forceBridgeFromObjectiveC(s!, result: &result)
+    return result!
   }
 }
 

validation-test/stdlib/Dictionary.swift

@@ -3154,6 +3154,26 @@ DictionaryTestSuite.test("BridgedFromObjC.Nonverbatim.ArrayOfDictionaries") {
   }
 }
 
+DictionaryTestSuite.test("BridgedFromObjC.Nonverbatim.StringEqualityMismatch") {
+  // NSString's isEqual(_:) implementation is stricter than Swift's String, so
+  // Dictionary values bridged over from Objective-C may have duplicate keys.
+  // rdar://problem/35995647
+  let cafe1 = "Cafe\u{301}" as NSString
+  let cafe2 = "Café" as NSString
+
+  let nsd = NSMutableDictionary()
+  nsd.setObject(42, forKey: cafe1)
+  nsd.setObject(23, forKey: cafe2)
+  expectEqual(2, nsd.count)
+  expectTrue((42 as NSNumber).isEqual(nsd.object(forKey: cafe1)))
+  expectTrue((23 as NSNumber).isEqual(nsd.object(forKey: cafe2)))
+
+  let d = convertNSDictionaryToDictionary(nsd) as [String: Int]
+  expectEqual(1, d.count)
+  expectEqual(d["Cafe\u{301}"], d["Café"])
+  let v = d["Café"]
+  expectTrue(v == 42 || v == 23)
+}
 
 //===---
 // Dictionary -> NSDictionary bridging tests.

validation-test/stdlib/Set.swift

@@ -2152,6 +2152,28 @@ SetTestSuite.test("BridgedFromObjC.Nonverbatim.ArrayOfSets") {
   }
 }
 
+SetTestSuite.test("BridgedFromObjC.Nonverbatim.StringEqualityMismatch") {
+  // NSString's isEqual(_:) implementation is stricter than Swift's String, so
+  // Set values bridged over from Objective-C may have duplicate keys.
+  // rdar://problem/35995647
+  let cafe1 = "Cafe\u{301}" as NSString
+  let cafe2 = "Café" as NSString
+
+  let nsset = NSMutableSet()
+  nsset.add(cafe1)
+  expectTrue(nsset.contains(cafe1))
+  expectFalse(nsset.contains(cafe2))
+  nsset.add(cafe2)
+  expectEqual(2, nsset.count)
+  expectTrue(nsset.contains(cafe1))
+  expectTrue(nsset.contains(cafe2))
+  
+  let s: Set<String> = convertNSSetToSet(nsset)
+  expectEqual(1, s.count)
+  expectTrue(s.contains("Cafe\u{301}"))
+  expectTrue(s.contains("Café"))
+  expectTrue(Array(s) == ["Café"])
+}
 
 //===---
 // Dictionary -> NSDictionary bridging tests.