Fix TODO

danmar · Aug 7, 2024 · 83d1ee1 · 83d1ee1
1 parent 1fb009d
commit 83d1ee1
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 13 deletions.
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
@@ -7175,27 +7175,28 @@ static void valueFlowContainerSize(const TokenList& tokenlist,
                 if (!next)
                     next = tok->next();
                 valueFlowForward(next, containerTok, std::move(value), tokenlist, errorLogger, settings);
-            } else if (tok->str() == "+" && Token::simpleMatch(tok->astParent(), "=")) { // TODO: handle multiple +
-                const Token* op1 = tok->astOperand1();
-                const Token* op2 = tok->astOperand2();
-
-                const bool op1Str = astIsContainerString(op1);
-                const bool op2Str = astIsContainerString(op2);
-                if (!op1Str && !op2Str)
+            } else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "+")) {
+                const Token* tok2 = tok->astOperand2();
+                bool haveString = false;
+                MathLib::bigint size = 0;
+                while (Token::simpleMatch(tok2, "+")) {
+                    size += valueFlowGetStrLength(tok2->astOperand2());
+                    haveString = haveString || astIsContainerString(tok2->astOperand2());
+                    tok2 = tok2->astOperand1();
+                }
+                size += valueFlowGetStrLength(tok2);
+                haveString = haveString || astIsContainerString(tok2);
+                if (size == 0 || !haveString)
                     continue;
 
-                const MathLib::bigint size1 = valueFlowGetStrLength(op1), size2 = valueFlowGetStrLength(op2);
-                const MathLib::bigint size = std::max(size1, size2);
-                if (size == 0)
-                    continue;
                 ValueFlow::Value value(size - 1);
                 value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
                 value.bound = ValueFlow::Value::Bound::Upper;
                 value.setImpossible();
                 Token* next = nextAfterAstRightmostLeaf(tok);
                 if (!next)
                     next = tok->next();
-                valueFlowForward(next, tok->astParent()->astOperand1(), std::move(value), tokenlist, errorLogger, settings);
+                valueFlowForward(next, tok->astOperand1(), std::move(value), tokenlist, errorLogger, settings);
             }
         }
     }

diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
@@ -6968,11 +6968,23 @@ class TestValueFlow : public TestFixture {
                "}";
         ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "c ."), 2));
 
+        code = "void f(const std::string& a) {\n"
+               "    std::string b = a + \"123\" + \"456\";\n"
+               "    if (b.empty()) {}\n"
+               "}";
+        ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "b ."), 5));
+
+        code = "void f(const std::string& a) {\n"
+               "    std::string b = \"123\" + a + \"456\";\n"
+               "    if (b.empty()) {}\n"
+               "}";
+        ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "b ."), 5));
+
         code = "void f(const std::string& a, const std::string& b) {\n"
                "    std::string c = \"123\" + a + b;\n"
                "    if (c.empty()) {}\n"
                "}";
-        TODO_ASSERT_EQUALS("", "values.size():0", isImpossibleContainerSizeValue(tokenValues(code, "c ."), 2));
+        ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "c ."), 2));
     }
 
     void valueFlowContainerElement()