Various fixes and improvements

Author: JacobHearst

Sources/_StringProcessing/ByteCodeGen.swift

@@ -148,7 +148,6 @@ fileprivate extension Compiler.ByteCodeGen {
 
     // Fast path for eliding boundary checks for an all ascii quoted literal
     if optimizationsEnabled && s.allSatisfy(\.isASCII) && !s.isEmpty {
-      builder.buildReverseUnicodeScalar(1)
       let lastIdx = s.unicodeScalars.indices.first!
       for idx in s.unicodeScalars.indices.reversed() {
         let boundaryCheck = idx == lastIdx
@@ -162,7 +161,6 @@ fileprivate extension Compiler.ByteCodeGen {
       return
     }
 
-    builder.buildReverse(1)
     for c in s.reversed() { emitCharacter(c) }
   }
 
@@ -223,7 +221,6 @@ fileprivate extension Compiler.ByteCodeGen {
 
   mutating func emitReverseMatchScalar(_ s: UnicodeScalar) {
     assert(options.semanticLevel == .unicodeScalar)
-    builder.buildReverseUnicodeScalar(1)
     if options.isCaseInsensitive && s.properties.isCased {
       builder.buildReverseMatchScalarCaseInsensitive(s, boundaryCheck: false)
     } else {
@@ -254,7 +251,6 @@ fileprivate extension Compiler.ByteCodeGen {
           boundaryCheck: true)
       } else {
         if reverse {
-          builder.buildReverse(1)
           builder.buildReverseMatch(c, isCaseInsensitive: true)
         } else {
           builder.buildMatch(c, isCaseInsensitive: true)
@@ -269,7 +265,6 @@ fileprivate extension Compiler.ByteCodeGen {
         let scalar = c.unicodeScalars[idx]
         let boundaryCheck = idx == lastIdx
         if reverse {
-          builder.buildReverseUnicodeScalar(1)
           builder.buildReverseMatchScalar(scalar, boundaryCheck: boundaryCheck)
         } else {
           builder.buildMatchScalar(scalar, boundaryCheck: boundaryCheck)
@@ -279,7 +274,6 @@ fileprivate extension Compiler.ByteCodeGen {
     }
 
     if reverse {
-      builder.buildReverse(1)
       builder.buildReverseMatch(c, isCaseInsensitive: false)
     } else {
       builder.buildMatch(c, isCaseInsensitive: false)
@@ -388,6 +382,9 @@ fileprivate extension Compiler.ByteCodeGen {
 
     builder.buildSave(success)
     builder.buildSave(intercept)
+    if reverse {
+       builder.buildReverse(1)
+    }
     try emitNode(child)
     builder.buildClearThrough(intercept)
     builder.buildFail(preservingCaptures: true) // Lookahead succeeds here
@@ -417,6 +414,9 @@ fileprivate extension Compiler.ByteCodeGen {
 
     builder.buildSave(success)
     builder.buildSave(intercept)
+    if reverse {
+      builder.buildReverse(1)
+    }
     try emitNode(child)
     builder.buildClearThrough(intercept)
     builder.buildClear()
@@ -795,7 +795,6 @@ fileprivate extension Compiler.ByteCodeGen {
         return false
       }
       if reverse {
-        builder.buildReverse(1)
         builder.buildReverseQuantify(bitset: bitset, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
       } else {
         builder.buildQuantify(bitset: bitset, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
@@ -809,15 +808,13 @@ fileprivate extension Compiler.ByteCodeGen {
           return false
         }
         if reverse {
-          builder.buildReverse(1)
           builder.buildReverseQuantify(asciiChar: val, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
         } else {
           builder.buildQuantify(asciiChar: val, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
         }
 
       case .any:
         if reverse {
-          builder.buildReverse(1)
           builder.buildReverseQuantifyAny(
             matchesNewlines: true, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
         } else {
@@ -826,7 +823,6 @@ fileprivate extension Compiler.ByteCodeGen {
         }
       case .anyNonNewline:
         if reverse {
-          builder.buildReverse(1)
           builder.buildReverseQuantifyAny(
             matchesNewlines: false, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
         } else {
@@ -835,7 +831,6 @@ fileprivate extension Compiler.ByteCodeGen {
         }
       case .dot:
         if reverse {
-          builder.buildReverse(1)
           builder.buildReverseQuantifyAny(
             matchesNewlines: options.dotMatchesNewline, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
         } else {
@@ -847,7 +842,6 @@ fileprivate extension Compiler.ByteCodeGen {
         // Custom character class that consumes a single grapheme
         let model = cc.asRuntimeModel(options)
         if reverse {
-          builder.buildReverse(1)
           builder.buildReverseQuantify(
             model: model,
             kind,
@@ -914,7 +908,6 @@ fileprivate extension Compiler.ByteCodeGen {
       guard let bitset = ccc.asAsciiBitset(options) else {
         return false
       }
-      builder.buildReverse(1)
       builder.buildReverseQuantify(bitset: bitset, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
 
     case .atom(let atom):
@@ -924,26 +917,21 @@ fileprivate extension Compiler.ByteCodeGen {
         guard let val = c._singleScalarAsciiValue else {
           return false
         }
-        builder.buildReverse(1)
         builder.buildReverseQuantify(asciiChar: val, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
 
       case .any:
-        builder.buildReverse(1)
         builder.buildReverseQuantifyAny(
           matchesNewlines: true, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
       case .anyNonNewline:
-        builder.buildReverse(1)
         builder.buildReverseQuantifyAny(
           matchesNewlines: false, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
       case .dot:
-        builder.buildReverse(1)
         builder.buildReverseQuantifyAny(
           matchesNewlines: options.dotMatchesNewline, kind, minTrips, maxExtraTrips, isScalarSemantics: isScalarSemantics)
 
       case .characterClass(let cc):
         // Custom character class that consumes a single grapheme
         let model = cc.asRuntimeModel(options)
-        builder.buildReverse(1)
         builder.buildReverseQuantify(
           model: model,
           kind,

Sources/_StringProcessing/Engine/Instruction.swift

@@ -262,7 +262,7 @@ extension Instruction {
     /// Quantifies the stored instruction in an inner loop instead of looping through instructions in processor
     /// Only quantifies specific nodes
     ///
-    ///     quantify(_:QuantifyPayload)
+    ///     reverseQuantify(_:QuantifyPayload)
     ///
     case reverseQuantify
     /// Begin the given capture

Sources/_StringProcessing/Engine/MEBuilder.swift

@@ -171,10 +171,18 @@ extension MEProgram.Builder {
     instructions.append(.init(.matchAnyNonNewline, .init(isScalar: false)))
   }
 
+  mutating func buildReverseConsumeNonNewline() {
+    instructions.append(.init(.reverseMatchAnyNonNewline, .init(isScalar: false)))
+  }
+
   mutating func buildConsumeScalarNonNewline() {
     instructions.append(.init(.matchAnyNonNewline, .init(isScalar: true)))
   }
 
+  mutating func buildReverseConsumeScalarNonNewline() {
+    instructions.append(.init(.reverseMatchAnyNonNewline, .init(isScalar: true)))
+  }
+
   mutating func buildMatch(_ e: Character, isCaseInsensitive: Bool) {
     instructions.append(.init(
       .match, .init(element: elements.store(e), isCaseInsensitive: isCaseInsensitive)))

Sources/_StringProcessing/Engine/MEBuiltins.swift

@@ -186,34 +186,34 @@ extension String {
   /// boundary of the returned character.
   ///
   /// This function handles loading a character from a string while respecting
-  /// an end boundary, even if that end boundary is sub-character or sub-scalar.
+  /// an start boundary, even if that start boundary is sub-character or sub-scalar.
   ///
-  ///   - If `pos` is at or past `end`, this function returns `nil`.
-  ///   - If `end` is between `pos` and the next grapheme cluster boundary (i.e.,
-  ///     `end` is before `self.index(after: pos)`, then the returned character
+  ///   - If `pos` is at or past `start`, this function returns `nil`.
+  ///   - If `start` is between `pos` and the next grapheme cluster boundary (i.e.,
+  ///     `start` is before `self.index(after: pos)`, then the returned character
   ///     is smaller than the one that would be produced by `self[pos]` and the
-  ///     returned index is at the end of that character.
-  ///   - If `end` is between `pos` and the next grapheme cluster boundary, and
+  ///     returned index is at the start of that character.
+  ///   - If `start` is between `pos` and the next grapheme cluster boundary, and
   ///     is not on a Unicode scalar boundary, the partial scalar is dropped. This
   ///     can result in a `nil` return or a character that includes only part of
   ///     the `self[pos]` character.
   ///
   /// - Parameters:
   ///   - pos: The position to load a character from.
-  ///   - end: The limit for the character at `pos`.
-  /// - Returns: The character at `pos`, bounded by `end`, if it exists, along
+  ///   - start: The limit for the character at `pos`.
+  /// - Returns: The character at `pos`, bounded by `start`, if it exists, along
   ///   with the upper bound of that character. The upper bound is always
   ///   scalar-aligned.
   func characterAndStart(at pos: String.Index, limitedBy start: String.Index) -> (Character, String.Index)? {
     // FIXME: Sink into the stdlib to avoid multiple boundary calculations
     guard pos > start else { return nil }
-    let next = index(before: pos)
-    if next >= start {
-      return (self[pos], next)
+    let previous = index(before: pos)
+    if previous >= start {
+      return (self[pos], previous)
     }
 
-    // `end` must be a sub-character position that is between `pos` and the
-    // next grapheme boundary. This is okay if `end` is on a Unicode scalar
+    // `start` must be a sub-character position that is between `pos` and the
+    // next grapheme boundary. This is okay if `start` is on a Unicode scalar
     // boundary, but if it's in the middle of a scalar's code units, there
     // may not be a character to return at all after rounding down. Use
     // `Substring`'s rounding to determine what we can return.
@@ -338,7 +338,7 @@ extension String {
       guard currentPosition >= start else { return nil }
       let scalar = unicodeScalars[currentPosition]
       guard !scalar.isNewline else { return nil }
-      return unicodeScalars.index(after: currentPosition)
+      return unicodeScalars.index(before: currentPosition)
     }
 
     guard let (char, previous) = characterAndStart(at: currentPosition, limitedBy: start),

Sources/_StringProcessing/Engine/MEReverseQuantify.swift

@@ -44,7 +44,7 @@ extension Processor {
     }
   }
 
-  /// Generic quantify instruction interpreter
+  /// Generic bounded reverseQuantify instruction interpreter
   /// - Handles .eager and .posessive
   /// - Handles arbitrary minTrips and maxExtraTrips
   mutating func runReverseQuantify(_ payload: QuantifyPayload) -> Bool {
@@ -62,11 +62,14 @@ extension Processor {
       trips += 1
     }
 
+    // If we don't have any more trips to take:
     if maxExtraTrips == 0 {
       // We're done
       return true
     }
 
+    // We've already consumed the minimum number of characters,
+    // If we can't get another match, the reverse quantify was successful
     guard let previous = _doReverseQuantifyMatch(payload) else {
       return true
     }
@@ -118,12 +121,12 @@ extension Processor {
 
     // Create a quantified save point for every part of the input matched up
     // to the final position.
-    let rangeStart = currentPosition
-    var rangeEnd = currentPosition
+    var rangeStart = currentPosition
+    let rangeEnd = currentPosition
     currentPosition = previous
     while true {
       guard let previous = _doReverseQuantifyMatch(payload) else { break }
-      rangeEnd = currentPosition
+      rangeStart = currentPosition
       currentPosition = previous
     }
 

Sources/_StringProcessing/Engine/Processor.swift

@@ -301,16 +301,16 @@ extension Processor {
   mutating func reverseMatch(
     _ e: Element, isCaseInsensitive: Bool
   ) -> Bool {
-    guard let next = input.match(
+    guard let previous = input.reverseMatch(
       e,
       at: currentPosition,
-      limitedBy: end,
+      limitedBy: start,
       isCaseInsensitive: isCaseInsensitive
     ) else {
       signalFailure()
       return false
     }
-    currentPosition = next
+    currentPosition = previous
     return true
   }
 
@@ -358,7 +358,7 @@ extension Processor {
     boundaryCheck: Bool,
     isCaseInsensitive: Bool
   ) -> Bool {
-    guard let next = input.reverseMatchScalar(
+    guard let previous = input.reverseMatchScalar(
       s,
       at: currentPosition,
       limitedBy: start,
@@ -368,7 +368,7 @@ extension Processor {
       signalFailure()
       return false
     }
-    currentPosition = next
+    currentPosition = previous
     return true
   }
 
@@ -474,10 +474,6 @@ extension Processor {
 
     controller.pc = pc
     currentPosition = pos ?? currentPosition
-//    if (input.startIndex..<input.endIndex).contains(currentPosition) {
-//      print("Restoring to: \(input[currentPosition]), \(pc)")
-//      print("Remaining save points: \(savePoints.map(\.destructure.pc))")
-//    }
     callStack.removeLast(callStack.count - stackEnd.rawValue)
     registers.ints = intRegisters
     registers.positions = posRegisters
@@ -819,18 +815,6 @@ extension Processor {
       storedCaptures[capNum].registerValue(value)
       controller.step()
     }
-
-//    print("==== State after executing \(instructions[controller.pc].description)")
-//    print("Pos: \(input.distance(from: input.startIndex, to: currentPosition))")
-//    print("Inst: \(currentPC)")
-//    let savePointsDescription = savePoints.map { sp in
-//      let posDescription = if let pos = sp.pos {
-//        input.distance(from: input.startIndex, to: pos).description
-//      } else { "" }
-//
-//      return "Pos: \(posDescription), Inst: \(sp.pc)"
-//    }
-//    print("Save points: \(savePointsDescription)")
   }
 }
 
@@ -984,7 +968,7 @@ extension String {
     // meanings in some code paths.
     let isInverted = bitset.isInverted
 
-    // TODO: More fodder for refactoring `_quickASCIICharacter`, see the comment 
+    // TODO: More fodder for refactoring `_quickASCIICharacter`, see the comment
     // there
     guard let (asciiByte, next, isCRLF) = _quickASCIICharacter(
       at: pos,

Sources/_StringProcessing/Engine/Tracing.swift

@@ -65,16 +65,33 @@ extension Instruction: CustomStringConvertible {
       } else {
         return "match char[\(reg)]"
       }
+    case .reverseMatch:
+      let (isCaseInsensitive, reg) = payload.elementPayload
+      if isCaseInsensitive {
+        return "reverseMatchCaseInsensitive char[\(reg)]"
+      } else {
+        return "reverseMatch char[\(reg)]"
+      }
     case .matchBitset:
       let (isScalar, reg) = payload.bitsetPayload
       if isScalar {
         return "matchBitsetScalar bitset[\(reg)]"
       } else {
         return "matchBitset bitset[\(reg)]"
       }
+    case .reverseMatchBitset:
+      let (isScalar, reg) = payload.bitsetPayload
+      if isScalar {
+        return "reverseMatchBitsetScalar bitset[\(reg)]"
+      } else {
+        return "reverseMatchBitset bitset[\(reg)]"
+      }
     case .matchBuiltin:
       let payload = payload.characterClassPayload
       return "matchBuiltin \(payload.cc) (\(payload.isInverted))"
+    case .reverseMatchBuiltin:
+      let payload = payload.characterClassPayload
+      return "\(opcode) \(payload.cc) (\(payload.isInverted))"
     case .matchBy:
       let (matcherReg, valReg) = payload.pairedMatcherValue
       return "\(opcode) match[\(matcherReg)] -> val[\(valReg)]"
@@ -101,6 +118,9 @@ extension Instruction: CustomStringConvertible {
     case .quantify:
       let payload = payload.quantify
       return "\(opcode) \(payload.type) \(payload.minTrips) \(payload.maxExtraTrips?.description ?? "unbounded" )"
+    case .reverseQuantify:
+      let payload = payload.quantify
+      return "\(opcode) \(payload.type) \(payload.minTrips) \(payload.maxExtraTrips?.description ?? "unbounded" )"
     case .save:
       let resumeAddr = payload.addr
       return "\(opcode) \(resumeAddr)"