Same expression when comparing with zero (#2762)

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()))

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"

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());