Merge pull request #26575 from numist/numist/application-denied

Return nil on applying() failure instead of crashing

Author: numist

stdlib/public/core/Diffing.swift

@@ -15,8 +15,8 @@
 @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
 extension CollectionDifference {
   fileprivate func _fastEnumeratedApply(
-    _ consume: (Change) -> Void
-  ) {
+    _ consume: (Change) throws -> Void
+  ) rethrows {
     let totalRemoves = removals.count
     let totalInserts = insertions.count
     var enumeratedRemoves = 0
@@ -41,7 +41,7 @@ extension CollectionDifference {
         preconditionFailure()
       }
 
-      consume(change)
+      try consume(change)
 
       switch change {
       case .remove(_, _, _):
@@ -53,6 +53,9 @@ extension CollectionDifference {
   }
 }
 
+// Error type allows the use of throw to unroll state on application failure
+private enum _ApplicationError : Error { case failed }
+
 extension RangeReplaceableCollection {
   /// Applies the given difference to this collection.
   ///
@@ -66,45 +69,54 @@ extension RangeReplaceableCollection {
   ///   number of changes contained by the parameter.
   @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
   public func applying(_ difference: CollectionDifference<Element>) -> Self? {
-    var result = Self()
-    var enumeratedRemoves = 0
-    var enumeratedInserts = 0
-    var enumeratedOriginals = 0
-    var currentIndex = self.startIndex
 
     func append(
       into target: inout Self,
       contentsOf source: Self,
       from index: inout Self.Index, count: Int
-    ) {
+    ) throws {
       let start = index
-      source.formIndex(&index, offsetBy: count)
+      if !source.formIndex(&index, offsetBy: count, limitedBy: source.endIndex) {
+        throw _ApplicationError.failed
+      }
       target.append(contentsOf: source[start..<index])
     }
 
-    difference._fastEnumeratedApply { change in
-      switch change {
-      case .remove(offset: let offset, element: _, associatedWith: _):
-        let origCount = offset - enumeratedOriginals
-        append(into: &result, contentsOf: self, from: &currentIndex, count: origCount)
-        enumeratedOriginals += origCount + 1 // Removal consumes an original element
-        currentIndex = self.index(after: currentIndex)
-        enumeratedRemoves += 1
-      case .insert(offset: let offset, element: let element, associatedWith: _):
-        let origCount = (offset + enumeratedRemoves - enumeratedInserts) - enumeratedOriginals
-        append(into: &result, contentsOf: self, from: &currentIndex, count: origCount)
-        result.append(element)
-        enumeratedOriginals += origCount
-        enumeratedInserts += 1
+    var result = Self()
+    do {
+      var enumeratedRemoves = 0
+      var enumeratedInserts = 0
+      var enumeratedOriginals = 0
+      var currentIndex = self.startIndex
+      try difference._fastEnumeratedApply { change in
+        switch change {
+        case .remove(offset: let offset, element: _, associatedWith: _):
+          let origCount = offset - enumeratedOriginals
+          try append(into: &result, contentsOf: self, from: &currentIndex, count: origCount)
+          if currentIndex == self.endIndex {
+            // Removing nonexistent element off the end of the collection
+            throw _ApplicationError.failed
+          }
+          enumeratedOriginals += origCount + 1 // Removal consumes an original element
+          currentIndex = self.index(after: currentIndex)
+          enumeratedRemoves += 1
+        case .insert(offset: let offset, element: let element, associatedWith: _):
+          let origCount = (offset + enumeratedRemoves - enumeratedInserts) - enumeratedOriginals
+          try append(into: &result, contentsOf: self, from: &currentIndex, count: origCount)
+          result.append(element)
+          enumeratedOriginals += origCount
+          enumeratedInserts += 1
+        }
+        _internalInvariant(enumeratedOriginals <= self.count)
+      }
+      if currentIndex < self.endIndex {
+        result.append(contentsOf: self[currentIndex...])
       }
-      _internalInvariant(enumeratedOriginals <= self.count)
+      _internalInvariant(result.count == self.count + enumeratedInserts - enumeratedRemoves)
+    } catch {
+      return nil
     }
-    let origCount = self.count - enumeratedOriginals
-    append(into: &result, contentsOf: self, from: &currentIndex, count: origCount)
 
-    _internalInvariant(currentIndex == self.endIndex)
-    _internalInvariant(enumeratedOriginals + origCount == self.count)
-    _internalInvariant(result.count == self.count + enumeratedInserts - enumeratedRemoves)
     return result
   }
 }

test/stdlib/Diffing.swift

@@ -629,6 +629,68 @@ if #available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) {
               }}}}}}
   }
 
+  suite.test("Fast applicator error condition") {
+    let bear = "bear"
+    let bird = "bird"
+    let bat = "bat"
+
+    let diff = bird.difference(from: bear)
+
+    expectEqual(nil, bat.applying(diff))
+  }
+
+  suite.test("Fast applicator boundary condition remove last element") {
+    let base = [1, 2, 3]
+
+    expectEqual([1, 2], base.applying(CollectionDifference<Int>([.remove(offset: base.count - 1, element: 3, associatedWith: nil)])!))
+  }
+
+  suite.test("Fast applicator boundary condition append element") {
+    let base = [1, 2, 3]
+
+    expectEqual([1, 2, 3, 4], base.applying(CollectionDifference<Int>([.insert(offset: base.count, element: 4, associatedWith: nil)])!))
+  }
+
+  suite.test("Fast applicator error boundary condition remove at endIndex") {
+    let base = [1, 2, 3]
+
+    expectEqual(nil, base.applying(CollectionDifference<Int>([.remove(offset: base.count, element: 4, associatedWith: nil)])!))
+  }
+
+  suite.test("Fast applicator error boundary condition insert beyond end") {
+    let base = [1, 2, 3]
+
+    expectEqual(nil, base.applying(CollectionDifference<Int>([.insert(offset: base.count + 1, element: 5, associatedWith: nil)])!))
+  }
+
+  suite.test("Fast applicator boundary condition replace tail") {
+    let base = [1, 2, 3]
+
+    expectEqual([1, 2, 4], base.applying(CollectionDifference<Int>([
+      .remove(offset: base.count - 1, element: 3, associatedWith: nil),
+      .insert(offset: base.count - 1, element: 4, associatedWith: nil)
+    ])!))
+  }
+
+  suite.test("Fast applicator error boundary condition insert beyond end after tail removal") {
+    let base = [1, 2, 3]
+
+    expectEqual(nil, base.applying(CollectionDifference<Int>([
+      .remove(offset: base.count - 1, element: 3, associatedWith: nil),
+      .insert(offset: base.count, element: 4, associatedWith: nil)
+    ])!))
+
+  }
+
+  suite.test("Fast applicator error boundary condition insert beyond end after non-tail removal") {
+    let base = [1, 2, 3]
+
+    expectEqual(nil, base.applying(CollectionDifference<Int>([
+      .remove(offset: base.count - 2, element: 2, associatedWith: nil),
+      .insert(offset: base.count, element: 4, associatedWith: nil)
+    ])!))
+  }
+
   suite.test("Fast applicator fuzzer") {
     func makeArray() -> [Int] {
       var arr = [Int]()