-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 #12658 Remove constructor initialize also with checkHeaders = false #6375
base: main
Are you sure you want to change the base?
Conversation
lib/tokenize.cpp
Outdated
if (isIncluded && !mSettings.checkHeaders && tok->str() == ":") { | ||
if (tok->previous() && tok->previous()->str() == ")") { | ||
const Token *next = tok->next(); | ||
while (next && next->str() != "{") |
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 won't work if a member is initialized using braces.
0890f88
to
8bc47d8
Compare
lib/tokenize.cpp
Outdated
@@ -6225,6 +6225,33 @@ void Tokenizer::simplifyHeadersAndUnusedTemplates() | |||
for (Token *tok = list.front(); tok; tok = tok->next()) { | |||
const bool isIncluded = (tok->fileIndex() != 0); | |||
|
|||
// Remove constructor initializer | |||
if (isIncluded && !mSettings.checkHeaders && tok->str() == ":") { |
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.
what if this is a ternary operation?
a ? (b + c) : ...
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.
sorry for late reply. Please feel free to ping if it takes couple of days for response.
lib/tokenize.cpp
Outdated
} | ||
if (start && start->str() == ":") { | ||
const Token *next = start; | ||
while (Token::Match(next, "[,:]")) { |
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 expect that there is a name token after next right?
while (Token::Match(next, "[,:] %name%"))
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.
Correct
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 my opinion it wouldn't hurt to add that more explicit check.
lib/tokenize.cpp
Outdated
const Token *next = start; | ||
while (Token::Match(next, "[,:]")) { | ||
// Skip to ( or { | ||
while (next && !next->link()) |
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.
I don't want to skip any random tokens. something more like:
next = next->next();
while (Token::Match(next, "%name%|::|<")) {
if (next->str() == "<")
// find end token.. I believe there is a utility function in template simplifier
next = next->next();
}
if (!Token::Match(next, "(|{"))
// not initializer list..
next = next->link()->next(); // <- goto "," or "{"
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.
Looking better at it, If after the previous change 'Token::Match(next, "[,:] %name%")' The symbol after %name% should always be a link in a constructor initializer list. If not is is something else. %name% should always be a member or delegate constructor.
Only thing now failing is a parameter pack expansion in an initializer list
template<class... Mixins>
class X : public Mixins...
{
public:
X(const Mixins&... mixins) : Mixins(mixins)... {}
};
const Token *next = start; | ||
while (Token::Match(next, "[,:] %name%")) { | ||
next = next->next(); // Go to %name% | ||
next = next->next(); // Go to opening '(' or '{' |
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.
please consider if baseclass is declared in a different namespace also:
C() : a::base(0) {}
Fixes https://trac.cppcheck.net/ticket/12658 by also removing the construcot initializer to leave valid code after removing executable code