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

Fixing newlines and comments for if/else and do/catch #58

Merged
merged 2 commits into from
Nov 28, 2021

Conversation

alex-pinkus
Copy link
Owner

Fixes #47

Adds catch to the semi suppressor, and also suppresses semis at the
start of a comment. This also required a bit of rework that's been
overdue for a while, to not suppress explicit semicolons - those don't
get used much, but there was one test case where one of those was
followed by a comment.

@alex-pinkus
Copy link
Owner Author

While this improves things corpus-wise (trading two firefox errors for one new one), I suspect that new error might be higher-impact, and that it's not worth the tradeoff. Basically, we're now suppressing the newline between two declarations if the second has a comment, which seems fairly likely when items have documentation use cases. The fact that this doesn't fail more often tells me that the parser is able to figure this out most of the time, but I'm not sure what the missing case is.

I'm going to hold off on merging this until I can either fix the firefox-ios/Client/Frontend/Strings.swift issue or better explain why it's an isolated case that wouldn't have broader impact.

Fixes #47

Adds `catch` to the semi suppressor. Also reworks the semi suppressor to
not suppress _explicit_ semicolons - this doesn't matter for any corpus
cases, but is theoretically more correct, and protects against a future
failure involving comments.
Teaches the newline-suppressor to advance through a comment to see if
the subsequent token is a newline-suppressing operator (or one of the
single-char suppressors).

This also requires us to parse multiline comments, since we would
otherwise eat the first character of a multiline comment before that
component got a chance to.
@alex-pinkus
Copy link
Owner Author

OK, this fixes the comment-semi problem that the original commit introduced, so all should be well.

@alex-pinkus alex-pinkus merged commit 015a7bd into main Nov 28, 2021
@alex-pinkus alex-pinkus deleted the else-catch-newlines branch November 28, 2021 00:50
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.

Certain newline combos still cause problems with else / catch
1 participant