Fix issue 470: Condition is always true or false on logical operators (#1294)

* Fix issue 470: Condition is always true or false on logical operators

* Dont warn on literals

* Compute logical operators using valueflow

* Fix FP when using literals

* Always warn on subconditions that are always true

* Use percent matches first

* Add test for logical operators

* Check if parent is null

lib/checkcondition.cpp

@@ -885,7 +885,6 @@ void CheckCondition::checkIncorrectLogicOperator()
                 continue;
             }
 
-
             // 'A && (!A || B)' is equivalent to 'A && B'
             // 'A || (!A && B)' is equivalent to 'A || B'
             if (printStyle &&
@@ -1192,9 +1191,8 @@ void CheckCondition::alwaysTrueFalse()
                 continue;
 
             const bool constIfWhileExpression =
-                tok->astParent()
+                tok->astParent() && Token::Match(tok->astTop()->astOperand1(), "if|while") &&
-                && Token::Match(tok->astParent()->astOperand1(), "if|while")
+                (Token::Match(tok->astParent(), "%oror%|&&") || Token::Match(tok->astParent()->astOperand1(), "if|while"));
-                && !tok->isBoolean();
             const bool constValExpr = tok->isNumber() && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number in boolean expression
             const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
 

lib/valueflow.cpp

@@ -444,6 +444,18 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
             return;
         }
 
+        // known result when a operand is true.
+        if (Token::simpleMatch(parent, "&&") && value.isKnown() && value.isIntValue() && value.intvalue==0) {
+            setTokenValue(parent, value, settings);
+            return;
+        }
+
+        // known result when a operand is false.
+        if (Token::simpleMatch(parent, "||") && value.isKnown() && value.isIntValue() && value.intvalue!=0) {
+            setTokenValue(parent, value, settings);
+            return;
+        }
+
         for (const ValueFlow::Value &value1 : parent->astOperand1()->values()) {
             if (!value1.isIntValue() && !value1.isFloatValue() && !value1.isTokValue())
                 continue;

test/testcondition.cpp

@@ -105,6 +105,7 @@ private:
 
         TEST_CASE(checkInvalidTestForOverflow);
         TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile);
+        TEST_CASE(alwaysTrueFalseInLogicalOperators);
         TEST_CASE(pointerAdditionResultNotNull);
     }
 
@@ -2507,6 +2508,36 @@ private:
         ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a+1' is always true\n", errout.str());
     }
 
+    void alwaysTrueFalseInLogicalOperators() {
+        check("bool f();\n"
+              "void foo() { bool x = true; if(x||f()) {}}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always true\n", errout.str());
+
+        check("void foo(bool b) { bool x = true; if(x||b) {}}\n");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Condition 'x' is always true\n", errout.str());
+
+        check("void foo(bool b) { if(true||b) {}}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("bool f();\n"
+              "void foo() { bool x = false; if(x||f()) {}}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always false\n", errout.str());
+
+        check("bool f();\n"
+              "void foo() { bool x = false; if(x&&f()) {}}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always false\n", errout.str());
+
+        check("void foo(bool b) { bool x = false; if(x&&b) {}}\n");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Condition 'x' is always false\n", errout.str());
+
+        check("void foo(bool b) { if(false&&b) {}}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("bool f();\n"
+              "void foo() { bool x = true; if(x&&f()) {}}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'x' is always true\n", errout.str());
+    }
+
     void pointerAdditionResultNotNull() {
         check("void f(char *ptr) {\n"
               "  if (ptr + 1 != 0);\n"

test/testvalueflow.cpp

@@ -559,6 +559,56 @@ private:
         ASSERT_EQUALS(1U, values.size());
         ASSERT_EQUALS(-10, values.back().intvalue);
 
+        // Logical and
+        code = "void f(bool b) {\n" 
+                "   bool x = false && b;\n"
+                "   bool a = x;\n"
+                "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 0));
+
+        code = "void f(bool b) {\n" 
+                "   bool x = b && false;\n"
+                "   bool a = x;\n"
+                "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 0));
+
+        code = "void f(bool b) {\n" 
+                "   bool x = true && b;\n"
+                "   bool a = x;\n"
+                "}";
+        ASSERT_EQUALS(false, testValueOfX(code, 3U, 1));
+
+        code = "void f(bool b) {\n" 
+                "   bool x = b && true;\n"
+                "   bool a = x;\n"
+                "}";
+        ASSERT_EQUALS(false, testValueOfX(code, 3U, 1));
+
+        // Logical or
+        code = "void f(bool b) {\n" 
+                "   bool x = true || b;\n"
+                "   bool a = x;\n"
+                "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 1));
+
+        code = "void f(bool b) {\n" 
+                "   bool x = b || true;\n"
+                "   bool a = x;\n"
+                "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 1));
+
+        code = "void f(bool b) {\n" 
+                "   bool x = false || b;\n"
+                "   bool a = x;\n"
+                "}";
+        ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
+
+        code = "void f(bool b) {\n" 
+                "   bool x = b || false;\n"
+                "   bool a = x;\n"
+                "}";
+        ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
+
         // function call => calculation
         code  = "void f(int x) {\n"
                 "    a = x + 8;\n"