lookAhead and error propagation

[redneb]

lookAhead p is a parser that calls p and if it succeeds then lookAhead p succeeds too, but it also rewinds the input stream back to where it was before p was tried. This was implemented in 3839639 by @feuerbach. I claim that this is not enough, lookAhead p should also discard any error messages generated by p. This is best illustrated with an example. Suppose that we have the following:

p = lookAhead (many1 $ char 'a') >> char 'b'

Then parse p "" "a" should fail with the following error message:

Left (line 1, column 1):
unexpected "a"
expecting "b"

but currently it fails with the following:

Left (line 1, column 2):
unexpected end of input
expecting "a"

In other words, if p succeeds, then lookAhead p should behave as if p never occurred, both in terms of the parser state and in terms of the errors generated.

[aslatter]

Just to make sure I'm not being dense - the parser p in your example could never succeed with any input, correct?

[redneb]

Correct. I guess the example is a bit contrived, but my point is that p should report correctly why it failed. In this instance, many1 $ char 'a' is tried first and succeeds, then the input stream is rewound back to the beginning and then char 'b' is tried which fails because it didn't find an 'b'. But the error message does not indicate that.

Here's another example:

variable :: Parser String
variable = do
    x <- lookAhead (many1 letter)
    if x == "return"
        then fail "'return' is a reserved keyword"
        else string x

Then parse p "" "return" currently fails with

Left (line 1, column 7):
unexpected end of input
expecting letter

With my patch it gives the following error:

Left (line 1, column 1):
return is a reserved keyword

The problem is that many1 letter succeeds having consumed the 6 letters of "returned". But it also generates an message about the end of input when many1 tries to apply letter for a seventh time. Then the input stream is rewound by lookAhead, and fail is called on the beginning of input. But parserBind ignores that error because it prefers the error that occurred later in the input. I claim that this error is irrelevant here because we have rewound the input stream.

[aslatter]

Oh, yes, I agree with you that your patch makes sense. I just wanted to make sure I understood your example.

If at all possible lookAhead should not update our parser-state when it succeeds.

[aslatter]

Fixed in f7faa2a.