8000 No-op changes in service of indentation-aware parser by jez · Pull Request #5261 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

No-op changes in service of indentation-aware parser #5261

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 7 commits into from
Feb 5, 2022
Merged

Conversation

jez
Copy link
Collaborator
@jez jez commented Feb 5, 2022

Motivation

I've had a somewhat long-lived branch full of changes in support of #5000. This
PR takes the ones that don't change behavior and lands them (except for one
small improvement for kIF error that is actually new).

Commit summary

  • no-op: Mark Builder::build const (1f25166)

  • no-op cleanup in Parser.cc (dbadbb8)

  • pre-work: Better recovery for kIF kEND (9ec9e72)

  • pre-work: Expose yytname_ and yytranslate_ to driver (1c171ca)

    This lets us avoid having to write our code in C macros at the top of
    the parser file, which will then get exploded into usage sites, and
    instead lets us factor it into a simple helper function on the driver.

  • pre-work: Factor rewind_and_reset to driver (979cbcf)

  • This test gets better (2629f75)

Test plan

Most things don't change, except one test.

jez added 6 commits February 4, 2022 14:59
This lets us avoid having to write our code in C macros at the top of
the parser file, which will then get exploded into usage sites, and
instead lets us factor it into a simple helper function on the driver.
@jez jez requested a review from a team as a code owner February 5, 2022 02:15
@jez jez requested review from aprocter and removed request for a team February 5, 2022 02:15
Copy link
Contributor
@aprocter aprocter left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the commit-by-commit!

@jez jez merged commit 8019922 into master Feb 5, 2022
@jez jez deleted the jez-checkpoint branch February 5, 2022 03:38
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.

2 participants
0