diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a40ca132a..8bed01d2a 100644 --- 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 index 5b02b318b..1431c726a 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -5041,7 +5041,7 @@ private: " 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,7 +5067,7 @@ private: 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" @@ -5075,15 +5075,14 @@ private: "}", "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()); }