Merge pull request #19563 from apple/revert-19500-updateValue-but-not-the-key

Author: swift-ci

stdlib/public/core/NativeDictionary.swift

@@ -413,6 +413,10 @@ extension _NativeDictionary { // Insertions
     if found {
       let oldValue = (_values + index.bucket).move()
       (_values + index.bucket).initialize(to: value)
+      // FIXME: Replacing the old key with the new is unnecessary, unintuitive,
+      // and actively harmful to some usecases. We shouldn't do it.
+      // rdar://problem/32144087
+      (_keys + index.bucket).pointee = key
       return oldValue
     }
     _insert(at: index, key: key, value: value)
@@ -428,6 +432,10 @@ extension _NativeDictionary { // Insertions
     let (index, found) = mutatingFind(key, isUnique: isUnique)
     if found {
       (_values + index.bucket).pointee = value
+      // FIXME: Replacing the old key with the new is unnecessary, unintuitive,
+      // and actively harmful to some usecases. We shouldn't do it.
+      // rdar://problem/32144087
+      (_keys + index.bucket).pointee = key
     } else {
       _insert(at: index, key: key, value: value)
     }

validation-test/stdlib/Dictionary.swift

@@ -411,7 +411,7 @@ DictionaryTestSuite.test("COW.Fast.UpdateValueForKeyDoesNotReallocate") {
   }
 }
 
-DictionaryTestSuite.test("COW.Slow.UpdateValueForKeyDoesNotReallocate") {
+DictionaryTestSuite.test("COW.Slow.AddDoesNotReallocate") {
   do {
     var d1 = getCOWSlowDictionary()
     let identity1 = d1._rawIdentifier()
@@ -4642,32 +4642,6 @@ DictionaryTestSuite.test("removeAt") {
   }
 }
 
-DictionaryTestSuite.test("updateValue") {
-  let key1 = TestKeyTy(42)
-  let key2 = TestKeyTy(42)
-  let value1 = TestValueTy(1)
-  let value2 = TestValueTy(2)
-
-  var d: [TestKeyTy: TestValueTy] = [:]
-
-  expectNil(d.updateValue(value1, forKey: key1))
-
-  expectEqual(d.count, 1)
-  let index1 = d.index(forKey: key2)
-  expectNotNil(index1)
-  expectTrue(d[index1!].key === key1)
-  expectTrue(d[index1!].value === value1)
-
-  expectTrue(d.updateValue(value2, forKey: key2) === value1)
-
-  expectEqual(d.count, 1)
-  let index2 = d.index(forKey: key2)
-  expectEqual(index1, index2)
-  // We expect updateValue to keep the original key in place.
-  expectTrue(d[index2!].key === key1) // Not key2
-  expectTrue(d[index2!].value === value2)
-}
-
 DictionaryTestSuite.test("localHashSeeds") {
   // With global hashing, copying elements in hash order between hash tables
   // can become quadratic. (See https://bugs.swift.org/browse/SR-3268)