8000 Fix parser shift/reduce conflicts by Morriar · Pull Request #4178 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix parser shift/reduce conflicts #4178

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

Merged
merged 2 commits into from
Apr 27, 2021

Conversation

Morriar
Copy link
Collaborator
@Morriar Morriar commented Apr 26, 2021

Motivation

I introduced shift/reduce conflicts in the grammar while adding support for begin-less ranges with .. and ... (#3313). This PR fixes them.

Example when compiling the parser before this PR:

image

The approach is the same as the one used by Whitequark (https://github.com/whitequark/parser/blob/de619a2ff36a39b4d43efa9b86cdec98621ed435/lib/parser/lexer.rl#L2026-L2035): duplicating the tokens tDOT2 and tDOT3 and lexing them differently depending on the Ruby version used.

One quirk with this approach is the way we display the parsing errors. Without any additional change, the error message would be:

^^^ error: unexpected token tBDOT3

In order to get a friendlier output, I tweaker the error generation method (see second commit) so we get back to the expected following error:

^^^ error: unexpected token "..."

Compiling the parser after this PR:

image

No more shift/reduce conflict warning. 🎉

Test plan

See included automated tests.

Morriar added 2 commits April 26, 2021 13:16
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar Morriar requested a review from a team as a code owner April 26, 2021 17:32
@Morriar Morriar requested review from jez and removed request for a team April 26, 2021 17:32
Copy link
Collaborator
@jez jez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@froydnj froydnj merged commit ae3bc7d into sorbet:master Apr 27, 2021
@Morriar Morriar deleted the at-fix-parser-conflicts branch July 26, 2022 15:47
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.

3 participants
0