Make redundantAssignment message inconclusive when printed on global variables to avoid false warning on semaphores/mutexes (#4467)

lib/checkother.cpp

@@ -661,11 +661,16 @@ void CheckOther::sizeofForPointerError(const Token *tok, const std::string &varn
 // Detect redundant assignments: x = 0; x = 4;
 //---------------------------------------------------------------------------
 
+static bool nonLocal(const Variable* var)
+{
+    return (!var->isLocal() && !var->isArgument()) || var->isStatic() || var->isReference();
+}
+
 static void eraseNotLocalArg(std::map<unsigned int, const Token*>& container, const SymbolDatabase* symbolDatabase)
 {
     for (std::map<unsigned int, const Token*>::iterator i = container.begin(); i != container.end();) {
         const Variable* var = symbolDatabase->getVariableFromVarId(i->first);
-        if (!var || (!var->isLocal() && !var->isArgument()) || var->isStatic() || var->isReference()) {
+        if (!var || nonLocal(var)) {
             container.erase(i++);
             if (i == container.end())
                 break;
@@ -717,7 +722,7 @@ void CheckOther::checkRedundantAssignment()
                             if (scope->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok))
                                 redundantAssignmentInSwitchError(it->second, tok, tok->str());
                             else
-                                redundantAssignmentError(it->second, tok, tok->str());
+                                redundantAssignmentError(it->second, tok, tok->str(), nonLocal(it->second->variable())); // Inconclusive for non-local variables
                         }
                         it->second = tok;
                     }
@@ -794,11 +799,16 @@ void CheckOther::redundantCopyInSwitchError(const Token *tok1, const Token* tok2
                 "Buffer '" + var + "' is being written before its old content has been used. 'break;' missing?");
 }
 
-void CheckOther::redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var)
+void CheckOther::redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var, bool inconclusive)
 {
     std::list<const Token*> callstack;
     callstack.push_back(tok1);
     callstack.push_back(tok2);
+    if (inconclusive)
+        reportError(callstack, Severity::performance, "redundantAssignment",
+                    "Variable '" + var + "' is reassigned a value before the old one has been used if variable is no semaphore variable.\n"
+                    "Variable '" + var + "' is reassigned a value before the old one has been used. Make sure that this variable is not used like a semaphore in a threading environment before simplifying this code.", true);
+    else
         reportError(callstack, Severity::performance, "redundantAssignment",
                     "Variable '" + var + "' is reassigned a value before the old one has been used.");
 }

lib/checkother.h

@@ -325,7 +325,7 @@ private:
     void zerodivError(const Token *tok);
     void mathfunctionCallError(const Token *tok, const unsigned int numParam = 1);
     void cctypefunctionCallError(const Token *tok, const std::string &functionName, const std::string &value);
-    void redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var);
+    void redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var, bool inconclusive);
     void redundantAssignmentInSwitchError(const Token *tok1, const Token *tok2, const std::string &var);
     void redundantCopyError(const Token *tok1, const Token* tok2, const std::string& var);
     void redundantCopyInSwitchError(const Token *tok1, const Token* tok2, const std::string &var);
@@ -396,7 +396,7 @@ private:
         //performance
         c.redundantCopyError(0, "varname");
         c.redundantCopyError(0, 0, "var");
-        c.redundantAssignmentError(0, 0, "var");
+        c.redundantAssignmentError(0, 0, "var", false);
 
         // style/warning
         c.oppositeInnerConditionError(0);

test/testother.cpp

@@ -6796,7 +6796,7 @@ private:
               "    i = 1;\n"
               "    i = 1;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance, inconclusive) Variable 'i' is reassigned a value before the old one has been used if variable is no semaphore variable.\n", errout.str());
 
         check("void f() {\n"
               "    int i;\n"
@@ -6810,7 +6810,7 @@ private:
               "    i = 1;\n"
               "    i = 1;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance, inconclusive) Variable 'i' is reassigned a value before the old one has been used if variable is no semaphore variable.\n", errout.str());
 
         // Testing different types
         check("void f() {\n"
@@ -6824,7 +6824,7 @@ private:
               "    bar = x;\n"
               "    bar = y();\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance) Variable 'bar' is reassigned a value before the old one has been used.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance, inconclusive) Variable 'bar' is reassigned a value before the old one has been used if variable is no semaphore variable.\n", errout.str());
 
         check("void f() {\n"
               "    Foo& bar = foo();\n" // #4425. bar might refer to something global, etc.