8000 Allow eof everywhere kEND is allowed by jez · Pull Request #5183 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Allow eof everywhere kEND is allowed #5183

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

Closed
wants to merge 7 commits into from
Closed

Allow eof everywhere kEND is allowed #5183

wants to merge 7 commits into from

Conversation

jez
Copy link
Collaborator
@jez jez commented Jan 27, 2022

Motivation

This is a huge hack, but it appears to maybe work okay in practice.

Test plan

See included automated tests.

jez added 7 commits January 26, 2022 22:31
This is a huge hack, but it appears to maybe work ok in practice.
I couldn't get this to work with the `# error:` assertions, because it
was fiddly with the end-of-file handling
This is an artifact of how it works right now: every time the
kend_or_eof rule reduces, it'll add another error. So if it reduces
multiple times because there are multiple `kEND` tokens missing, each
one will get a new error. It's kind of weird, and we could attempt to
dedup, but honestly it seems kind of not terrible.
@jez
Copy link
Collaborator Author
jez commented Jan 27, 2022

We have a policy of testing changes to Sorbet against Stripe's codebase before
merging them. I've kicked off a test run for the current PR. When the build
finishes, I'll share with you whether or how it failed. Thanks!

Stripe employees can see the build results here:

https://go/builds/bui_L2YKAcXt8aCMoM
https://go/builds/bui_L2YKHQsitTIiwb

@jez
Copy link
Collaborator Author
jez commented Feb 15, 2022

#5000 superceded this

@jez jez closed this Feb 15, 2022
@jez jez deleted the jez-kend-or-eof branch February 15, 2022 18:16
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.

1 participant
0