Fixed #4842 (condition is always true (variable is assigned constant value and then used in condition))

lib/checkcondition.cpp

@@ -930,3 +930,40 @@ void CheckCondition::clarifyConditionError(const Token *tok, bool assign, bool b
                 "clarifyCondition",
                 errmsg);
 }
+
+
+void CheckCondition::alwaysTrueFalse()
+{
+    if (!_settings->isEnabled("style"))
+        return;
+
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * scope = symbolDatabase->functionScopes[i];
+        for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
+            if (!Token::Match(tok, "%comp%|!"))
+                continue;
+            if (tok->link()) // don't write false positives when templates are used
+                continue;
+            if (tok->values.size() != 1U)
+                continue;
+            if (tok->values.front().valueKind != ValueFlow::Value::Known)
+                continue;
+
+            if (tok->astParent() && Token::Match(tok->astParent()->previous(), "%name% ("))
+                alwaysTrueFalseError(tok, tok->values.front().intvalue != 0);
+        }
+    }
+}
+
+void CheckCondition::alwaysTrueFalseError(const Token *tok, bool knownResult)
+{
+    const std::string expr = tok ? tok->expressionString() : std::string("x");
+
+    reportError(tok,
+                Severity::style,
+                "knownConditionTrueFalse",
+                "Condition " + expr + " is always " + (knownResult ? "true" : "false"));
+}

lib/checkcondition.h

@@ -59,6 +59,7 @@ public:
         checkCondition.oppositeInnerCondition();
         checkCondition.checkIncorrectLogicOperator();
         checkCondition.checkModuloAlwaysTrueFalse();
+        checkCondition.alwaysTrueFalse();
     }
 
     /** mismatching assignment / comparison */
@@ -93,6 +94,9 @@ public:
     /** @brief Suspicious condition (assignment+comparison) */
     void clarifyCondition();
 
+    /** @brief Condition is always true/false */
+    void alwaysTrueFalse();
+
 private:
 
     bool isOverlappingCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions) const;
@@ -123,6 +127,8 @@ private:
 
     void clarifyConditionError(const Token *tok, bool assign, bool boolop);
 
+    void alwaysTrueFalseError(const Token *tok, bool knownResult);
+
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckCondition c(0, settings, errorLogger);
 
@@ -136,6 +142,7 @@ private:
         c.redundantConditionError(0, "If x > 11 the condition x > 10 is always true.");
         c.moduloAlwaysTrueFalseError(0, "1");
         c.clarifyConditionError(0, true, false);
+        c.alwaysTrueFalseError(0, true);
     }
 
     static std::string myName() {
@@ -152,7 +159,8 @@ private:
                "- Find dead code which is inaccessible due to the counter-conditions check in nested if statements\n"
                "- condition that is always true/false\n"
                "- mutual exclusion over || always evaluating to true\n"
-               "- Comparisons of modulo results that are always true/false.\n";
+               "- Comparisons of modulo results that are always true/false.\n"
+               "- Known variable values => condition is always true/false\n";
     }
 };
 /// @}

lib/valueflow.cpp

@@ -407,6 +407,18 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value)
         }
     }
 
+    // !
+    else if (parent->str() == "!") {
+        std::list<ValueFlow::Value>::const_iterator it;
+        for (it = tok->values.begin(); it != tok->values.end(); ++it) {
+            if (it->tokvalue)
+                continue;
+            ValueFlow::Value v(*it);
+            v.intvalue = !v.intvalue;
+            setTokenValue(parent, v);
+        }
+    }
+
     // Array element
     else if (parent->str() == "[" && parent->astOperand1() && parent->astOperand2()) {
         std::list<ValueFlow::Value>::const_iterator value1, value2;

test/testcondition.cpp

@@ -62,6 +62,8 @@ private:
         TEST_CASE(clarifyCondition4);     // ticket #3110
         TEST_CASE(clarifyCondition5);     // #3609 CWinTraits<WS_CHILD|WS_VISIBLE>..
         TEST_CASE(clarifyCondition6);     // #3818
+
+        TEST_CASE(alwaysTrue);
     }
 
     void check(const char code[], bool validate=true, const char* filename = "test.cpp") {
@@ -1487,6 +1489,15 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void alwaysTrue() {
+        check("void f() {\n" // #4842
+              "  int x = 0;\n"
+              "  if (a) { return; }\n" // <- this is just here to fool simplifyKnownVariabels
+              "  if (!x) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (style) Condition !x is always true\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestCondition)

test/testvalueflow.cpp

@@ -265,6 +265,15 @@ private:
         ASSERT_EQUALS(2, values.front().intvalue);
         ASSERT_EQUALS(3, values.back().intvalue);
 
+        // !
+        code  = "void f(int x) {\n"
+                "    a = !x;\n"
+                "    if (x==0) {}\n"
+                "}";
+        values = tokenValues(code,"!");
+        ASSERT_EQUALS(1U, values.size());
+        ASSERT_EQUALS(1, values.back().intvalue);
+
         // function call => calculation
         code  = "void f(int x) {\n"
                 "    a = x + 8;\n"