Skip to content

Commit

Permalink
Fix #12084, #12392 FP constStatement, duplicateExpression with macro …
Browse files Browse the repository at this point in the history 
…(regression) (#5925)
  • Loading branch information
@chrchr-github
chrchr-github committed Jan 31, 2024
1 parent 6375880 commit c252427
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 27 deletions.
30 changes: 15 additions & 15 deletions lib/templatesimplifier.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1000,11 +1000,11 @@ void TemplateSimplifier::getTemplateInstantiations()
std::string::size_type offset = 0;
std::string::size_type pos = 0;
while ((pos = nameSpace.find(' ', offset)) != std::string::npos) {
qualificationTok->insertToken(nameSpace.substr(offset, pos - offset), emptyString, true);
qualificationTok->insertTokenBefore(nameSpace.substr(offset, pos - offset));
offset = pos + 1;
}
qualificationTok->insertToken(nameSpace.substr(offset), emptyString, true);
qualificationTok->insertToken("::", emptyString, true);
qualificationTok->insertTokenBefore(nameSpace.substr(offset));
qualificationTok->insertTokenBefore("::");
addInstantiation(tok, it1->scope());
found = true;
break;
Expand Down Expand Up @@ -1635,9 +1635,9 @@ void TemplateSimplifier::expandTemplate(

// add forward declarations
if (copy && isClass) {
templateDeclaration.token()->insertToken(templateDeclarationToken->strAt(1), emptyString, true);
templateDeclaration.token()->insertToken(newName, emptyString, true);
templateDeclaration.token()->insertToken(";", emptyString, true);
templateDeclaration.token()->insertTokenBefore(templateDeclarationToken->strAt(1));
templateDeclaration.token()->insertTokenBefore(newName);
templateDeclaration.token()->insertTokenBefore(";");
} else if ((isFunction && (copy || isSpecialization)) ||
(isVariable && !isSpecialization) ||
(isClass && isSpecialization && mTemplateSpecializationMap.find(templateDeclaration.token()) != mTemplateSpecializationMap.end())) {
Expand Down Expand Up @@ -1701,7 +1701,7 @@ void TemplateSimplifier::expandTemplate(
}

if (isStatic) {
dst->insertToken("static", emptyString, true);
dst->insertTokenBefore("static");
if (start) {
dst->previous()->linenr(start->linenr());
dst->previous()->column(start->column());
Expand Down Expand Up @@ -1742,7 +1742,7 @@ void TemplateSimplifier::expandTemplate(
++typeindentlevel;
else if (typetok->str() == ")")
--typeindentlevel;
dst->insertToken(typetok->str(), typetok->originalName(), true);
dst->insertToken(typetok->str(), typetok->originalName(), typetok->getMacroName(), true);
dst->previous()->linenr(start->linenr());
dst->previous()->column(start->column());
Token *previous = dst->previous();
Expand Down Expand Up @@ -1770,7 +1770,7 @@ void TemplateSimplifier::expandTemplate(
}
}
if (pointerType && Token::simpleMatch(dst1, "const")) {
dst->insertToken("const", dst1->originalName(), true);
dst->insertToken("const", dst1->originalName(), dst1->getMacroName(), true);
dst->previous()->linenr(start->linenr());
dst->previous()->column(start->column());
dst1->deleteThis();
Expand All @@ -1784,13 +1784,13 @@ void TemplateSimplifier::expandTemplate(
!(templateDeclaration.isFunction() && templateDeclaration.scope().empty() &&
(start->strAt(-1) == "." || Token::simpleMatch(start->tokAt(-2), ". template")))) {
if (start->strAt(1) != "<" || Token::Match(start, newName.c_str()) || !inAssignment) {
dst->insertToken(newName, emptyString, true);
dst->insertTokenBefore(newName);
dst->previous()->linenr(start->linenr());
dst->previous()->column(start->column());
if (start->strAt(1) == "<")
start = start->next()->findClosingBracket();
} else {
dst->insertToken(start->str(), emptyString, true);
dst->insertTokenBefore(start->str());
dst->previous()->linenr(start->linenr());
dst->previous()->column(start->column());
newInstantiations.emplace_back(dst->previous(), templateDeclaration.scope());
Expand All @@ -1814,23 +1814,23 @@ void TemplateSimplifier::expandTemplate(
return Token::simpleMatch(inst.token(), name.c_str(), name.size());
})) {
// use the instantiated name
dst->insertToken(name, "", true);
dst->insertTokenBefore(name);
dst->previous()->linenr(start->linenr());
dst->previous()->column(start->column());
start = closing;
}
}
// just copy the token if it wasn't instantiated
if (start != closing) {
dst->insertToken(start->str(), start->originalName(), true);
dst->insertToken(start->str(), start->originalName(), start->getMacroName(), true);
dst->previous()->linenr(start->linenr());
dst->previous()->column(start->column());
dst->previous()->isSigned(start->isSigned());
dst->previous()->isUnsigned(start->isUnsigned());
dst->previous()->isLong(start->isLong());
}
} else {
dst->insertToken(start->str(), start->originalName(), true);
dst->insertToken(start->str(), start->originalName(), start->getMacroName(), true);
dst->previous()->linenr(start->linenr());
dst->previous()->column(start->column());
dst->previous()->isSigned(start->isSigned());
Expand Down Expand Up @@ -1858,7 +1858,7 @@ void TemplateSimplifier::expandTemplate(

start = start->next();
}
dst->insertToken(";", emptyString, true);
dst->insertTokenBefore(";");
dst->previous()->linenr(dst->tokAt(-2)->linenr());
dst->previous()->column(dst->tokAt(-2)->column() + 1);

Expand Down
6 changes: 3 additions & 3 deletions lib/token.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1129,16 +1129,16 @@ void Token::function(const Function *f)
tokType(eName);
}

Token* Token::insertToken(const std::string& tokenStr, const std::string& originalNameStr, bool prepend)
Token* Token::insertToken(const std::string& tokenStr, const std::string& originalNameStr, const std::string& macroNameStr, bool prepend)
{
Token *newToken;
if (mStr.empty())
newToken = this;
else
newToken = new Token(mTokensFrontBack);
newToken->str(tokenStr);
if (!originalNameStr.empty())
newToken->originalName(originalNameStr);
newToken->originalName(originalNameStr);
newToken->setMacroName(macroNameStr);

if (newToken != this) {
newToken->mImpl->mLineNumber = mImpl->mLineNumber;
Expand Down
6 changes: 3 additions & 3 deletions lib/token.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -850,11 +850,11 @@ class CPPCHECKLIB Token {
* @param prepend Insert the new token before this token when it's not
* the first one on the tokens list.
*/
Token* insertToken(const std::string& tokenStr, const std::string& originalNameStr = emptyString, bool prepend = false);
Token* insertToken(const std::string& tokenStr, const std::string& originalNameStr = emptyString, const std::string& macroNameStr = emptyString, bool prepend = false);

Token* insertTokenBefore(const std::string& tokenStr, const std::string& originalNameStr = emptyString)
Token* insertTokenBefore(const std::string& tokenStr, const std::string& originalNameStr = emptyString, const std::string& macroNameStr = emptyString)
{
return insertToken(tokenStr, originalNameStr, true);
return insertToken(tokenStr, originalNameStr, macroNameStr, true);
}

Token* previous() {
Expand Down
6 changes: 3 additions & 3 deletions lib/tokenize.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7389,11 +7389,11 @@ void Tokenizer::simplifyStaticConst()
// Move the qualifier to the left-most position in the declaration
tok->deleteNext();
if (!leftTok) {
list.front()->insertToken(qualifiers[i], emptyString, false);
list.front()->insertToken(qualifiers[i]);
list.front()->swapWithNext();
tok = list.front();
} else if (leftTok->next()) {
leftTok->next()->insertToken(qualifiers[i], emptyString, true);
leftTok->next()->insertTokenBefore(qualifiers[i]);
tok = leftTok->next();
} else {
leftTok->insertToken(qualifiers[i]);
Expand Down Expand Up @@ -10312,7 +10312,7 @@ void Tokenizer::prepareTernaryOpForAST()
}
if (parenthesesNeeded && tok2 && tok2->str() == ":") {
tok->insertToken("(");
tok2->insertToken(")", emptyString, true);
tok2->insertTokenBefore(")");
Token::createMutualLinks(tok->next(), tok2->previous());
}
}
Expand Down
6 changes: 3 additions & 3 deletions lib/tokenlist.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,13 @@ void TokenList::addtoken(const Token *tok)
return;

if (mTokensFrontBack.back) {
mTokensFrontBack.back->insertToken(tok->str(), tok->originalName());
mTokensFrontBack.back->insertToken(tok->str(), tok->originalName(), tok->getMacroName());
} else {
mTokensFrontBack.front = new Token(&mTokensFrontBack);
mTokensFrontBack.back = mTokensFrontBack.front;
mTokensFrontBack.back->str(tok->str());
if (!tok->originalName().empty())
mTokensFrontBack.back->originalName(tok->originalName());
mTokensFrontBack.back->originalName(tok->originalName());
mTokensFrontBack.back->setMacroName(tok->getMacroName());
}

mTokensFrontBack.back->flags(tok->flags());
Expand Down
10 changes: 10 additions & 0 deletions test/testother.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7379,6 +7379,16 @@ class TestOther : public TestFixture {
" if (std::is_same_v<T, int> || std::is_same_v<T, int32_t>) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());

checkP("#define F(v) (v) != 0\n" // #12392
"template<class T>\n"
"void f() {\n"
" if (F(0)) {}\n"
"}\n"
"void g() {\n"
" f<int>();\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}

void duplicateExpressionCompareWithZero() {
Expand Down

1 comment on commit c252427

@firewave
Copy link
Collaborator

Choose a reason for hiding this comment

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

This causes an Ir regression in the callgrind CI job:

59,388,418,483 (100.0%)  PROGRAM TOTALS
[...]
6,788,502,374 (11.43%)  build/token.cpp:Token::Match(Token const*, char const*, int) [/home/runner/work/cppcheck/cppcheck/cppcheck]
2,842,749,024 ( 4.79%)  ./malloc/./malloc/malloc.c:_int_free [/usr/lib/x86_64-linux-gnu/libc.so.6]
2,512,925,996 ( 4.23%)  ./malloc/./malloc/malloc.c:_int_malloc [/usr/lib/x86_64-linux-gnu/libc.so.6]
2,046,652,970 ( 3.45%)  build/library.cpp:Library::detectContainerInternal(Token const*, Library::DetectContainer, bool*, bool) const [/home/runner/work/cppcheck/cppcheck/cppcheck]
1,430,701,327 ( 2.41%)  ./malloc/./malloc/malloc.c:malloc [/usr/lib/x86_64-linux-gnu/libc.so.6]

60,739,484,090 (100.0%)  PROGRAM TOTALS
[...]
6,788,502,374 (11.18%)  build/token.cpp:Token::Match(Token const*, char const*, int) [/home/runner/work/cppcheck/cppcheck/cppcheck]
3,074,172,929 ( 5.06%)  ./malloc/./malloc/malloc.c:_int_free [/usr/lib/x86_64-linux-gnu/libc.so.6]
2,916,450,148 ( 4.80%)  ./malloc/./malloc/malloc.c:_int_malloc [/usr/lib/x86_64-linux-gnu/libc.so.6]
2,046,652,970 ( 3.37%)  build/library.cpp:Library::detectContainerInternal(Token const*, Library::DetectContainer, bool*, bool) const [/home/runner/work/cppcheck/cppcheck/cppcheck]
1,551,717,562 ( 2.55%)  ./malloc/./malloc/malloc.c:malloc [/usr/lib/x86_64-linux-gnu/libc.so.6]

Could just be noise though...

Please sign in to comment.