Skip to content

Fix \o crasher and allow (?) #487

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 16, 2022
Merged

Fix \o crasher and allow (?) #487

merged 3 commits into from
Jun 16, 2022

Conversation

hamishknight
Copy link
Contributor

Separate these changes off #481 so they can be independently cherry-picked to 5.7:

  • Fix a crash that occurred with \o if it was not followed by {.
  • Allow (?), which is permitted by PCRE (but is a no-op).

Resolves #472

@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight hamishknight requested a review from milseman June 15, 2022 19:33
milseman
milseman reviewed Jun 16, 2022
View reviewed changes
@hamishknight
Move some LexTests over to ParseTests
0b46160
@hamishknight
Allow (?)
027ccc7 
This is accepted by PCRE, and forms an empty
option change sequence. We probably ought to warn
on it though as it's a no-op.
@hamishknight
Fix \o parsing crash
e861af5 
This was caused by the fact that we'd walk into
`expectUnicodeScalar` if we saw `\o`, but we only
want to parse `\o{`. Instead, change it to be a `lex..`
method, and bail if we don't lex a scalar.
@hamishknight
Copy link
Contributor Author

@swift-ci please test

milseman
milseman approved these changes Jun 16, 2022
View reviewed changes
Copy link
Member

@milseman milseman left a comment

Choose a reason for hiding this comment

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

LGTM

@hamishknight hamishknight merged commit 0878029 into swiftlang:main Jun 16, 2022
@hamishknight hamishknight deleted the separate-these branch June 16, 2022 16:56
@hamishknight hamishknight mentioned this pull request Jun 17, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@milseman milseman milseman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

\o crashes the parser
2 participants
@hamishknight @milseman