New check: Iterating a known empty container (#2740)

lib/astutils.cpp

@@ -1349,6 +1349,13 @@ static const Variable* getArgumentVar(const Token* tok, int argnr)
     return nullptr;
 }
 
+static bool isCPPCastKeyword(const Token* tok)
+{
+    if (!tok)
+        return false;
+    return endsWith(tok->str(), "_cast", 5);
+}
+
 bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive)
 {
     if (!tok)
@@ -1366,6 +1373,8 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
     tok = getTokenArgumentFunction(tok, argnr);
     if (!tok)
         return false; // not a function => variable not changed
+    if (tok->isKeyword() && !isCPPCastKeyword(tok))
+        return false;
     const Token * parenTok = tok->next();
     if (Token::simpleMatch(parenTok, "<") && parenTok->link())
         parenTok = parenTok->link()->next();
@@ -1734,7 +1743,7 @@ bool isLikelyStreamRead(bool cpp, const Token *op)
 
 bool isCPPCast(const Token* tok)
 {
-    return tok && Token::simpleMatch(tok->previous(), "> (") && tok->astOperand2() && tok->astOperand1() && tok->astOperand1()->str().find("_cast") != std::string::npos;
+    return tok && Token::simpleMatch(tok->previous(), "> (") && tok->astOperand2() && tok->astOperand1() && isCPPCastKeyword(tok->astOperand1());
 }
 
 bool isConstVarExpression(const Token *tok, const char* skipMatch)

lib/checkstl.cpp

@@ -2461,6 +2461,48 @@ void CheckStl::useStlAlgorithm()
     }
 }
 
+void CheckStl::knownEmptyContainerLoopError(const Token *tok)
+{
+    const std::string cont = tok ? tok->expressionString() : std::string("var");
+
+    reportError(tok, Severity::style,
+                "knownEmptyContainerLoop",
+                "Iterating over container '" + cont + "' that is always empty.", CWE398, false);
+}
+
+void CheckStl::knownEmptyContainerLoop()
+{
+    if (!mSettings->isEnabled(Settings::STYLE))
+        return;
+    for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
+        for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
+            // Parse range-based for loop
+            if (!Token::simpleMatch(tok, "for ("))
+                continue;
+            if (!Token::simpleMatch(tok->next()->link(), ") {"))
+                continue;
+            const Token *bodyTok = tok->next()->link()->next();
+            const Token *splitTok = tok->next()->astOperand2();
+            if (!Token::simpleMatch(splitTok, ":"))
+                continue;
+            const Token* contTok = splitTok->astOperand2();
+            if (!contTok)
+                continue;
+            for(const ValueFlow::Value& v:contTok->values()) {
+                if (!v.isKnown())
+                    continue;
+                if (!v.isContainerSizeValue())
+                    continue;
+                if (v.intvalue != 0)
+                    continue;
+                knownEmptyContainerLoopError(contTok);
+            }
+
+
+        }
+    }
+}
+
 static bool isMutex(const Variable* var)
 {
     const Token* tok = Token::typeDecl(var->nameToken()).first;

lib/checkstl.h

@@ -81,6 +81,7 @@ public:
         checkStl.invalidContainerLoop();
         checkStl.mismatchingContainers();
         checkStl.mismatchingContainerIterator();
+        checkStl.knownEmptyContainerLoop();
 
         checkStl.stlBoundaries();
         checkStl.checkDereferenceInvalidIterator();
@@ -189,6 +190,8 @@ public:
     /** @brief Look for loops that can replaced with std algorithms */
     void useStlAlgorithm();
     
+    void knownEmptyContainerLoop();
+
     void checkMutexes();
 
 private:
@@ -235,6 +238,8 @@ private:
 
     void useStlAlgorithmError(const Token *tok, const std::string &algoName);
 
+    void knownEmptyContainerLoopError(const Token *tok);
+
     void globalLockGuardError(const Token *tok);
     void localMutexError(const Token *tok);
 
@@ -274,6 +279,7 @@ private:
         c.dereferenceInvalidIteratorError(nullptr, "i");
         c.readingEmptyStlContainerError(nullptr);
         c.useStlAlgorithmError(nullptr, "");
+        c.knownEmptyContainerLoopError(nullptr);
         c.globalLockGuardError(nullptr);
         c.localMutexError(nullptr);
     }
@@ -298,6 +304,7 @@ private:
                "- useless calls of string and STL functions\n"
                "- dereferencing an invalid iterator\n"
                "- reading from empty STL container\n"
+               "- iterating over an empty STL container\n"
                "- consider using an STL algorithm instead of raw loop\n"
                "- incorrect locking with mutex\n";
     }

lib/forwardanalyzer.cpp

@@ -330,9 +330,19 @@ struct ForwardTraversal {
                 if (initTok && updateRecursive(initTok) == Progress::Break)
                     return Progress::Break;
                 if (Token::Match(tok, "for|while (")) {
+                    // For-range loop
+                    if (Token::simpleMatch(condTok, ":")) {
+                        Token* conTok = condTok->astOperand2();
+                        if (conTok && updateRecursive(conTok) == Progress::Break)
+                            return Progress::Break;
+                        if (updateLoop(endBlock, condTok) == Progress::Break)
+                            return Progress::Break;
+                    } else {
                         Token* stepTok = getStepTok(tok);
                         if (updateLoop(endBlock, condTok, initTok, stepTok) == Progress::Break)
                             return Progress::Break;
+
+                    }
                     tok = endBlock;
                 } else {
                     // Traverse condition

lib/valueflow.cpp

@@ -1961,7 +1961,7 @@ static bool isConditionKnown(const Token* tok, bool then)
     if (then)
         op = "&&";
     const Token* parent = tok->astParent();
-    while (parent && parent->str() == op)
+    while (parent && (parent->str() == op || parent->str() == "!"))
         parent = parent->astParent();
     return (parent && parent->str() == "(");
 }
@@ -5977,7 +5977,7 @@ static void valueFlowContainerAfterCondition(TokenList *tokenlist,
                 return cond;
             const Token *parent = tok->astParent();
             while (parent) {
-                if (Token::Match(parent, "%comp%|!"))
+                if (Token::Match(parent, "%comp%"))
                     return cond;
                 parent = parent->astParent();
             }

test/teststl.cpp

@@ -166,6 +166,7 @@ private:
         TEST_CASE(invalidContainerLoop);
         TEST_CASE(findInsert);
 
+        TEST_CASE(checkKnownEmptyContainerLoop);
         TEST_CASE(checkMutexes);
     }
 
@@ -4524,6 +4525,36 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void checkKnownEmptyContainerLoop() {
+        check("void f() {\n"
+              "    std::vector<int> v;\n"
+              "    for(auto x:v) {}\n"
+              "}\n",
+              true);
+        ASSERT_EQUALS("[test.cpp:3]: (style) Iterating over container 'v' that is always empty.\n", errout.str());
+
+        check("void f(std::vector<int> v) {\n"
+              "    v.clear();\n"
+              "    for(auto x:v) {}\n"
+              "}\n",
+              true);
+        ASSERT_EQUALS("[test.cpp:3]: (style) Iterating over container 'v' that is always empty.\n", errout.str());
+
+        check("void f(std::vector<int> v) {\n"
+              "    if (!v.empty()) { return; }\n"
+              "    for(auto x:v) {}\n"
+              "}\n",
+              true);
+        ASSERT_EQUALS("[test.cpp:3]: (style) Iterating over container 'v' that is always empty.\n", errout.str());
+
+        check("void f(std::vector<int> v) {\n"
+              "    if (v.empty()) { return; }\n"
+              "    for(auto x:v) {}\n"
+              "}\n",
+              true);
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void checkMutexes() {
         check("void f() {\n"
               "    static std::mutex m;\n"

test/testvalueflow.cpp

@@ -4068,7 +4068,7 @@ private:
                "  if (!static_cast<bool>(ints.empty()))\n"
                "    ints.front();\n"
                "}";
-        ASSERT(tokenValues(code, "ints . front").empty());
+        ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "ints . front"), 0));
 
         // valueFlowContainerReverse
         code = "void f(const std::list<int> &ints) {\n"