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

mmanek patch 1 #6352

Closed
wants to merge 2 commits into from
Closed

mmanek patch 1 #6352

wants to merge 2 commits into from

Conversation

mmanek
Copy link

@mmanek mmanek commented Apr 26, 2024

I added a condition in the “IsEnumStart” function that checks the token three positions previous to the current one which will help verify that the pattern of the program is correct. Additionally, I added a condition that comes before the return statement of “isClassStructUnionEnumStart” that checks if the enum -> type -> name pattern is detected.

@firewave
Copy link
Collaborator

Thanks for contribution.

Please update the title of the PR to reflect what it does. And please add a unit test.

@chrchr-github
Copy link
Collaborator

Not sure where it is used, but isEnumStart() doesn't seem to handle e.g. enum E : ::std::uint8_t {. I also dimly remember adding a similar function recently somewhere.

@chrchr-github
Copy link
Collaborator

There is isEnumScope() in tokenize.cpp. We should probably consolidate.

@@ -152,7 +151,7 @@ static bool isClassStructUnionEnumStart(const Token * tok)
const Token * tok2 = tok->previous();
while (tok2 && !Token::Match(tok2, "class|struct|union|enum|{|}|;"))
tok2 = tok2->previous();
return Token::Match(tok2, "class|struct|union|enum");
return Token::Match(tok2, "class|struct|union|enum") || Token::Match(tok2, "enum %type% %name%");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is unnecessary. We just check whether we have arrived at one of the four keywords here.

@@ -80,7 +79,7 @@ static bool isEnumStart(const Token* tok)
{
if (!tok || tok->str() != "{")
return false;
return (tok->strAt(-1) == "enum") || (tok->strAt(-2) == "enum") || Token::Match(tok->tokAt(-3), "enum class %name%");
return (tok->strAt(-1) == "enum") || (tok->strAt(-2) == "enum") || Token::Match(tok->tokAt(-3), "enum class %name%") || Token::Match(tok->tokAt(-3), "enum %type% %name%");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest moving the code from isEnumScope() here and replacing the call to isEnumScope() with isEnumStart().

@chrchr-github
Copy link
Collaborator

Feel free to reopen once comments have been addressed.

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.

3 participants