Skip to content

[Parse] Allow prefix operators containing / with /.../ regex literals #58835

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 3 commits into from
May 12, 2022

Conversation

hamishknight
Copy link
Contributor

@hamishknight hamishknight commented May 11, 2022

When lexing a regex literal, treat a prefix operator containing / the same as the unapplied infix operator case, where we tentatively lex. This means that we bail if there is no closing / or the starting character is invalid. This leaves binary operator containing / in expression position as the last place where we know that we definitely have a regex literal.

Additionally, expand the starting ) character heuristic to cover the entire range of the regex literal, ensuring to take escapes and custom character classes into account. This means we will not lex a regex literal if there is an unbalanced ) somewhere in the body.

@hamishknight hamishknight requested a review from rintaro May 11, 2022 19:23
@hamishknight
Copy link
Contributor Author

Source compat running on #42250

Treat a prefix operator containing `/` the same as
the unapplied infix operator case, where we
tentatively lex. This means that we bail if there
is no closing `/` or the starting character is
invalid. This leaves binary operator containing
`/` in expression position as the last place where
we know that we definitely have a regex literal.
Previously we would only check for a starting
character of `)` when performing a tentative
lex of a regex literal. Expand this to cover the
entire range of the regex literal, ensuring to
take escapes and custom character classes into
account.
@hamishknight
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

LGTM

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