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

Add explicit support for scope resolution operator in C++ lexer #1523

Merged
merged 1 commit into from Jun 3, 2020

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented May 30, 2020

The C++ lexer inherits the majority of its rules from the C lexer. While this approach reduces duplication, it can cause issues where assumptions that are correct in C, cause issues in C++.

One such issue occurs when the scope resolution operator, ::, is used in a case statement. The C lexer contains a rule that treats colons in a case statement as representing the 'end' of that state. While the assumption that a colon would not otherwise occur in a case statement is correct in C, that is not the case in C++. The scope resolution operator uses two colons and it can occur within a case statement.

This PR creates a new :case state in the C++ lexer that uses a negative lookahead to avoid matching a scope resolution operator. A rule matching the scope resolution operator is also prepended to the :statements state. This fixes #1506.

@pyrmont pyrmont changed the title Add explicit support for scope resolution operator Add explicit support for scope resolution operator in C++ lexer May 30, 2020
@pyrmont pyrmont added the needs-review The PR needs to be reviewed label May 30, 2020
@pyrmont
Copy link
Contributor Author

pyrmont commented May 30, 2020

@tim-becker Apologies for the delay in getting to this. I'm not sure why I was confused by your original bug report: the error was right there in the title! This PR fixes the example on my end. Please let me know how it looks to you.

@pyrmont pyrmont self-assigned this May 30, 2020
@tim-becker
Copy link

LGTM. Thanks for the fix!

@pyrmont pyrmont merged commit 646e551 into rouge-ruby:master Jun 3, 2020
@pyrmont pyrmont deleted the bugfix.cpp-scope-operator branch June 3, 2020 20:13
@pyrmont
Copy link
Contributor Author

pyrmont commented Jun 3, 2020

@tim-becker Great! Well, I've merged this in and it will be part of v3.20.0 of Rouge. That will be pushed out to RubyGems on Tuesday 9 June. Thanks for your help in making Rouge better! 🎉

@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label Jun 3, 2020
mattt pushed a commit to NSHipster/rouge that referenced this pull request May 19, 2021
The C++ lexer inherits the majority of its rules from the C lexer.
While this approach reduces duplication, it can cause issues where
assumptions that are correct in C, cause issues in C++.

One such issue occurs when the scope resolution operator, `::`, is used
in a case statement. The C lexer contains a rule that treats colons in
a case statement as representing the 'end' of that state. While the
assumption that a colon would not otherwise occur in a case statement
is correct in C, that is not the case in C++. The scope resolution
operator uses two colons and it can occur within a case statement.

This commit creates a new `:case` state in the C++ lexer that uses a
negative lookahead to avoid matching a scope resolution operator. A
rule matching the scope resolution operator is also prepended to the
`:statements` state.
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.

c++ lexer can't handle scope resolution operator in case expression
2 participants