From ccd95d17490f418eb9cccc529c224a8b8ee11b2a Mon Sep 17 00:00:00 2001 From: PKEuS Date: Fri, 15 Feb 2013 09:01:10 -0800 Subject: [PATCH] Make redundantAssignment message inconclusive when printed on global variables to avoid false warning on semaphores/mutexes (#4467) --- lib/checkother.cpp | 20 +++++++++++++++----- lib/checkother.h | 4 ++-- test/testother.cpp | 6 +++--- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 7585a9bec..7530e6908 100644 --- a/lib/checkother.cpp +++ b/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& container, const SymbolDatabase* symbolDatabase) { for (std::map::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,13 +799,18 @@ 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 callstack; callstack.push_back(tok1); callstack.push_back(tok2); - reportError(callstack, Severity::performance, "redundantAssignment", - "Variable '" + var + "' is reassigned a value before the old one has been used."); + 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."); } void CheckOther::redundantAssignmentInSwitchError(const Token *tok1, const Token* tok2, const std::string &var) diff --git a/lib/checkother.h b/lib/checkother.h index b67b55850..2a6b57ecd 100644 --- a/lib/checkother.h +++ b/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); diff --git a/test/testother.cpp b/test/testother.cpp index 659f45ebc..2a2091fb1 100644 --- a/test/testother.cpp +++ b/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.