From 2989c44f59e70e197909825083385be5ef44576c Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sun, 21 Oct 2018 00:09:20 -0500 Subject: [PATCH] Enable checking duplicate expressions across associative operators (#1445) * Enable checking duplicate expressions across associative operators * Remove bitshift operators and check for streamRead --- lib/astutils.cpp | 8 ++++++-- lib/astutils.h | 2 +- lib/checkother.cpp | 15 ++++++--------- test/testother.cpp | 8 +++++++- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 33475db9f..ad9f9adc3 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -587,7 +587,7 @@ bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * cons 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) return true; @@ -599,10 +599,14 @@ bool isConstExpression(const Token *tok, const Library& library, bool pure) } if (tok->tokType() == Token::eIncDecOp) return false; + if(tok->isAssignmentOp()) + return false; + if(isLikelyStreamRead(cpp, tok)) + return false; // bailout when we see ({..}) if (tok->str() == "{") 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) diff --git a/lib/astutils.h b/lib/astutils.h index b170f25eb..80ef0d23c 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -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 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); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 3727cc4d8..d6f3244a3 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2063,19 +2063,16 @@ void CheckOther::checkDuplicateExpression() isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { oppositeExpressionError(tok, errorPath); } 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); - else if (tok->astOperand2()) { + else if (tok->astOperand2() && isConstExpression(tok->astOperand1(), mSettings->library, true, mTokenizer->isCPP())) { const Token *ast1 = tok->astOperand1(); 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())) - // TODO: warn if variables are unchanged. See #5683 + if (isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand1(), tok->astOperand2(), mSettings->library, true, true, &errorPath) && + 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'. - ;//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; + duplicateExpressionError(ast1->astOperand1(), tok->astOperand2(), tok, errorPath); ast1 = ast1->astOperand1(); } } diff --git a/test/testother.cpp b/test/testother.cpp index b1d17398b..05334ed18 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -3571,7 +3571,7 @@ private: check("void foo() {\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" " if (x!=2 && (x=y) && x!=2) {}\n" @@ -4041,6 +4041,12 @@ private: " }\n" "}\n"); 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() {