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

Enable checking duplicate expressions across associative operators (#1445) 

* Enable checking duplicate expressions across associative operators

* Remove bitshift operators and check for streamRead
This commit is contained in:
Paul Fultz II 2018-10-21 00:09:20 -05:00 committed by Daniel Marjamäki
parent f3b127032a
commit 2989c44f59
4 changed files with 20 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
lib/astutils.cpp
View File

@ -587,7 +587,7 @@ bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * cons
return false; return false;
} }
bool isConstExpression(const Token *tok, const Library& library, bool pure) bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp)
{ {
if (!tok) if (!tok)
return true; return true;
@ -599,10 +599,14 @@ bool isConstExpression(const Token *tok, const Library& library, bool pure)
} }
if (tok->tokType() == Token::eIncDecOp) if (tok->tokType() == Token::eIncDecOp)
return false; return false;
if(tok->isAssignmentOp())
return false;
if(isLikelyStreamRead(cpp, tok))
return false;
// bailout when we see ({..}) // bailout when we see ({..})
if (tok->str() == "{") if (tok->str() == "{")
return false; return false;
return isConstExpression(tok->astOperand1(), library, pure) && isConstExpression(tok->astOperand2(), library, pure); return isConstExpression(tok->astOperand1(), library, pure, cpp) && isConstExpression(tok->astOperand2(), library, pure, cpp);
} }
bool isWithoutSideEffects(bool cpp, const Token* tok) bool isWithoutSideEffects(bool cpp, const Token* tok)

2
lib/astutils.h
View File

@ -78,7 +78,7 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr); bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * const tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr);
bool isConstExpression(const Token *tok, const Library& library, bool pure); bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp);
bool isWithoutSideEffects(bool cpp, const Token* tok); bool isWithoutSideEffects(bool cpp, const Token* tok);

15
lib/checkother.cpp
View File

@ -2063,19 +2063,16 @@ void CheckOther::checkDuplicateExpression()
isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) {
oppositeExpressionError(tok, errorPath); oppositeExpressionError(tok, errorPath);
} else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative } else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative
if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), mSettings->library, true, false, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2())) if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), mSettings->library, true, true, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2()))
duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath); duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath);
else if (tok->astOperand2()) { else if (tok->astOperand2() && isConstExpression(tok->astOperand1(), mSettings->library, true, mTokenizer->isCPP())) {
const Token *ast1 = tok->astOperand1(); const Token *ast1 = tok->astOperand1();
while (ast1 && tok->str() == ast1->str()) { while (ast1 && tok->str() == ast1->str()) {
if (isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand1(), tok->astOperand2(), mSettings->library, true, false, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand1())) if (isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand1(), tok->astOperand2(), mSettings->library, true, true, &errorPath) &&
// TODO: warn if variables are unchanged. See #5683 isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand1()) &&
isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2()))
// Probably the message should be changed to 'duplicate expressions X in condition or something like that'. // Probably the message should be changed to 'duplicate expressions X in condition or something like that'.
;//duplicateExpressionError(ast1->astOperand1(), tok->astOperand2(), tok, errorPath); duplicateExpressionError(ast1->astOperand1(), tok->astOperand2(), tok, errorPath);
else if (styleEnabled && isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand2(), tok->astOperand2(), mSettings->library, true, false, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2()))
duplicateExpressionError(ast1->astOperand2(), tok->astOperand2(), tok, errorPath);
if (!isConstExpression(ast1->astOperand2(), mSettings->library, true))
break;
ast1 = ast1->astOperand1(); ast1 = ast1->astOperand1();
} }
} }

8
test/testother.cpp
View File

@ -3571,7 +3571,7 @@ private:
check("void foo() {\n" check("void foo() {\n"
" if (x!=2 || y!=3 || x!=2) {}\n" " if (x!=2 || y!=3 || x!=2) {}\n"
"}"); "}");
TODO_ASSERT_EQUALS("error", "", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (x!=2 && (x=y) && x!=2) {}\n" " if (x!=2 && (x=y) && x!=2) {}\n"
@ -4041,6 +4041,12 @@ private:
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("bool f(bool a, bool b) {\n"
" const bool c = a;\n"
" return a && b && c;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '&&' because 'a' and 'c' represent the same value.\n", errout.str());
} }
void duplicateExpression8() { void duplicateExpression8() {