From 3e99bff764b852b6455a252315dbefabbd3ca6e6 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Mon, 31 Aug 2020 01:48:48 -0500 Subject: [PATCH] Same expression when comparing with zero (#2762) --- lib/astutils.cpp | 44 +++++++++++++++++++++++++++++++++++ test/testcondition.cpp | 2 +- test/testother.cpp | 52 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 370dc0d55..6a6a49c0c 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -729,6 +729,11 @@ static bool isSameConstantValue(bool macro, const Token * const tok1, const Toke return isEqualKnownValue(tok1, tok2); } +static bool astIsBoolLike(const Token* tok) +{ + return astIsBool(tok) || astIsPointer(tok) || astIsSmartPointer(tok); +} + bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors) { if (tok1 == nullptr && tok2 == nullptr) @@ -777,6 +782,45 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 return isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, followVar, errors) && isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, followVar, errors); } + const Token* condTok = nullptr; + const Token* exprTok = nullptr; + if (Token::Match(tok1, "==|!=")) { + condTok = tok1; + exprTok = tok2; + } else if (Token::Match(tok2, "==|!=")) { + condTok = tok2; + exprTok = tok1; + } + if (condTok && condTok->astOperand1() && condTok->astOperand2() && !Token::Match(exprTok, "%comp%")) { + const Token* varTok1 = nullptr; + const Token* varTok2 = exprTok; + const ValueFlow::Value* value = nullptr; + if (condTok->astOperand1()->hasKnownIntValue()) { + value = &condTok->astOperand1()->values().front(); + varTok1 = condTok->astOperand2(); + } else if (condTok->astOperand2()->hasKnownIntValue()) { + value = &condTok->astOperand2()->values().front(); + varTok1 = condTok->astOperand1(); + } + if (Token::simpleMatch(exprTok, "!")) + varTok2 = exprTok->astOperand1(); + bool compare = false; + if (value) { + if (value->intvalue == 0 && Token::simpleMatch(exprTok, "!") && Token::simpleMatch(condTok, "==")) { + compare = true; + } else if (value->intvalue == 0 && !Token::simpleMatch(exprTok, "!") && Token::simpleMatch(condTok, "!=")) { + compare = true; + } else if (value->intvalue != 0 && Token::simpleMatch(exprTok, "!") && Token::simpleMatch(condTok, "!=")) { + compare = true; + } else if (value->intvalue != 0 && !Token::simpleMatch(exprTok, "!") && Token::simpleMatch(condTok, "==")) { + compare = true; + } + + } + if (compare && astIsBoolLike(varTok1) && astIsBoolLike(varTok2)) + return isSameExpression(cpp, macro, varTok1, varTok2, library, pure, followVar, errors); + + } return false; } if (macro && (tok1->isExpandedMacro() || tok2->isExpandedMacro() || tok1->isTemplateArg() || tok2->isTemplateArg())) diff --git a/test/testcondition.cpp b/test/testcondition.cpp index ddf843fa7..588f10a77 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -3205,7 +3205,7 @@ private: " if (handle) return 1;\n" " else return 0;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition 'handle' is always false\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Identical condition 'handle!=0', second condition is always false\n", errout.str()); check("void f(void* x, void* y) {\n" " if (x == nullptr && y == nullptr)\n" diff --git a/test/testother.cpp b/test/testother.cpp index 73a0c0a7a..906db1ab7 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -149,6 +149,7 @@ private: TEST_CASE(duplicateValueTernary); TEST_CASE(duplicateExpressionTernary); // #6391 TEST_CASE(duplicateExpressionTemplate); // #6930 + TEST_CASE(duplicateExpressionCompareWithZero); TEST_CASE(oppositeExpression); TEST_CASE(duplicateVarExpression); TEST_CASE(duplicateVarExpressionUnique); @@ -5073,6 +5074,57 @@ private: ASSERT_EQUALS("", errout.str()); } + void duplicateExpressionCompareWithZero() { + check("void f(int* x, bool b) {\n" + " if ((x && b) || (x != 0 && b)) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||' because 'x&&b' and 'x!=0&&b' represent the same value.\n", errout.str()); + + check("void f(int* x, bool b) {\n" + " if ((x != 0 && b) || (x && b)) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||' because 'x!=0&&b' and 'x&&b' represent the same value.\n", errout.str()); + + check("void f(int* x, bool b) {\n" + " if ((x && b) || (b && x != 0)) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||' because 'x&&b' and 'b&&x!=0' represent the same value.\n", errout.str()); + + check("void f(int* x, bool b) {\n" + " if ((!x && b) || (x == 0 && b)) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||' because '!x&&b' and 'x==0&&b' represent the same value.\n", errout.str()); + + check("void f(int* x, bool b) {\n" + " if ((x == 0 && b) || (!x && b)) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||' because 'x==0&&b' and '!x&&b' represent the same value.\n", errout.str()); + + check("void f(int* x, bool b) {\n" + " if ((!x && b) || (b && x == 0)) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||' because '!x&&b' and 'b&&x==0' represent the same value.\n", errout.str()); + + check("struct A {\n" + " int* getX() const;\n" + " bool getB() const;\n" + " void f() {\n" + " if ((getX() && getB()) || (getX() != 0 && getB())) {}\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:5]: (style) Same expression on both sides of '||' because 'getX()&&getB()' and 'getX()!=0&&getB()' represent the same value.\n", errout.str()); + + check("void f(int* x, bool b) {\n" + " if ((x && b) || (x == 0 && b)) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(int* x, bool b) {\n" + " if ((!x && b) || (x != 0 && b)) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void oppositeExpression() { check("void f(bool a) { if(a && !a) {} }"); ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '&&'.\n", errout.str());