Fixed danmar#7134 (False positive redundantAssignment - assignment to local variable of unknown type)

danmar · danmar · commit 939b42df06d9 · 2016-08-29T03:48:08.000+02:00
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -467,8 +467,9 @@ static bool checkExceptionHandling(const Token* tok)
 void CheckOther::checkRedundantAssignment()
 {
     const bool printPerformance = _settings->isEnabled("performance");
+    const bool printStyle = _settings->isEnabled("style");
     const bool printWarning = _settings->isEnabled("warning");
-    if (!printWarning && !printPerformance)
+    if (!printWarning && !printPerformance && !printStyle)
         return;
 
     const bool printInconclusive = _settings->inconclusive;
@@ -597,15 +598,24 @@ void CheckOther::checkRedundantAssignment()
                                 }
                             }
                         }
+
                         if (error) {
                             if (printWarning && scope->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok))
                                 redundantAssignmentInSwitchError(it->second, tok, eq->astOperand1()->expressionString());
-                            else if (printPerformance) {
-                                // See #7133
+                            else if (printStyle) {
+                                // c++, unknown type => assignment might have additional side effects
+                                const bool possibleSideEffects(_tokenizer->isCPP() && !tok->valueType());
+
+                                // TODO nonlocal variables are not tracked entirely.
                                 const bool nonlocal = it->second->variable() && nonLocalVolatile(it->second->variable());
-                                if (printInconclusive || !nonlocal) // report inconclusive only when requested
+                                
+                                // Warnings are inconclusive if there are possible side effects or if variable is not
+                                // tracked perfectly.
+                                const bool inconclusive = possibleSideEffects | nonlocal;
+
+                                if (printInconclusive || !inconclusive)
                                     if (_tokenizer->isC() || checkExceptionHandling(tok)) // see #6555 to see how exception handling might have an impact
-                                        redundantAssignmentError(it->second, tok, eq->astOperand1()->expressionString(), nonlocal); // Inconclusive for non-local variables
+                                        redundantAssignmentError(it->second, tok, eq->astOperand1()->expressionString(), inconclusive);
                             }
                         }
                         it->second = tok;
diff --git a/test/testother.cpp b/test/testother.cpp
@@ -5041,7 +5041,7 @@ class TestOther : public TestFixture {
               "    if (memptr)\n"
               "        memptr = 0;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'memptr' is reassigned a value before the old one has been used.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style, inconclusive) Variable 'memptr' is reassigned a value before the old one has been used if variable is no semaphore variable.\n", errout.str());
     }
 
     void redundantVarAssignment_7133() {
@@ -5067,23 +5067,22 @@ class TestOther : public TestFixture {
         ASSERT_EQUALS("", errout.str());
 
         check("void ConvertBitmapData(sal_uInt16 nDestBits) {\n"
-              "BitmapBuffer aSrcBuf;\n"
+              "    BitmapBuffer aSrcBuf;\n"
               "    aSrcBuf.mnBitCount = nSrcBits;\n"
               "    BitmapBuffer aDstBuf;\n"
               "    aSrcBuf.mnBitCount = nDestBits;\n"
               "    bConverted = ::ImplFastBitmapConversion( aDstBuf, aSrcBuf, aTwoRects );\n"
               "}", "test.c");
         ASSERT_EQUALS("[test.c:3] -> [test.c:5]: (style) Variable 'aSrcBuf.mnBitCount' is reassigned a value before the old one has been used.\n", errout.str());
         check("void ConvertBitmapData(sal_uInt16 nDestBits) {\n"
-              "BitmapBuffer aSrcBuf;\n"
+              "    BitmapBuffer aSrcBuf;\n"
               "    aSrcBuf.mnBitCount = nSrcBits;\n"
               "    BitmapBuffer aDstBuf;\n"
               "    aSrcBuf.mnBitCount = nDestBits;\n"
               "    bConverted = ::ImplFastBitmapConversion( aDstBuf, aSrcBuf, aTwoRects );\n"
               "}");
-        TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style, inconclusive) Variable 'aSrcBuf.mnBitCount' is reassigned a value before the old one has been used.\n",
-                           "[test.cpp:3] -> [test.cpp:5]: (style) Variable 'aSrcBuf.mnBitCount' is reassigned a value before the old one has been used.\n",
-                           errout.str());
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style, inconclusive) Variable 'aSrcBuf.mnBitCount' is reassigned a value before the old one has been used if variable is no semaphore variable.\n",
+                      errout.str());
 
     }
 
