[Deque] Work around stdlib issue with Array.withContiguousStorageIfAvailable (apple#44)

lorentey · web-flow · commit de5f1e606178 · 2021-05-27T18:59:45.000-07:00
Resolves apple#27.
diff --git a/Sources/DequeModule/Compatibility.swift b/Sources/DequeModule/Compatibility.swift
@@ -0,0 +1,62 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift Collections open source project
+//
+// Copyright (c) 2021 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+//
+//===----------------------------------------------------------------------===//
+
+extension Array {
+  /// Returns true if `Array.withContiguousStorageIfAvailable` is broken
+  /// in the stdlib we're currently running on.
+  ///
+  /// See https://bugs.swift.org/browse/SR-14663.
+  @inlinable
+  internal static func _isWCSIABroken() -> Bool {
+    #if _runtime(_ObjC)
+    guard _isBridgedVerbatimToObjectiveC(Element.self) else {
+      // SR-14663 only triggers on array values that are verbatim bridged
+      // from Objective-C, so it cannot ever trigger for element types
+      // that aren't verbatim bridged.
+      return false
+    }
+
+    // SR-14663 was introduced in Swift 5.1. Check if we have a broken stdlib.
+
+    // The bug is caused by a bogus precondition inside a non-inlinable stdlib
+    // method, so to determine if we're affected, we need to check the currently
+    // running OS version.
+    #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
+    guard #available(macOS 10.15, iOS 13, watchOS 6, tvOS 13, *) else {
+      // The OS is too old to be affected by this bug.
+      return false
+    }
+    #endif
+    // FIXME: When a stdlib is released that contains a fix, add a check for it.
+    return true
+
+    #else
+    // Platforms that don't have an Objective-C runtime don't have verbatim
+    // bridged array values, so the bug doesn't apply to them.
+    return false
+    #endif
+  }
+}
+
+extension Sequence {
+  // An adjusted version of the standard `withContiguousStorageIfAvailable`
+  // method that works around https://bugs.swift.org/browse/SR-14663.
+  @inlinable
+  internal func _withContiguousStorageIfAvailable_SR14663<R>(
+    _ body: (UnsafeBufferPointer<Element>) throws -> R
+  ) rethrows -> R? {
+    if Self.self == Array<Element>.self && Array<Element>._isWCSIABroken() {
+      return nil
+    }
+
+    return try self.withContiguousStorageIfAvailable(body)
+  }
+}
diff --git a/Sources/DequeModule/Deque+Extras.swift b/Sources/DequeModule/Deque+Extras.swift
@@ -131,7 +131,7 @@ extension Deque {
   /// - SeeAlso: `append(contentsOf:)`
   @inlinable
   public mutating func prepend<C: Collection>(contentsOf newElements: C) where C.Element == Element {
-    let done: Void? = newElements.withContiguousStorageIfAvailable { source in
+    let done: Void? = newElements._withContiguousStorageIfAvailable_SR14663 { source in
       _storage.ensureUnique(minimumCapacity: count + source.count)
       _storage.update { $0.uncheckedPrepend(contentsOf: source) }
     }
@@ -166,6 +166,12 @@ extension Deque {
   /// - SeeAlso: `append(contentsOf:)`
   @inlinable
   public mutating func prepend<S: Sequence>(contentsOf newElements: S) where S.Element == Element {
+    let done: Void? = newElements._withContiguousStorageIfAvailable_SR14663 { source in
+      _storage.ensureUnique(minimumCapacity: count + source.count)
+      _storage.update { $0.uncheckedPrepend(contentsOf: source) }
+    }
+    guard done == nil else { return }
+
     let originalCount = self.count
     self.append(contentsOf: newElements)
     let newCount = self.count
diff --git a/Sources/DequeModule/Deque+RangeReplaceableCollection.swift b/Sources/DequeModule/Deque+RangeReplaceableCollection.swift
@@ -141,7 +141,7 @@ extension Deque: RangeReplaceableCollection {
     _storage.update { handle in
       assert(handle.startSlot == .zero)
       let target = handle.mutableBuffer(for: .zero ..< _Slot(at: c))
-      let done: Void? = elements.withContiguousStorageIfAvailable { source in
+      let done: Void? = elements._withContiguousStorageIfAvailable_SR14663 { source in
         target._initialize(from: source)
       }
       if done == nil {
@@ -198,7 +198,7 @@ extension Deque: RangeReplaceableCollection {
   /// - Complexity: Amortized O(`newElements.count`).
   @inlinable
   public mutating func append<S: Sequence>(contentsOf newElements: S) where S.Element == Element {
-    let done: Void? = newElements.withContiguousStorageIfAvailable { source in
+    let done: Void? = newElements._withContiguousStorageIfAvailable_SR14663 { source in
       _storage.ensureUnique(minimumCapacity: count + source.count)
       _storage.update { $0.uncheckedAppend(contentsOf: source) }
     }
@@ -240,7 +240,7 @@ extension Deque: RangeReplaceableCollection {
   /// - Complexity: Amortized O(`newElements.count`).
   @inlinable
   public mutating func append<C: Collection>(contentsOf newElements: C) where C.Element == Element {
-    let done: Void? = newElements.withContiguousStorageIfAvailable { source in
+    let done: Void? = newElements._withContiguousStorageIfAvailable_SR14663 { source in
       _storage.ensureUnique(minimumCapacity: count + source.count)
       _storage.update { $0.uncheckedAppend(contentsOf: source) }
     }
diff --git a/Tests/DequeTests/DequeTests.swift b/Tests/DequeTests/DequeTests.swift
@@ -301,13 +301,14 @@ final class DequeTests: CollectionTestCase {
     }
   }
 
-  func test_prependManyFromArray() {
+  func test_prependManyFromContiguousArray_asCollection() {
     withEveryDeque("deque", ofCapacities: [0, 1, 2, 3, 5, 10]) { layout in
       withEvery("prependCount", in: 0 ..< 10) { prependCount in
         withEvery("isShared", in: [false, true]) { isShared in
           withLifetimeTracking { tracker in
             var (deque, contents) = tracker.deque(with: layout)
-            let extra = tracker.instances(for: layout.count ..< layout.count + prependCount)
+            let extraRange = layout.count ..< layout.count + prependCount
+            let extra = ContiguousArray(tracker.instances(for: extraRange))
             withHiddenCopies(if: isShared, of: &deque) { deque in
               contents.insert(contentsOf: extra, at: 0)
               deque.prepend(contentsOf: extra)
@@ -319,5 +320,55 @@ final class DequeTests: CollectionTestCase {
     }
   }
 
+  func test_prependManyFromContiguousArray_asSequence() {
+    // This calls the Sequence-based `Deque.prepend` overload, even if
+    // `elements` happens to be of a Collection type.
+    func prependSequence<S: Sequence>(
+      contentsOf elements: S,
+      to deque: inout Deque<S.Element>
+    ) {
+      deque.prepend(contentsOf: elements)
+    }
+
+    withEveryDeque("deque", ofCapacities: [0, 1, 2, 3, 5, 10]) { layout in
+      withEvery("prependCount", in: 0 ..< 10) { prependCount in
+        withEvery("isShared", in: [false, true]) { isShared in
+          withLifetimeTracking { tracker in
+            var (deque, contents) = tracker.deque(with: layout)
+            let extraRange = layout.count ..< layout.count + prependCount
+            let extra = ContiguousArray(tracker.instances(for: extraRange))
+            withHiddenCopies(if: isShared, of: &deque) { deque in
+              contents.insert(contentsOf: extra, at: 0)
+              prependSequence(contentsOf: extra, to: &deque)
+              expectEqualElements(deque, contents)
+            }
+          }
+        }
+      }
+    }
+  }
+
+  func test_prependManyFromBridgedArray() {
+    // https://github.com/apple/swift-collections/issues/27
+    withEveryDeque("deque", ofCapacities: [0, 1, 2, 3, 5, 10]) { layout in
+      withEvery("appendCount", in: 0 ..< 10) { appendCount in
+        withEvery("isShared", in: [false, true]) { isShared in
+          var contents: [NSObject] = (0 ..< layout.count).map { _ in NSObject() }
+          var deque = Deque(layout: layout, contents: contents)
+          let extra: [NSObject] = (0 ..< appendCount)
+            .map { _ in NSObject() }
+            .withUnsafeBufferPointer { buffer in
+              NSArray(objects: buffer.baseAddress, count: buffer.count) as! [NSObject]
+            }
+          withHiddenCopies(if: isShared, of: &deque) { deque in
+            contents.insert(contentsOf: extra, at: 0)
+            deque.prepend(contentsOf: extra)
+            expectEquivalentElements(deque, contents, by: ===)
+          }
+        }
+      }
+    }
+  }
+
 }
 
diff --git a/Tests/DequeTests/RangeReplaceableCollectionTests.swift b/Tests/DequeTests/RangeReplaceableCollectionTests.swift
@@ -55,6 +55,28 @@ final class RangeReplaceableCollectionTests: CollectionTestCase {
     }
   }
 
+  func test_sequenceInitializer_ContiguousArray() {
+    withEvery("count", in: [0, 1, 2, 10, 100]) { count in
+      withLifetimeTracking { tracker in
+        let contents = ContiguousArray(tracker.instances(for: 0 ..< count))
+        let d1 = Deque(contents)
+        expectEqualElements(d1, contents)
+      }
+    }
+  }
+
+  func test_sequenceInitializer_bridgedArray() {
+    // https://github.com/apple/swift-collections/issues/27
+    withEvery("count", in: [0, 1, 2, 10, 100]) { count in
+      let contents: [AnyObject] = (0 ..< count).map { _ in NSObject() }
+      let array: [AnyObject] = contents.withUnsafeBufferPointer { buffer in
+        NSArray(objects: buffer.baseAddress, count: buffer.count) as [AnyObject]
+      }
+      let deque = Deque(array)
+      expectEquivalentElements(deque, contents, by: ===)
+    }
+  }
+
   func test_replaceSubrange_withMinimalCollection() {
     withEveryDeque("deque", ofCapacities: [0, 1, 2, 3, 5, 10]) { layout in
       withEveryRange("range", in: 0 ..< layout.count) { range in
@@ -172,13 +194,14 @@ final class RangeReplaceableCollectionTests: CollectionTestCase {
     }
   }
 
-  func test_appendManyFromArray() {
+  func test_appendManyFromContiguousArray() {
     withEveryDeque("deque", ofCapacities: [0, 1, 2, 3, 5, 10]) { layout in
       withEvery("appendCount", in: 0 ..< 10) { appendCount in
         withEvery("isShared", in: [false, true]) { isShared in
           withLifetimeTracking { tracker in
             var (deque, contents) = tracker.deque(with: layout)
-            let extra = tracker.instances(for: layout.count ..< layout.count + appendCount)
+            let extraRange = layout.count ..< layout.count + appendCount
+            let extra = ContiguousArray(tracker.instances(for: extraRange))
             withHiddenCopies(if: isShared, of: &deque) { deque in
               contents.append(contentsOf: extra)
               deque.append(contentsOf: extra)
@@ -190,6 +213,28 @@ final class RangeReplaceableCollectionTests: CollectionTestCase {
     }
   }
 
+  func test_appendManyFromBridgedArray() {
+    // https://github.com/apple/swift-collections/issues/27
+    withEveryDeque("deque", ofCapacities: [0, 1, 2, 3, 5, 10]) { layout in
+      withEvery("appendCount", in: 0 ..< 10) { appendCount in
+        withEvery("isShared", in: [false, true]) { isShared in
+          var contents: [NSObject] = (0 ..< layout.count).map { _ in NSObject() }
+          var deque = Deque(layout: layout, contents: contents)
+          let extra: [NSObject] = (0 ..< appendCount)
+            .map { _ in NSObject() }
+            .withUnsafeBufferPointer { buffer in
+              NSArray(objects: buffer.baseAddress, count: buffer.count) as! [NSObject]
+            }
+          withHiddenCopies(if: isShared, of: &deque) { deque in
+            contents.append(contentsOf: extra)
+            deque.append(contentsOf: extra)
+            expectEquivalentElements(deque, contents, by: ===)
+          }
+        }
+      }
+    }
+  }
+
   func test_insertOneElement() {
     withEveryDeque("deque", ofCapacities: [0, 1, 2, 3, 5, 10]) { layout in
       withEvery("offset", in: 0 ... layout.count) { offset in
