Fix firstRange(of:) search (#656)

Calls to `ranges(of:)` and `firstRange(of:)` with a string parameter
actually use two different string searching algorithms. `ranges(of:)`
uses the "z-searcher" algorithm, while `firstRange(of:)` uses a
two-way search. Since it's better to align on a single path for these
searches, the z-searcher has lower requirements, and the two-way
search implementation has a correctness bug, this change removes
the two-way search algorithm and uses z-search for `firstRange(of:)`.

The correctness bug in `firstRange(of:)` appears only when searching
for the second (or later) occurrence of a substring, which you have
to be fairly deliberate about. In the example below, the substring
at offsets `7..<12` is missed:

    let text = "ADACBADADACBADACB"
    //          =====  -----=====
    let pattern = "ADACB"
    let firstRange = text.firstRange(of: pattern)!
    // firstRange ~= 0..<5
    let secondRange = text[firstRange.upperBound...].firstRange(of: pattern)!
    // secondRange ~= 12..<17

This change also removes some unrelated, unused code in Split.swift,
in addition to removing an (unused) usage of `TwoWaySearcher`.

rdar://92794248

Author: natecook1000

Sources/_StringProcessing/Algorithms/Algorithms/FirstRange.swift

@@ -50,11 +50,8 @@ extension BidirectionalCollection where Element: Comparable {
   public func firstRange<C: Collection>(
     of other: C
   ) -> Range<Index>? where C.Element == Element {
-    let searcher = PatternOrEmpty(
-      searcher: TwoWaySearcher<SubSequence>(pattern: Array(other)))
-    let slice = self[...]
-    var state = searcher.state(for: slice, in: startIndex..<endIndex)
-    return searcher.search(slice, &state)
+    let searcher = ZSearcher<SubSequence>(pattern: Array(other), by: ==)
+    return searcher.search(self[...], in: startIndex..<endIndex)
   }
 }
 

Sources/_StringProcessing/Algorithms/Algorithms/Split.swift

@@ -122,31 +122,6 @@ extension Collection {
   }
 }
 
-// MARK: Predicate algorithms
-
-extension Collection {
-  // TODO: Non-escaping and throwing
-  func split(
-    whereSeparator predicate: @escaping (Element) -> Bool,
-    maxSplits: Int,
-    omittingEmptySubsequences: Bool
-  ) -> SplitCollection<PredicateConsumer<Self>> {
-    split(by: PredicateConsumer(predicate: predicate), maxSplits: maxSplits, omittingEmptySubsequences: omittingEmptySubsequences)
-  }
-}
-
-// MARK: Single element algorithms
-
-extension Collection where Element: Equatable {
-  func split(
-    by separator: Element,
-    maxSplits: Int,
-    omittingEmptySubsequences: Bool
-  ) -> SplitCollection<PredicateConsumer<Self>> {
-    split(whereSeparator: { $0 == separator }, maxSplits: maxSplits, omittingEmptySubsequences: omittingEmptySubsequences)
-  }
-}
-
 // MARK: Fixed pattern algorithms
 
 extension Collection where Element: Equatable {
@@ -180,41 +155,6 @@ extension Collection where Element: Equatable {
   }
 }
 
-extension BidirectionalCollection where Element: Equatable {
-  // FIXME
-//  public func splitFromBack<S: Sequence>(
-//    separator: S
-//  ) -> ReversedSplitCollection<ZSearcher<SubSequence>>
-//    where S.Element == Element
-//  {
-//    splitFromBack(separator: ZSearcher(pattern: Array(separator), by: ==))
-//  }
-}
-
-extension BidirectionalCollection where Element: Comparable {
-  func split<C: Collection>(
-    by separator: C,
-    maxSplits: Int,
-    omittingEmptySubsequences: Bool
-  ) -> SplitCollection<PatternOrEmpty<TwoWaySearcher<Self>>>
-    where C.Element == Element
-  {
-    split(
-      by: PatternOrEmpty(searcher: TwoWaySearcher(pattern: Array(separator))),
-    maxSplits: maxSplits, omittingEmptySubsequences: omittingEmptySubsequences)
-  }
-  
-  // FIXME
-//  public func splitFromBack<S: Sequence>(
-//    separator: S
-//  ) -> ReversedSplitCollection<PatternOrEmpty<TwoWaySearcher<SubSequence>>>
-//    where S.Element == Element
-//  {
-//    splitFromBack(separator: PatternOrEmpty(
-//      searcher: TwoWaySearcher(pattern: Array(separator))))
-//  }
-}
-
 // String split overload breakers
 //
 // These are underscored and marked as SPI so that the *actual* public overloads

Sources/_StringProcessing/Algorithms/Searchers/TwoWaySearcher.swift

@@ -23,7 +23,6 @@ add_library(_StringProcessing
   Algorithms/Searchers/NaivePatternSearcher.swift
   Algorithms/Searchers/PatternOrEmpty.swift
   Algorithms/Searchers/PredicateSearcher.swift
-  Algorithms/Searchers/TwoWaySearcher.swift
   Algorithms/Searchers/ZSearcher.swift
   Engine/Backtracking.swift
   Engine/Consume.swift

Sources/_StringProcessing/CMakeLists.txt

@@ -165,8 +165,12 @@ class AlgorithmTests: XCTestCase {
       let actualCol: [Range<Int>] = input.ranges(of: pattern)[...].map(input.offsets(of:))
       XCTAssertEqual(actualCol, expected, file: file, line: line)
 
-      let firstRange = input.firstRange(of: pattern).map(input.offsets(of:))
-      XCTAssertEqual(firstRange, expected.first, file: file, line: line)
+      let firstRange = input.firstRange(of: pattern)
+      XCTAssertEqual(firstRange.map(input.offsets(of:)), expected.first, file: file, line: line)
+      if let upperBound = firstRange?.upperBound, !pattern.isEmpty {
+        let secondRange = input[upperBound...].firstRange(of: pattern).map(input.offsets(of:))
+        XCTAssertEqual(secondRange, expected.dropFirst().first, file: file, line: line)
+      }
     }
 
     expectRanges("", "", [0..<0])
@@ -176,6 +180,9 @@ class AlgorithmTests: XCTestCase {
     expectRanges("abcde", "bcd", [1..<4])
     expectRanges("ababacabababa", "abababa", [6..<13])
     expectRanges("ababacabababa", "aba", [0..<3, 6..<9, 10..<13])
+    
+    // Test for rdar://92794248
+    expectRanges("ADACBADADACBADACB", "ADACB", [0..<5, 7..<12, 12..<17])
   }
 
   // rdar://105154010