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

AST: improved 'incorrect logic' checking when there are nested expressions

This commit is contained in:
Daniel Marjamäki 2013-11-15 06:51:35 +01:00
parent d1721b9d1b
commit 5af2fe6e5b
2 changed files with 34 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
lib/checkother.cpp
View File

@ -1319,37 +1319,43 @@ void CheckOther::checkIncorrectLogicOperator()
if (_settings->ast) { if (_settings->ast) {
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
if (Token::Match(tok, "&&|%oror%")) { if (Token::Match(tok, "&&|%oror%")) {
if (!tok->astOperand1() || !tok->astOperand1()->astOperand1() || !tok->astOperand1()->astOperand2()) // Comparison #1 (LHS)
const Token *comp1 = tok->astOperand1();
if (comp1 && comp1->str() == tok->str())
comp1 = comp1->astOperand2();
// Comparison #2 (RHS)
const Token *comp2 = tok->astOperand2();
if (!comp1 || !comp1->isComparisonOp() || !comp1->astOperand1() || !comp1->astOperand2())
continue; continue;
if (!tok->astOperand2() || !tok->astOperand2()->astOperand1() || !tok->astOperand2()->astOperand2()) if (!comp2 || !comp2->isComparisonOp() || !comp2->astOperand1() || !comp2->astOperand2())
continue;
if (!tok->astOperand1()->isComparisonOp() || !tok->astOperand2()->isComparisonOp())
continue; continue;
std::string op1, value1; std::string op1, value1;
const Token *expr1; const Token *expr1;
if (tok->astOperand1()->astOperand1()->isLiteral()) { if (comp1->astOperand1()->isLiteral()) {
op1 = invertOperatorForOperandSwap(tok->astOperand1()->str()); op1 = invertOperatorForOperandSwap(comp1->str());
value1 = tok->astOperand1()->astOperand1()->str(); value1 = comp1->astOperand1()->str();
expr1 = tok->astOperand1()->astOperand2(); expr1 = comp1->astOperand2();
} else if (tok->astOperand1()->astOperand2()->isLiteral()) { } else if (comp1->astOperand2()->isLiteral()) {
op1 = tok->astOperand1()->str(); op1 = comp1->str();
value1 = tok->astOperand1()->astOperand2()->str(); value1 = comp1->astOperand2()->str();
expr1 = tok->astOperand1()->astOperand1(); expr1 = comp1->astOperand1();
} else { } else {
continue; continue;
} }
std::string op2, value2; std::string op2, value2;
const Token *expr2; const Token *expr2;
if (tok->astOperand2()->astOperand1()->isLiteral()) { if (comp2->astOperand1()->isLiteral()) {
op2 = invertOperatorForOperandSwap(tok->astOperand2()->str()); op2 = invertOperatorForOperandSwap(comp2->str());
value2 = tok->astOperand2()->astOperand1()->str(); value2 = comp2->astOperand1()->str();
expr2 = tok->astOperand2()->astOperand2(); expr2 = comp2->astOperand2();
} else if (tok->astOperand2()->astOperand2()->isLiteral()) { } else if (comp2->astOperand2()->isLiteral()) {
op2 = tok->astOperand2()->str(); op2 = comp2->str();
value2 = tok->astOperand2()->astOperand2()->str(); value2 = comp2->astOperand2()->str();
expr2 = tok->astOperand2()->astOperand1(); expr2 = comp2->astOperand1();
} else { } else {
continue; continue;
} }
@ -1361,7 +1367,7 @@ void CheckOther::checkIncorrectLogicOperator()
continue; continue;
const std::set<std::string> constStandardFunctions; const std::set<std::string> constStandardFunctions;
if (isSameExpression(tok->astOperand1(), tok->astOperand2(), constStandardFunctions)) if (isSameExpression(comp1, comp2, constStandardFunctions))
continue; // same expressions => only report that continue; // same expressions => only report that
if (!isSameExpression(expr1, expr2, constStandardFunctions)) if (!isSameExpression(expr1, expr2, constStandardFunctions))
continue; continue;

9
test/testother.cpp
View File

@ -128,8 +128,8 @@ private:
TEST_CASE(incorrectLogicOperator2); TEST_CASE(incorrectLogicOperator2);
TEST_CASE(incorrectLogicOperator3); TEST_CASE(incorrectLogicOperator3);
TEST_CASE(incorrectLogicOperator4); TEST_CASE(incorrectLogicOperator4);
TEST_CASE(incorrectLogicOperator5); TEST_CASE(incorrectLogicOperator5); // complex expressions
TEST_CASE(incorrectLogicOperator6); TEST_CASE(incorrectLogicOperator6); // char literals
TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError); TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError);
TEST_CASE(incorrectLogicOp_condSwapping); TEST_CASE(incorrectLogicOp_condSwapping);
TEST_CASE(sameExpression); TEST_CASE(sameExpression);
@ -3734,6 +3734,11 @@ private:
); );
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str());
check("void f(int x) {\n" // ast..
" if (y == 1 && x == 1 && x == 7) { }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1 && x == 7.\n", errout.str());
check("void f(int x, int y) {\n" check("void f(int x, int y) {\n"
" if (x != 1 || y != 1)\n" " if (x != 1 || y != 1)\n"
" a++;\n" " a++;\n"