-
Notifications
You must be signed in to change notification settings - Fork 591
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
Fix Slice Scanner to Only Treat '/**' as Doc Comments #3193
Fix Slice Scanner to Only Treat '/**' as Doc Comments #3193
Conversation
cpp/src/Slice/Scanner.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is generated by flex
, and probably not worth reviewing.
@@ -92,6 +92,7 @@ namespace | |||
|
|||
/* List of start-condition states the scanner can be in. This lets the scanning be context dependent. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At any moment the Slice scanner is in a specific state (called a "Start Condition").
This list contains all the possible states that it can be within.
Previously, we only had a single C_COMMENT state, and whenever the scanner was in this state, it would add any text it saw to the current doc-comment. This was bad.
So, now we have 2 states. C_DOC_COMMENT is triggered when the scanner sees /** and C_COMMENT is triggered when the scanner sees /*. The scanner will try to match rules in order so it will always try to match /** first.
And, the scanner will only add things to doc comments while it is in the C_DOC_COMMENT state.
<*>"///".* { | ||
currentUnit->addToDocComment(yytext + 3); | ||
} | ||
|
||
/* Matches and consumes a C++ style comment. */ | ||
<*>"//".* {} | ||
|
||
/* Matches the start of a C style doc-comment, and switches the scanner to the C_DOC_COMMENT state. | ||
* Specifically we match the literal "/**" when it is _not_ followed by either a '/' or '*' character. */ | ||
<*>"/**"/[^/*] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the flex-regex-style for "match /**
, then do a positive lookahead (the /
) for anything that is not (the ^
) either /
or *
".
<C_COMMENT,C_DOC_COMMENT>"*" | | ||
<C_COMMENT,C_DOC_COMMENT>[^\n*]+ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Flex, anything in angle brackets means: "only match this rule if you are in one of these start conditions".
We re-use these rules for both the C_COMMENT
and C_DOC_COMMENT
conditions, since they only difference is whether the text counts towards a doc-comment or not.
if (YY_START == C_DOC_COMMENT) | ||
{ | ||
currentUnit->setDocComment(comment.substr(0, yyleng - 2)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only set the text as a doc comment if we're in the C_DOC_COMMENT start condition.
This PR fixes #3192, a pretty egregious bug in the Slice scanner.
It was treating all comments of the form
/* */
as doc comments, when doc comments should require two stars on the opening slash (/**
). So, this whole time, we've been generating doc-comments for all C-style comments.Now, the Slice compiler will only treat a C-style comment as a doc-comment if it starts with
/**
, and the next character is anything other than*
or/
. It must have exactly 2 stars, no more and no less.These rules protect us against:
/**/
which isn't a doc comment, it's really just an empty C-style comment (like/* */
).and people who like to write big banner-style comments like: