From 3af0d73f820b4a9e8ef73f70646355ce7423d896 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Sun, 16 Dec 2018 11:18:37 +0100
Subject: [PATCH] Unused value: Fixed false negatives for loops

---
 lib/astutils.cpp       | 19 ++++++++-----------
 test/testunusedvar.cpp | 38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/lib/astutils.cpp b/lib/astutils.cpp
index 0454792bb..73db58b8d 100644
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@@ -1065,11 +1065,6 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
             return Result(Result::Type::BAILOUT);
         }
 
-        if (tok->str() == "}" && (tok->scope()->type == Scope::eFor || tok->scope()->type == Scope::eWhile)) {
-            // TODO: handle loops better
-            return Result(Result::Type::BAILOUT);
-        }
-
         if (Token::simpleMatch(tok, "break ;")) {
             return Result(Result::Type::BREAK, tok);
         }
@@ -1097,17 +1092,19 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
 
         if (tok->str() == "}") {
             Scope::ScopeType scopeType = tok->scope()->type;
-            if (scopeType == Scope::eWhile || scopeType == Scope::eFor || scopeType == Scope::eDo)
-                // TODO handle loops better
-                return Result(Result::Type::BAILOUT);
+            if (scopeType == Scope::eWhile || scopeType == Scope::eFor || scopeType == Scope::eDo) {
+                // TODO: check condition
+                const struct FwdAnalysis::Result &result = checkRecursive(expr, tok->link(), tok, exprVarIds, local);
+                if (result.type == Result::Type::BAILOUT || result.type == Result::Type::READ)
+                    return result;
+            }
         }
 
         if (Token::simpleMatch(tok, "else {"))
             tok = tok->linkAt(1);
 
-        if (Token::simpleMatch(tok, "asm (")) {
+        if (Token::simpleMatch(tok, "asm ("))
             return Result(Result::Type::BAILOUT);
-        }
 
         if (!local && Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) {
             // TODO: this is a quick bailout
@@ -1255,7 +1252,7 @@ FwdAnalysis::Result FwdAnalysis::check(const Token *expr, const Token *startToke
         const Scope *s = result.token->scope();
         while (s->type == Scope::eIf)
             s = s->nestedIn;
-        if (s->type != Scope::eSwitch)
+        if (s->type != Scope::eSwitch && s->type != Scope::eWhile && s->type != Scope::eFor)
             break;
         result = checkRecursive(expr, s->bodyEnd->next(), endToken, exprVarIds, local);
     }
diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp
index 691843dfb..4c099e5aa 100644
--- a/test/testunusedvar.cpp
+++ b/test/testunusedvar.cpp
@@ -108,6 +108,7 @@ private:
         TEST_CASE(localvar52);
         TEST_CASE(localvar53); // continue
         TEST_CASE(localvar54); // ast, {}
+        TEST_CASE(localvarloops); // loops
         TEST_CASE(localvaralias1);
         TEST_CASE(localvaralias2); // ticket #1637
         TEST_CASE(localvaralias3); // ticket #1639
@@ -746,7 +747,9 @@ private:
                               "        d = code;\n"
                               "    }\n"
                               "}");
-        ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'd' is assigned a value that is never used.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'd' is assigned a value that is never used.\n"
+                      "[test.cpp:7]: (style) Variable 'd' is assigned a value that is never used.\n",
+                      errout.str());
 
         functionVariableUsage("void foo()\n"
                               "{\n"
@@ -2119,6 +2122,39 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void localvarloops() {
+        // loops
+        functionVariableUsage("void fun() {\n"
+                              "  int x;\n"
+                              "  while (c) { x=10; }\n"
+                              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'x' is assigned a value that is never used.\n", errout.str());
+
+        functionVariableUsage("void fun() {\n"
+                              "  int x = 1;\n"
+                              "  while (c) {\n"
+                              "    dostuff(x);\n"
+                              "    if (y) { x=10; break; }\n"
+                              "  }\n"
+                              "}\n");
+        ASSERT_EQUALS("[test.cpp:5]: (style) Variable 'x' is assigned a value that is never used.\n", errout.str());
+
+        functionVariableUsage("void fun() {\n"
+                              "  int x = 0;\n"
+                              "  while (c) {\n"
+                              "    dostuff(x);\n"
+                              "    x = 10;\n"
+                              "  }\n"
+                              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        functionVariableUsage("void fun() {\n"
+                              "  int x = 0;\n"
+                              "  while (x < 10) { x = x + 1; }\n"
+                              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void localvaralias1() {
         functionVariableUsage("void foo()\n"
                               "{\n"