Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly handle cuts in LHS of flatMap calls and Not/Lookahead/Filter #124

Merged
merged 5 commits into from Sep 4, 2016

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Sep 3, 2016

We were not properly propagating the cuts in the LHS of flatMaps; that meant that if there was a cut inside the left hand parser, it doesn't take effect if something fails in the RHS of the flatMap call, or if the flatMap succeeds but something fails later.

Also makes ! and & negative/positive lookaheads behave as NoCuts: the whole point of those operators is to backtrack, so there isn't any conceivable use-case where someone would want to use them and have a cut disable-backtracking/drop-buffer when inside one of these operators.

Also properly propagates cuts in foo.filter(pred) when foo passes but pred fails.

Found these when investigating the build failure in #122. Possibly not the only issue, but it's something. This changes behavior, but if this makes any tests fail, I'll fix them.

Review by @vovapolu

@lihaoyi lihaoyi changed the title Properly handle cuts in LHS of flatMap calls Properly handle cuts in LHS of flatMap calls and Not/Lookahead Sep 3, 2016
… inner-parser passes but the filter-condition fails
@lihaoyi lihaoyi changed the title Properly handle cuts in LHS of flatMap calls and Not/Lookahead Properly handle cuts in LHS of flatMap calls and Not/Lookahead/Filter Sep 3, 2016
@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 3, 2016

In theory, this should be all that is necessary to make #122 pass

@vovapolu vovapolu merged commit 518efdf into master Sep 4, 2016
@lihaoyi lihaoyi deleted the fix-cut branch October 27, 2018 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants