Skip to content

Fix #12084, #12392 FP constStatement, duplicateExpression with macro (regression) #5925

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.

Sign up for GitHub

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

Merged
chrchr-github merged 3 commits into from
Jan 31, 2024
Merged

Fix #12084, #12392 FP constStatement, duplicateExpression with macro (regression) #5925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b05d194
Fix #12084 FP constStatement with macro (regression)
chrchr-github Jan 30, 2024
e90feb3
Add test
chrchr-github Jan 30, 2024
0725397
Fix
chrchr-github Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions lib/templatesimplifier.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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());

@firewave firewave Mar 6, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The whole macro name stuff looks super messy. It is not forwarded in all the other addtoken() functions and the forwarding is also missing all across the code.

I also wonder if we need an Token::insertToken(const Token*) to make sure all of the values are being transferred.

} 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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7371,6 +7371,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