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

Reenable follow var for logical conjunction (#1400)

This commit is contained in:
Paul Fultz II 2018-10-01 07:40:03 -05:00 committed by Daniel Marjamäki
parent 2c91b95d2a
commit 1fe1ec09a8
4 changed files with 19 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
lib/astutils.cpp
View File

@ -481,11 +481,11 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
// condition found .. get comparator // condition found .. get comparator
std::string comp2; std::string comp2;
if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, followVar) && if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, followVar, errors) &&
isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure, followVar)) { isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), library, pure, followVar, errors)) {
comp2 = cond2->str(); comp2 = cond2->str();
} else if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), library, pure, followVar) && } else if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), library, pure, followVar, errors) &&
isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand1(), library, pure, followVar)) { isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand1(), library, pure, followVar, errors)) {
comp2 = cond2->str(); comp2 = cond2->str();
if (comp2[0] == '>') if (comp2[0] == '>')
comp2[0] = '<'; comp2[0] = '<';

19
lib/checkcondition.cpp
View File

@ -973,20 +973,22 @@ void CheckCondition::checkIncorrectLogicOperator()
const bool isfloat = astIsFloat(expr1, true) || MathLib::isFloat(value1) || astIsFloat(expr2, true) || MathLib::isFloat(value2); const bool isfloat = astIsFloat(expr1, true) || MathLib::isFloat(value1) || astIsFloat(expr2, true) || MathLib::isFloat(value2);
ErrorPath errorPath;
// Opposite comparisons around || or && => always true or always false // Opposite comparisons around || or && => always true or always false
if (!isfloat && isOppositeCond(tok->str() == "||", mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, true, false)) { if (!isfloat && isOppositeCond(tok->str() == "||", mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, true, true, &errorPath)) {
const bool alwaysTrue(tok->str() == "||"); const bool alwaysTrue(tok->str() == "||");
incorrectLogicOperatorError(tok, conditionString(tok), alwaysTrue, inconclusive); incorrectLogicOperatorError(tok, conditionString(tok), alwaysTrue, inconclusive, errorPath);
continue; continue;
} }
if (!parseable) if (!parseable)
continue; continue;
if (isSameExpression(mTokenizer->isCPP(), true, comp1, comp2, mSettings->library, true, false)) if (isSameExpression(mTokenizer->isCPP(), true, comp1, comp2, mSettings->library, true, true))
continue; // same expressions => only report that there are same expressions continue; // same expressions => only report that there are same expressions
if (!isSameExpression(mTokenizer->isCPP(), true, expr1, expr2, mSettings->library, true, false)) if (!isSameExpression(mTokenizer->isCPP(), true, expr1, expr2, mSettings->library, true, true))
continue; continue;
@ -1046,7 +1048,7 @@ void CheckCondition::checkIncorrectLogicOperator()
const std::string cond2str = conditionString(not2, expr2, op2, value2); const std::string cond2str = conditionString(not2, expr2, op2, value2);
if (printWarning && (alwaysTrue || alwaysFalse)) { if (printWarning && (alwaysTrue || alwaysFalse)) {
const std::string text = cond1str + " " + tok->str() + " " + cond2str; const std::string text = cond1str + " " + tok->str() + " " + cond2str;
incorrectLogicOperatorError(tok, text, alwaysTrue, inconclusive); incorrectLogicOperatorError(tok, text, alwaysTrue, inconclusive, errorPath);
} else if (printStyle && secondTrue) { } else if (printStyle && secondTrue) {
const std::string text = "If '" + cond1str + "', the comparison '" + cond2str + const std::string text = "If '" + cond1str + "', the comparison '" + cond2str +
"' is always true."; "' is always true.";
@ -1063,15 +1065,16 @@ void CheckCondition::checkIncorrectLogicOperator()
} }
} }
void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive) void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive, ErrorPath errors)
{ {
errors.emplace_back(tok, "");
if (always) if (always)
reportError(tok, Severity::warning, "incorrectLogicOperator", reportError(errors, Severity::warning, "incorrectLogicOperator",
"Logical disjunction always evaluates to true: " + condition + ".\n" "Logical disjunction always evaluates to true: " + condition + ".\n"
"Logical disjunction always evaluates to true: " + condition + ". " "Logical disjunction always evaluates to true: " + condition + ". "
"Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?", CWE571, inconclusive); "Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?", CWE571, inconclusive);
else else
reportError(tok, Severity::warning, "incorrectLogicOperator", reportError(errors, Severity::warning, "incorrectLogicOperator",
"Logical conjunction always evaluates to false: " + condition + ".\n" "Logical conjunction always evaluates to false: " + condition + ".\n"
"Logical conjunction always evaluates to false: " + condition + ". " "Logical conjunction always evaluates to false: " + condition + ". "
"Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?", CWE570, inconclusive); "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?", CWE570, inconclusive);

4
lib/checkcondition.h
View File

@ -137,7 +137,7 @@ private:
void identicalConditionAfterEarlyExitError(const Token *cond1, const Token *cond2, ErrorPath errorPath); void identicalConditionAfterEarlyExitError(const Token *cond1, const Token *cond2, ErrorPath errorPath);
void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive); void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive, ErrorPath errors);
void redundantConditionError(const Token *tok, const std::string &text, bool inconclusive); void redundantConditionError(const Token *tok, const std::string &text, bool inconclusive);
void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal); void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal);
@ -162,7 +162,7 @@ private:
c.oppositeInnerConditionError(nullptr, nullptr, errorPath); c.oppositeInnerConditionError(nullptr, nullptr, errorPath);
c.identicalInnerConditionError(nullptr, nullptr, errorPath); c.identicalInnerConditionError(nullptr, nullptr, errorPath);
c.identicalConditionAfterEarlyExitError(nullptr, nullptr, errorPath); c.identicalConditionAfterEarlyExitError(nullptr, nullptr, errorPath);
c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true, false); c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true, false, errorPath);
c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false); c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false);
c.moduloAlwaysTrueFalseError(nullptr, "1"); c.moduloAlwaysTrueFalseError(nullptr, "1");
c.clarifyConditionError(nullptr, true, false); c.clarifyConditionError(nullptr, true, false);

4
test/testcondition.cpp
View File

@ -1246,7 +1246,7 @@ private:
" if (a > x && a < y)\n" " if (a > x && a < y)\n"
" return;\n" " return;\n"
"}\n"); "}\n");
// ASSERT_EQUALS("[test.cpp:8]: (warning) Logical conjunction always evaluates to false: a > x && a < y.\n", errout.str()); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6] -> [test.cpp:8]: (warning) Logical conjunction always evaluates to false: a > x && a < y.\n", errout.str());
check("struct A {\n" check("struct A {\n"
" void f();\n" " void f();\n"
@ -1276,7 +1276,7 @@ private:
" if (a > x && a < y)\n" " if (a > x && a < y)\n"
" return;\n" " return;\n"
"}\n"); "}\n");
// ASSERT_EQUALS("[test.cpp:5]: (warning) Logical conjunction always evaluates to false: a > x && a < y.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:5]: (warning) Logical conjunction always evaluates to false: a > x && a < y.\n", errout.str());
} }
void secondAlwaysTrueFalseWhenFirstTrueError() { void secondAlwaysTrueFalseWhenFirstTrueError() {