ZipX parsers should reset input to original with any throws in parser-builder-throws-2

[randomeizer]

For example, for ZipVV's parse method looks like this:

   @inlinable public func parse(_ input: inout P0.Input) rethrows -> () {
      do {
        try p0.parse(&input)
        try p1.parse(&input)
        return ()
      } catch {
        throw ParsingError.wrap(error, at: input)
      }
   }

Unfortunately, if p0 passes and p1 fails, whatever input was consumed successfully by p1 will remain consumed. It should be:

   @inlinable public func parse(_ input: inout P0.Input) rethrows -> () {
      let original = input
      do {
        try p0.parse(&input)
        try p1.parse(&input)
        return ()
      } catch {
        defer { input = original }
        throw ParsingError.wrap(error, at: input)
      }
   }

Same goes for all the other variations.

[randomeizer]

Pushed #107 as a suggested fix.

[stephencelis]

Timely! We were actually planning on changing backtracking this week and just opened this PR here: #108

[stephencelis]

@randomeizer Can you share here, or in #108, where Parse failing to backtrack was problematic for you? We're under the impression that all backtracking should be in a single place: OneOf. It might also be useful to have a Backtracking parser that enhances a failing parser with backtracking, but we haven't found a motivating example yet.

[randomeizer]

Yeah, no problem. The library I'm working on is open source, with the work on this branch here:

https://github.com/randomeizer/hsdoc/tree/parse-throws

In my case, it was using OneOf that was problematic, but it seemed like ZipX was the only built-in parser which wasn't backtracking correctly (that I'm using anyway). Specifically, I was having issues with this parser:

// Parses documentation comment prefixes, including a single optional space.
let docPrefix = Parse {
    OneOf {
        Parse { // ObjC
            "///"
            Not { "/" }
        }
        Parse { // Lua
            "---"
            Not { "-" }
        }
    }
    Skip { optionalSpace }
}

It would fail when given the following:

var input = "——" // four dashes
XCTAssertThrowsError(try docPrefix.parse(&input)) { error in
  XCTAssertEqual(...)
}
XCTAssertEqual(input, "——") // still four dashes

The first test passes (it comes back with "") but the second fails (input is actually "-" - one dash).

I did consider augmenting OneOf to do the backtracking instead, but it seemed like all the other parsers were cleaning up after themselves (including Many), so it made more sense to fix Parse in this case.

[randomeizer]

By the way, I've run my test cases against the updated parser-builder-throws-2 branch with the OneOf backtracking and they all passed with flying colours.

[stephencelis]

var input = "——" // four dashes
XCTAssertEqual(try docPrefix.parse(input: &input), "")
XCTAssertEqual(input, "——") // still four dashes

Did the formatting get messed up here? I'm seeing 2 em dash characters, not 4 dashes.

Is everything working for you now? This is the behavior I currently see on the throws branch, which is what I think I'd expect:

var input = "----"[...]
do { try docPrefix.parse(&input) } catch { print("\(error)") }
input

error: multiple failures occurred

error: unexpected input
 --> input:1:4
1 | ----
  |    ^ expected not to be processed

error: unexpected input
 --> input:1:1
1 | ----
  | ^ expected "///"

[randomeizer]

Yeah, my computer helpfully turned my double dashes in to emdashes, sorry about that.

And to confirm, the updated code from parser-builder-throws-2 is working correctly for me now.