From e170a452309f4338f2380996eaed4550dc6383cb Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Thu, 4 Oct 2018 14:17:47 -0500 Subject: [PATCH] Enable followVar for opposite expressions (#1404) Enable followVar for opposite expressions --- lib/astutils.cpp | 4 ++-- lib/checkother.cpp | 13 ++++++------- lib/checkother.h | 4 ++-- test/testcondition.cpp | 2 +- test/testother.cpp | 34 +++++++++++++++++++++++++--------- 5 files changed, 36 insertions(+), 21 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 59985e954..a4f948f9c 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -450,7 +450,7 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token } if (cond2->str() == "!") - return isOppositeCond(isNot, cpp, cond2, cond1, library, pure, followVar); + return isOppositeCond(isNot, cpp, cond2, cond1, library, pure, followVar, errors); if (!isNot) { if (cond1->str() == "==" && cond2->str() == "==") { @@ -553,7 +553,7 @@ bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * cons { if (!tok1 || !tok2) return false; - if (isOppositeCond(true, cpp, tok1, tok2, library, pure, followVar)) + if (isOppositeCond(true, cpp, tok1, tok2, library, pure, followVar, errors)) return true; if (tok1->isUnaryOp("-")) return isSameExpression(cpp, true, tok1->astOperand1(), tok2, library, pure, followVar, errors); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index d1258a6f1..937bc295b 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2005,10 +2005,10 @@ void CheckOther::checkDuplicateExpression() } } } else if (styleEnabled && - isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false, false, &errorPath) && + isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false, true, &errorPath) && !Token::Match(tok, "=|-|-=|/|/=") && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { - oppositeExpressionError(tok, tok, tok->str(), errorPath); + oppositeExpressionError(tok->astOperand1(), tok->astOperand2(), 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())) duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath); @@ -2037,12 +2037,11 @@ void CheckOther::checkDuplicateExpression() } } -void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op, ErrorPath errors) +void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors) { - if (tok1) - errors.emplace_back(tok1, ""); - if (tok2) - errors.emplace_back(tok2, ""); + errors.emplace_back(opTok, ""); + + const std::string& op = opTok ? opTok->str() : "&&"; reportError(errors, Severity::style, "oppositeExpression", "Opposite expression on both sides of \'" + op + "\'.\n" "Finding the opposite expression on both sides of an operator is suspicious and might " diff --git a/lib/checkother.h b/lib/checkother.h index ddae3aee7..b150e53c9 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -239,7 +239,7 @@ private: void misusedScopeObjectError(const Token *tok, const std::string &varname); void duplicateBranchError(const Token *tok1, const Token *tok2); void duplicateAssignExpressionError(const Token *tok1, const Token *tok2); - void oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op, ErrorPath errors); + void oppositeExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors); void duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors); void duplicateValueTernaryError(const Token *tok); void duplicateExpressionTernaryError(const Token *tok, ErrorPath errors); @@ -302,7 +302,7 @@ private: c.clarifyCalculationError(nullptr, "+"); c.clarifyStatementError(nullptr); c.duplicateBranchError(nullptr, nullptr); - c.oppositeExpressionError(nullptr, nullptr, "&&", errorPath); + c.oppositeExpressionError(nullptr, nullptr, nullptr, errorPath); c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath); c.duplicateValueTernaryError(nullptr); c.duplicateExpressionTernaryError(nullptr, errorPath); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 7e2de11ea..75b1f28b7 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -1601,7 +1601,7 @@ private: " if(!b) {}\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str()); check("void foo(unsigned u) {\n" " if (u != 0) {\n" diff --git a/test/testother.cpp b/test/testother.cpp index 78fd3030c..0bed145ce 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -4232,34 +4232,50 @@ private: void oppositeExpression() { check("void f(bool a) { if(a && !a) {} }"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '&&'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '&&'.\n", errout.str()); check("void f(bool a) { if(a != !a) {} }"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str()); check("void f(bool a) { if( a == !(a) ) {}}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str()); check("void f(bool a) { if( a != !(a) ) {}}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str()); check("void f(bool a) { if( !(a) == a ) {}}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str()); check("void f(bool a) { if( !(a) != a ) {}}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str()); check("void f(bool a) { if( !(!a) == !(a) ) {}}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str()); check("void f(bool a) { if( !(!a) != !(a) ) {}}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str()); + + check("void f1(bool a) {\n" + " const bool b = a;\n" + " if( a == !(b) ) {}\n" + " if( b == !(a) ) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Opposite expression on both sides of '=='.\n" + "[test.cpp:2] -> [test.cpp:4]: (style) Opposite expression on both sides of '=='.\n", errout.str()); + + check("void f2(bool *a) {\n" + " const bool b = *a;\n" + " if( *a == !(b) ) {}\n" + " if( b == !(*a) ) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Opposite expression on both sides of '=='.\n" + "[test.cpp:2] -> [test.cpp:4]: (style) Opposite expression on both sides of '=='.\n", errout.str()); check("void f(bool a) { a = !a; }"); ASSERT_EQUALS("", errout.str()); check("void f(int a) { if( a < -a ) {}}"); - ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '<'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '<'.\n", errout.str()); check("void f(int a) { a -= -a; }"); ASSERT_EQUALS("", errout.str());