walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix #9406 FN redundant assignment of Boolean variable (#4482) 

* Fix #9406 FN redundant assignment of Boolean variable

* Fix warning, refactor

* Update testcondition.cpp
This commit is contained in:
chrchr-github 2022-09-20 07:30:12 +02:00 committed by GitHub
parent f274d499a1
commit dcb332acb0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 66 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
lib/checkcondition.cpp
View File

@ -1750,9 +1750,10 @@ void CheckCondition::checkDuplicateConditionalAssign()
continue;
const Token *blockTok = tok->next()->link()->next();
const Token *condTok = tok->next()->astOperand2();
if (!Token::Match(condTok, "==|!="))
const bool isBoolVar = Token::Match(condTok, "!| %var%");
if (!isBoolVar && !Token::Match(condTok, "==|!="))
continue;
if (condTok->str() == "!=" && Token::simpleMatch(blockTok->link(), "} else {"))
if ((isBoolVar || condTok->str() == "!=") && Token::simpleMatch(blockTok->link(), "} else {"))
continue;
if (!blockTok->next())
continue;
@ -1761,18 +1762,33 @@ void CheckCondition::checkDuplicateConditionalAssign()
continue;
if (nextAfterAstRightmostLeaf(assignTok) != blockTok->link()->previous())
continue;
bool isRedundant = false;
if (isBoolVar) {
if (!astIsBool(condTok))
continue;
const bool isNegation = condTok->str() == "!";
if (!(assignTok->astOperand1() && assignTok->astOperand1()->varId() == (isNegation ? condTok->next() : condTok)->varId()))
continue;
if (!(assignTok->astOperand2() && assignTok->astOperand2()->hasKnownIntValue()))
continue;
const MathLib::bigint val = assignTok->astOperand2()->getKnownIntValue();
if (val < 0 || val > 1)
continue;
isRedundant = (isNegation && val == 0) || (!isNegation && val == 1);
} else { // comparison
if (!isSameExpression(
mTokenizer->isCPP(), true, condTok->astOperand1(), assignTok->astOperand1(), mSettings->library, true, true))
continue;
if (!isSameExpression(
mTokenizer->isCPP(), true, condTok->astOperand2(), assignTok->astOperand2(), mSettings->library, true, true))
continue;
duplicateConditionalAssignError(condTok, assignTok);
}
duplicateConditionalAssignError(condTok, assignTok, isRedundant);
}
}
}
void CheckCondition::duplicateConditionalAssignError(const Token *condTok, const Token* assignTok)
void CheckCondition::duplicateConditionalAssignError(const Token *condTok, const Token* assignTok, bool isRedundant)
{
ErrorPath errors;
std::string msg = "Duplicate expression for the condition and assignment.";
@ -1782,7 +1798,8 @@ void CheckCondition::duplicateConditionalAssignError(const Token *condTok, const
errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "'");
errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "' is redundant");
} else {
msg = "The statement 'if (" + condTok->expressionString() + ") " + assignTok->expressionString() + "' is logically equivalent to '" + assignTok->expressionString() + "'.";
msg = "The statement 'if (" + condTok->expressionString() + ") " + assignTok->expressionString();
msg += isRedundant ? "' is redundant." : "' is logically equivalent to '" + assignTok->expressionString() + "'.";
errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "'");
errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "' is redundant");
}

2
lib/checkcondition.h
View File

@ -165,7 +165,7 @@ private:
void invalidTestForOverflow(const Token* tok, const ValueType *valueType, const std::string &replace);
void pointerAdditionResultNotNullError(const Token *tok, const Token *calc);
void duplicateConditionalAssignError(const Token *condTok, const Token* assignTok);
void duplicateConditionalAssignError(const Token *condTok, const Token* assignTok, bool isRedundant = false);
void assignmentInCondition(const Token *eq);

2
lib/checkunusedfunctions.cpp
View File

@ -120,9 +120,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi
while ((scope || start) && markupVarToken) {
if (markupVarToken->str() == settings->library.blockstart(FileName)) {
scope++;
if (start) {
start = false;
}
} else if (markupVarToken->str() == settings->library.blockend(FileName))
scope--;
else if (!settings->library.iskeyword(FileName, markupVarToken->str())) {

39
test/testcondition.cpp
View File

@ -4073,21 +4073,21 @@ private:
" if (init)\n"
" init = false;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style) The statement 'if (init) init=false' is logically equivalent to 'init=false'.\n", errout.str());
check("void f() {\n"
" static bool init(true);\n"
" if (init)\n"
" init = false;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style) The statement 'if (init) init=false' is logically equivalent to 'init=false'.\n", errout.str());
check("void f() {\n"
" static bool init{ true };\n"
" if (init)\n"
" init = false;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style) The statement 'if (init) init=false' is logically equivalent to 'init=false'.\n", errout.str());
// #10248
check("void f() {\n"
@ -5431,6 +5431,39 @@ private:
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool f(bool b) {\n"
" if (b)\n"
" b = false;\n"
" else\n"
" g();\n"
" return b;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("struct S {\n" // #9406
" S() : b(false) {}\n"
" void f() {\n"
" if (b) b = true;\n"
" if (b) b = false;\n"
" if (!b) b = true;\n"
" if (!b) b = false;\n"
" }\n"
" bool b;\n"
"};\n");
ASSERT_EQUALS("test.cpp:4:style:The statement 'if (b) b=true' is redundant.\n"
"test.cpp:4:note:Assignment 'b=true'\n"
"test.cpp:4:note:Condition 'b' is redundant\n"
"test.cpp:5:style:The statement 'if (b) b=false' is logically equivalent to 'b=false'.\n"
"test.cpp:5:note:Assignment 'b=false'\n"
"test.cpp:5:note:Condition 'b' is redundant\n"
"test.cpp:6:style:The statement 'if (!b) b=true' is logically equivalent to 'b=true'.\n"
"test.cpp:6:note:Assignment 'b=true'\n"
"test.cpp:6:note:Condition '!b' is redundant\n"
"test.cpp:7:style:The statement 'if (!b) b=false' is redundant.\n"
"test.cpp:7:note:Assignment 'b=false'\n"
"test.cpp:7:note:Condition '!b' is redundant\n",
errout.str());
}
void checkAssignmentInCondition() {