Zero copy separatedby

[Zij-IT]

Thanks to your assistance with getting the parser to compile, I was able to give it a run and found that despite having changed none of the parsing logic itself, my Lua parser couldn't correctly parse some of the previous test items, such as the following:

x, y = 2, 3

The error was that the token representing 3 wasn't expected... which is odd because both the left and the right are basically comma separated lists, so it should have failed on y. I realized a subtle difference though:

The lhs was parsed effectively with this:

just(ident).then(just(comma).ignore_then(ident).repeated())

And the rhs was parsed with this:

expr.separated_by(comma).at_least(1)

I also noticed that this: x = 2, would successfully parse without having set allow_trailing. Since these should (I believe) always parse the same input to the same end result, I looked into how parsing for separated was done, and found this subtle bug:

match self.parser.go::<M>(inp) {
    Ok(out) => {
        /* snip */

        let before_separator = inp.save();
        if let Err(e) = self.separator.go::<Check>(inp) {
            inp.rewind(before_separator);
            break if count >= self.at_least {
                Ok(output)
            } else {
                Err(e)
            };
        }

        /* snip */
    }
    Err(e) => { /* snip */ }
}

The current implementation will always parse the separator after parsing the item. I was able to confirm this by using the tests from master and just moved them over to zero-copy, as well as updated the implementation. I tried to document my implementation well enough without going overboard, so hopefully it is up to par with what you expect.

[zesterer]

Nice catch! It's frustrating how incredibly subtle the logic for separated_by is. Do you know whether this bug affects separated_by_exactly too?

[Zij-IT]

As far as I can tell the bug only affects SeparatedBy because with SeparatedByExactly you can safely parse the item-separator pairs until the last one, in which the separator is only parsed if the allow_trailing is set

[zesterer]

Makes sense. Thanks for the change!

[zesterer]

Oh by the way, make sure to add yourself to the authors field in the Cargo.toml. I keep forgetting to mention this to contributors, but I think it's worth doing :)