Incorporate feedback from @lorentey and @moiseev

Author: numist

stdlib/public/core/CollectionDifference.swift

@@ -20,6 +20,7 @@ public struct CollectionDifference<ChangeElement> {
   /// each `remove` refers to the offset of its `element` in the original
   /// state. Non-`nil` values of `associatedWith` refer to the offset of the
   /// complementary change.
+  @_frozen
   public enum Change {
     case insert(offset: Int, element: ChangeElement, associatedWith: Int?)
     case remove(offset: Int, element: ChangeElement, associatedWith: Int?)
@@ -87,22 +88,22 @@ public struct CollectionDifference<ChangeElement> {
     var insertOffset = Set<Int>()
     var removeOffset = Set<Int>()
 
-    for c in changes {
-      let offset = c._offset
+    for change in changes {
+      let offset = change._offset
       if offset < 0 { return false }
 
-      switch c {
+      switch change {
       case .remove(_, _, _):
         if removeOffset.contains(offset) { return false }
         removeOffset.insert(offset)
       case .insert(_, _, _):
         if insertOffset.contains(offset) { return false }
         insertOffset.insert(offset)
-      }
+      } 
 
-      if let assoc = c._associatedOffset {
+      if let assoc = change._associatedOffset {
         if assoc < 0 { return false }
-        switch c {
+        switch change {
         case .remove(_, _, _):
           if removeOffsetToAssoc[offset] != nil { return false }
           removeOffsetToAssoc[offset] = assoc
@@ -137,7 +138,7 @@ public struct CollectionDifference<ChangeElement> {
   public init?<Changes: Collection>(
     _ changes: Changes
   ) where Changes.Element == Change {
-    if !CollectionDifference<ChangeElement>._validateChanges(changes) {
+    guard CollectionDifference<ChangeElement>._validateChanges(changes) else {
       return nil
     }
 
@@ -217,8 +218,10 @@ public struct CollectionDifference<ChangeElement> {
 extension CollectionDifference: Collection {
   public typealias Element = Change
 
-  public struct Index: Equatable, Hashable, Comparable {
+  @_fixed_layout
+  public struct Index {
     // Opaque index type is isomorphic to Int
+    @usableFromInline
     internal let _offset: Int
 
     internal init(_offset offset: Int) {
@@ -259,7 +262,19 @@ extension CollectionDifference: Collection {
 }
 
 @available(macOS 9999, iOS 9999, tvOS 9999, watchOS 9999, *) // FIXME(availability-5.1)
-extension CollectionDifference.Index { // Comparable
+extension CollectionDifference.Index: Equatable {
+  @inlinable
+  public static func == (
+    lhs: CollectionDifference.Index,
+    rhs: CollectionDifference.Index
+  ) -> Bool {
+    return lhs._offset == rhs._offset
+  }
+}
+
+@available(macOS 9999, iOS 9999, tvOS 9999, watchOS 9999, *) // FIXME(availability-5.1)
+extension CollectionDifference.Index: Comparable {
+  @inlinable
   public static func < (
     lhs: CollectionDifference.Index,
     rhs: CollectionDifference.Index
@@ -268,6 +283,14 @@ extension CollectionDifference.Index { // Comparable
   }
 }
 
+@available(macOS 9999, iOS 9999, tvOS 9999, watchOS 9999, *) // FIXME(availability-5.1)
+extension CollectionDifference.Index: Hashable {
+  @inlinable
+  public func hash(into hasher: inout Hasher) {
+    hasher.combine(_offset)
+  }
+}
+
 @available(macOS 9999, iOS 9999, tvOS 9999, watchOS 9999, *) // FIXME(availability-5.1)
 extension CollectionDifference.Change: Equatable where ChangeElement: Equatable {}
 
@@ -289,50 +312,50 @@ extension CollectionDifference where ChangeElement: Hashable {
   ///
   /// - Complexity: O(*n*) where *n* is `self.count`
   public func inferringMoves() -> CollectionDifference<ChangeElement> {
-    let removeDict: [ChangeElement:Int?] = {
-      var res = [ChangeElement:Int?](minimumCapacity: Swift.min(removals.count, insertions.count))
-      for r in removals {
-        let element = r._element
-        if res[element] != .none {
-          res[element] = .some(.none)
+    let uniqueRemovals: [ChangeElement:Int?] = {
+      var result = [ChangeElement:Int?](minimumCapacity: Swift.min(removals.count, insertions.count))
+      for removal in removals {
+        let element = removal._element
+        if result[element] != .none {
+          result[element] = .some(.none)
         } else {
-          res[element] = .some(r._offset)
+          result[element] = .some(removal._offset)
         }
       }
-      return res.filter { (_, v) -> Bool in v != .none }
+      return result.filter { (_, v) -> Bool in v != .none }
     }()
 
-    let insertDict: [ChangeElement:Int?] = {
-      var res = [ChangeElement:Int?](minimumCapacity: Swift.min(removals.count, insertions.count))
-      for i in insertions {
-        let element = i._element
-        if res[element] != .none {
-          res[element] = .some(.none)
+    let uniqueInsertions: [ChangeElement:Int?] = {
+      var result = [ChangeElement:Int?](minimumCapacity: Swift.min(removals.count, insertions.count))
+      for insertion in insertions {
+        let element = insertion._element
+        if result[element] != .none {
+          result[element] = .some(.none)
         } else {
-          res[element] = .some(i._offset)
+          result[element] = .some(insertion._offset)
         }
       }
-      return res.filter { (_, v) -> Bool in v != .none }
+      return result.filter { (_, v) -> Bool in v != .none }
     }()
 
-    return CollectionDifference(_validatedChanges: map({ (c: Change) -> Change in
-      switch c {
-      case .remove(offset: let o, element: let e, associatedWith: _):
-        if removeDict[e] == nil {
-          return c
+    return CollectionDifference(_validatedChanges: map({ (change: Change) -> Change in
+      switch change {
+      case .remove(offset: let offset, element: let element, associatedWith: _):
+        if uniqueRemovals[element] == nil {
+          return change
         }
-        if let assoc = insertDict[e] {
-          return .remove(offset: o, element: e, associatedWith: assoc)
+        if let assoc = uniqueInsertions[element] {
+          return .remove(offset: offset, element: element, associatedWith: assoc)
         }
-      case .insert(offset: let o, element: let e, associatedWith: _):
-        if insertDict[e] == nil {
-          return c
+      case .insert(offset: let offset, element: let element, associatedWith: _):
+        if uniqueInsertions[element] == nil {
+          return change
         }
-        if let assoc = removeDict[e] {
-          return .insert(offset: o, element: e, associatedWith: assoc)
+        if let assoc = uniqueRemovals[element] {
+          return .insert(offset: offset, element: element, associatedWith: assoc)
         }
       }
-      return c
+      return change
     }))
   }
 }

stdlib/public/core/Diffing.swift

@@ -75,7 +75,8 @@ extension RangeReplaceableCollection {
     func append(
       into target: inout Self,
       contentsOf source: Self,
-      from index: inout Self.Index, count: Int) {
+      from index: inout Self.Index, count: Int
+    ) {
       let start = index
       source.formIndex(&index, offsetBy: count)
       target.append(contentsOf: source[start..<index])
@@ -96,14 +97,14 @@ extension RangeReplaceableCollection {
         enumeratedOriginals += origCount
         enumeratedInserts += 1
       }
-      assert(enumeratedOriginals <= self.count)
+      _internalInvariant(enumeratedOriginals <= self.count)
     }
     let origCount = self.count - enumeratedOriginals
     append(into: &result, contentsOf: self, from: &currentIndex, count: origCount)
 
-    assert(currentIndex == self.endIndex)
-    assert(enumeratedOriginals + origCount == self.count)
-    assert(result.count == self.count + enumeratedInserts - enumeratedRemoves)
+    _internalInvariant(currentIndex == self.endIndex)
+    _internalInvariant(enumeratedOriginals + origCount == self.count)
+    _internalInvariant(result.count == self.count + enumeratedInserts - enumeratedRemoves)
     return result
   }
 }
@@ -133,8 +134,7 @@ extension BidirectionalCollection {
     from other: C,
     by areEquivalent: (Element, C.Element) -> Bool
   ) -> CollectionDifference<Element>
-  where C.Element == Self.Element
-  {
+  where C.Element == Self.Element {
     var rawChanges: [CollectionDifference<Element>.Change] = []
 
     let source = _CountingIndexCollection(other)
@@ -198,7 +198,8 @@ extension BidirectionalCollection {
   fileprivate func _commonPrefix<Other: BidirectionalCollection>(
     with other: Other,
     by areEquivalent: (Element, Other.Element) -> Bool
-  ) -> (SubSequence, Other.SubSequence) where Element == Other.Element {
+  ) -> (SubSequence, Other.SubSequence)
+  where Element == Other.Element {
     let (s1, s2) = (startIndex, other.startIndex)
     let (e1, e2) = (endIndex, other.endIndex)
     var (i1, i2) = (s1, s2)
@@ -319,7 +320,7 @@ extension _CollectionChanges: RandomAccessCollection {
   }
 
   fileprivate subscript(position: Index) -> Element {
-    precondition((startIndex..<endIndex).contains(position))
+    _internalInvariant((startIndex..<endIndex).contains(position))
 
     let current = pathStorage[position + pathStartIndex]
     let next = pathStorage[position + pathStartIndex + 1]
@@ -500,13 +501,13 @@ fileprivate struct _SearchState<
   /// diagonal `k`.
   fileprivate subscript(d: Int, k: Int) -> Endpoint {
     get {
-      assert((-d...d).contains(k))
-      assert((d + k) % 2 == 0)
+      _internalInvariant((-d...d).contains(k))
+      _internalInvariant((d + k) % 2 == 0)
       return endpoints[d, (d + k) / 2]
     }
     set {
-      assert((-d...d).contains(k))
-      assert((d + k) % 2 == 0)
+      _internalInvariant((-d...d).contains(k))
+      _internalInvariant((d + k) % 2 == 0)
       endpoints[d, (d + k) / 2] = newValue
     }
   }
@@ -582,7 +583,7 @@ extension _SearchState {
     // *d* is the minimal number of inserted and removed elements). If there
     // are no consecutive removes or inserts and every remove or insert is
     // sandwiched between matches, the path will need `2 + d * 2` elements.
-    assert(d > 0, "Must be at least one difference between `a` and `b`")
+    _internalInvariant(d > 0, "Must be at least one difference between `a` and `b`")
     if d == 1 {
       endpoints.storage.append(pathEnd)
     }
@@ -737,11 +738,11 @@ fileprivate struct _LowerTriangularMatrix<Element> {
 
   fileprivate subscript(row: Int, column: Int) -> Element {
     get {
-      assert((0...row).contains(column))
+      _internalInvariant((0...row).contains(column))
       return storage[_triangularNumber(row) + column]
     }
     set {
-      assert((0...row).contains(column))
+      _internalInvariant((0...row).contains(column))
       storage[_triangularNumber(row) + column] = newValue
     }
   }

test/stdlib/Diffing.swift

@@ -336,7 +336,7 @@ suite.test("Basic diffing algorithm validators") {
 
   for (source, target, expected, line) in expectedChanges {
     let actual = target.difference(from: source).inferringMoves()
-    expectEqual(actual, CollectionDifference(expected), "failed test at line \(line)")
+    expectEqual(CollectionDifference(expected), actual, "failed test at line \(line)")
   }
 }
 
@@ -444,7 +444,7 @@ suite.test("Enumeration order is safe") {
   diff.forEach { c in
     enumerationOrderedChanges.append(c)
   }
-  expectEqual(safelyOrderedChanges, enumerationOrderedChanges)
+  expectEqual(enumerationOrderedChanges, safelyOrderedChanges)
 }
 
 suite.test("Change validator rejects bad associations") {
@@ -546,15 +546,10 @@ suite.test("Move inference") {
   expectEqual(w, n?.inferringMoves())
 }
 
-suite.test("Three way diff demo code") {
-  let base = "Is\nit\ntime\nalready?"
-  let theirs = "Hi\nthere\nis\nit\ntime\nalready?"
-  let mine = "Is\nit\nreview\ntime\nalready?"
-
-  // Split the contents of the sources into lines
-  let baseLines = base.components(separatedBy: "\n")
-  let theirLines = theirs.components(separatedBy: "\n")
-  let myLines = mine.components(separatedBy: "\n")
+suite.test("Three way merge") {
+  let baseLines = ["Is", "it", "time", "already?"]
+  let theirLines = ["Hi", "there", "is", "it", "time", "already?"]
+  let myLines = ["Is", "it", "review", "time", "already?"]
 
   // Create a difference from base to theirs
   let diff = theirLines.difference(from: baseLines)
@@ -566,8 +561,7 @@ suite.test("Three way diff demo code") {
   }
 
   // Reassemble the result
-  let patched = patchedLines.joined(separator: "\n")
-  expectEqual(patched, "Hi\nthere\nis\nit\nreview\ntime\nalready?")
+  expectEqual(patchedLines, ["Hi", "there", "is", "it", "review", "time", "already?"])
   // print(patched)
 }
 
@@ -586,12 +580,8 @@ suite.test("Diff reversal demo code") {
 }
 
 suite.test("Naive application by enumeration") {
-  let base = "Is\nit\ntime\nalready?"
-  let theirs = "Hi\nthere\nis\nit\ntime\nalready?"
-
-  // Split the contents of the sources into lines
-  var arr = base.components(separatedBy: "\n")
-  let theirLines = theirs.components(separatedBy: "\n")
+  var arr = ["Is", "it", "time", "already?"]
+  let theirLines = ["Hi", "there", "is", "it", "time", "already?"]
 
   // Create a difference from base to theirs
   let diff = theirLines.difference(from: arr)
@@ -605,7 +595,7 @@ suite.test("Naive application by enumeration") {
     }
   }
 
-  expectEqual(arr, theirLines)
+  expectEqual(theirLines, arr)
 }
 
 suite.test("Fast applicator boundary conditions") {