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

Fix Slice Scanner to Only Treat '/**' as Doc Comments #3193

Conversation

InsertCreativityHere
Copy link
Member

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:

/********************
        THING
********************/

Copy link
Member Author

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. */
Copy link
Member Author

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. */
<*>"/**"/[^/*] {
Copy link
Member Author

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 *".

Comment on lines +340 to +341
<C_COMMENT,C_DOC_COMMENT>"*" |
<C_COMMENT,C_DOC_COMMENT>[^\n*]+ {
Copy link
Member Author

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.

Comment on lines +357 to +360
if (YY_START == C_DOC_COMMENT)
{
currentUnit->setDocComment(comment.substr(0, yyleng - 2));
}
Copy link
Member Author

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.

@InsertCreativityHere InsertCreativityHere merged commit 8c76edb into zeroc-ice:main Nov 26, 2024
19 checks passed
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.

The Slice Scanner Treats All C-Style Comments as Doc-Comments
3 participants