Improve valueflow after pushing to container (#4803)

gui/projectfiledialog.cpp

@@ -587,7 +587,7 @@ QString ProjectFileDialog::getImportProject() const
 
 void ProjectFileDialog::addIncludeDir(const QString &dir)
 {
-    if (dir.isNull() || dir.isEmpty())
+    if (dir.isEmpty())
         return;
 
     const QString newdir = QDir::toNativeSeparators(dir);
@@ -598,7 +598,7 @@ void ProjectFileDialog::addIncludeDir(const QString &dir)
 
 void ProjectFileDialog::addCheckPath(const QString &path)
 {
-    if (path.isNull() || path.isEmpty())
+    if (path.isEmpty())
         return;
 
     const QString newpath = QDir::toNativeSeparators(path);
@@ -609,7 +609,7 @@ void ProjectFileDialog::addCheckPath(const QString &path)
 
 void ProjectFileDialog::addExcludePath(const QString &path)
 {
-    if (path.isNull() || path.isEmpty())
+    if (path.isEmpty())
         return;
 
     const QString newpath = QDir::toNativeSeparators(path);

lib/astutils.cpp

@@ -2863,10 +2863,52 @@ int getArgumentPos(const Variable* var, const Function* f)
     return std::distance(f->argumentList.cbegin(), arg_it);
 }
 
+const Token* getIteratorExpression(const Token* tok)
+{
+    if (!tok)
+        return nullptr;
+    if (tok->isUnaryOp("*"))
+        return nullptr;
+    if (!tok->isName()) {
+        const Token* iter1 = getIteratorExpression(tok->astOperand1());
+        if (iter1)
+            return iter1;
+        if (tok->str() == "(")
+            return nullptr;
+        const Token* iter2 = getIteratorExpression(tok->astOperand2());
+        if (iter2)
+            return iter2;
+    } else if (Token::Match(tok, "begin|cbegin|rbegin|crbegin|end|cend|rend|crend (")) {
+        if (Token::Match(tok->previous(), ". %name% ( ) !!."))
+            return tok->previous()->astOperand1();
+        if (!Token::simpleMatch(tok->previous(), ".") && Token::Match(tok, "%name% ( !!)") &&
+            !Token::simpleMatch(tok->linkAt(1), ") ."))
+            return tok->next()->astOperand2();
+    }
+    return nullptr;
+}
+
 bool isIteratorPair(std::vector<const Token*> args)
 {
-    return args.size() == 2 &&
+    if (args.size() != 2)
-           ((astIsIterator(args[0]) && astIsIterator(args[1])) || (astIsPointer(args[0]) && astIsPointer(args[1])));
+        return false;
+    if (astIsPointer(args[0]) && astIsPointer(args[1]))
+        return true;
+    // Check if iterator is from same container
+    const Token* tok1 = nullptr;
+    const Token* tok2 = nullptr;
+    if (astIsIterator(args[0]) && astIsIterator(args[1])) {
+        tok1 = ValueFlow::getLifetimeObjValue(args[0]).tokvalue;
+        tok2 = ValueFlow::getLifetimeObjValue(args[1]).tokvalue;
+        if (!tok1 || !tok2)
+            return true;
+    } else {
+        tok1 = getIteratorExpression(args[0]);
+        tok2 = getIteratorExpression(args[1]);
+    }
+    if (tok1 && tok2)
+        return tok1->exprId() == tok2->exprId();
+    return tok1 || tok2;
 }
 
 const Token *findLambdaStartToken(const Token *last)

lib/astutils.h

@@ -375,6 +375,8 @@ std::vector<const Token *> getArguments(const Token *ftok);
 
 int getArgumentPos(const Variable* var, const Function* f);
 
+const Token* getIteratorExpression(const Token* tok);
+
 /**
  * Are the arguments a pair of iterators/pointers?
  */

lib/checkstl.cpp

@@ -678,30 +678,6 @@ void CheckStl::sameIteratorExpressionError(const Token *tok)
     reportError(tok, Severity::style, "sameIteratorExpression", "Same iterators expression are used for algorithm.", CWE664, Certainty::normal);
 }
 
-static const Token * getIteratorExpression(const Token * tok)
-{
-    if (!tok)
-        return nullptr;
-    if (tok->isUnaryOp("*"))
-        return nullptr;
-    if (!tok->isName()) {
-        const Token *iter1 = getIteratorExpression(tok->astOperand1());
-        if (iter1)
-            return iter1;
-        if (tok->str() == "(")
-            return nullptr;
-        const Token *iter2 = getIteratorExpression(tok->astOperand2());
-        if (iter2)
-            return iter2;
-    } else if (Token::Match(tok, "begin|cbegin|rbegin|crbegin|end|cend|rend|crend (")) {
-        if (Token::Match(tok->previous(), ". %name% ( ) !!."))
-            return tok->previous()->astOperand1();
-        if (!Token::simpleMatch(tok->previous(), ".") && Token::Match(tok, "%name% ( !!)") && !Token::simpleMatch(tok->linkAt(1), ") ."))
-            return tok->next()->astOperand2();
-    }
-    return nullptr;
-}
-
 static const Token* getAddressContainer(const Token* tok)
 {
     if (Token::simpleMatch(tok, "[") && tok->astOperand1())

lib/preprocessor.cpp

@@ -498,7 +498,7 @@ static void getConfigs(const simplecpp::TokenList &tokens, std::set<std::string>
                     if (c.find(configs_ifndef.back()) != std::string::npos)
                         ret.insert(c);
                     else if (c.empty())
-                        ret.insert(configs.empty() ? configs_ifndef.back() : "");
+                        ret.insert("");
                     else
                         ret.insert(c + ";" + configs_ifndef.back());
                 }

lib/valueflow.cpp

@@ -665,8 +665,11 @@ static void setTokenValue(Token* tok,
                 setTokenValue(parent->astParent(), v, settings);
             } else if (yields == Library::Container::Yield::EMPTY) {
                 ValueFlow::Value v(value);
-                v.intvalue = !v.intvalue;
                 v.valueType = ValueFlow::Value::ValueType::INT;
+                if (value.isImpossible() && value.intvalue == 0)
+                    v.setKnown();
+                else
+                    v.intvalue = !v.intvalue;
                 setTokenValue(parent->astParent(), v, settings);
             }
         } else if (Token::Match(parent->previous(), "%name% (")) {
@@ -8431,6 +8434,11 @@ static void valueFlowContainerSize(TokenList* tokenlist,
                     value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
                     value.setKnown();
                     valueFlowForward(tok->linkAt(2), containerTok, value, tokenlist);
+                } else if (action == Library::Container::Action::PUSH && !isIteratorPair(getArguments(tok->tokAt(2)))) {
+                    ValueFlow::Value value(0);
+                    value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
+                    value.setImpossible();
+                    valueFlowForward(tok->linkAt(2), containerTok, value, tokenlist);
                 }
             }
         }

test/testcondition.cpp

@@ -2585,7 +2585,7 @@ private:
         ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
 
         check("void f1(const std::string &s) { if(s.size() < 0) if(s.empty()) {}} "); // <- CheckOther reports: checking if unsigned expression is less than zero
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Condition 's.empty()' is always false\n", errout.str());
 
         check("void f1(const std::string &s) { if(s.empty()) if(s.size() > 42) {}}");
         ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
@@ -4971,6 +4971,22 @@ private:
               "    return a.empty() || (b.empty() && a.empty());\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Return value 'a.empty()' is always false\n", errout.str());
+
+        check("struct A {\n"
+              "    struct iterator;\n"
+              "    iterator begin() const;\n"
+              "    iterator end() const;\n"
+              "};\n"
+              "A g();\n"
+              "void f(bool b) {\n"
+              "    std::set<int> s;\n"
+              "    auto v = g();\n"
+              "    s.insert(v.begin(), v.end());\n"
+              "    if(!b && s.size() != 1)\n"
+              "        return;\n"
+              "    if(!s.empty()) {}\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void alwaysTrueLoop()

test/testvalueflow.cpp

@@ -6378,6 +6378,13 @@ private:
                "  else if (a.empty() == false && b.empty() == false) {}\n"
                "}\n";
         ASSERT("" != isImpossibleContainerSizeValue(tokenValues(code, "a . empty ( ) == false"), 0));
+
+        code = "bool g(std::vector<int>& v) {\n"
+               "    v.push_back(1);\n"
+               "    int x = v.empty();\n"
+               "    return x;\n"
+               "}\n";
+        ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 0));
     }
 
     void valueFlowContainerElement()