From d55874ec37ae0ffa1b7270053f6049b3a57fe92a Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Sun, 25 Oct 2020 08:11:45 +0200 Subject: [PATCH] checkunusedvar: handle initialization list (#2836) --- lib/checkunusedvar.cpp | 76 +++++++++++++--- lib/checkunusedvar.h | 2 + test/testunusedvar.cpp | 198 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 259 insertions(+), 17 deletions(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index 7e443fef4..8b8ed725b 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1135,6 +1135,12 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const } } } + else if (tok->variable() && tok->variable()->isClass() && tok->variable()->type() && + (tok->variable()->type()->needInitialization == Type::NeedInitialization::False) && + tok->next()->str() == ";") + { + variables.write(tok->varId(), tok); + } } } @@ -1471,7 +1477,6 @@ bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type) { // a type that has no side effects (no constructors and no members with constructors) /** @todo false negative: check constructors for side effects */ - const std::pair::iterator,bool> found=mIsRecordTypeWithoutSideEffectsMap.insert( std::pair(type,false)); //Initialize with side effects for possible recursions bool & withoutSideEffects = found.first->second; @@ -1489,8 +1494,40 @@ bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type) if (f.argDef && Token::simpleMatch(f.argDef->link(), ") =")) continue; // ignore default/deleted constructors const bool emptyBody = (f.functionScope && Token::simpleMatch(f.functionScope->bodyStart, "{ }")); - const bool hasInitList = f.argDef && Token::simpleMatch(f.argDef->link(), ") :"); - if (!emptyBody || hasInitList) + + Token* nextToken = f.argDef->link(); + if (Token::simpleMatch(nextToken, ") :")) { + // validating initialization list + nextToken = nextToken->next(); // goto ":" + + for (const Token *initListToken = nextToken; Token::Match(initListToken, "[:,] %var% [({]"); initListToken = initListToken->linkAt(2)->next()) + { + const Token* varToken = initListToken->next(); + const Variable* variable = varToken->variable(); + if (variable && !isVariableWithoutSideEffects(*variable)) + { + return withoutSideEffects = false; + } + + const Token* valueEnd = initListToken->linkAt(2); + for (const Token* valueToken = initListToken->tokAt(3); valueToken != valueEnd; valueToken = valueToken->next()) { + const Variable* initValueVar = valueToken->variable(); + if (initValueVar && !isVariableWithoutSideEffects(*initValueVar)) + { + return withoutSideEffects = false; + } + if ((valueToken->tokType() == Token::Type::eFunction) || + (valueToken->tokType() == Token::Type::eName) || + (valueToken->tokType() == Token::Type::eLambda) || + (valueToken->tokType() == Token::Type::eOther)) + { + return withoutSideEffects = false; + } + } + } + } + + if (!emptyBody) return (withoutSideEffects = false); } @@ -1502,17 +1539,10 @@ bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type) // Is there a member variable with possible side effects for (const Variable& var : type->classScope->varlist) { - if (var.isPointer()) - continue; - - const Type* variableType = var.type(); - if (variableType) { - if (!isRecordTypeWithoutSideEffects(variableType)) - return (withoutSideEffects = false); - } else { - ValueType::Type valueType = var.valueType()->type; - if ((valueType == ValueType::Type::UNKNOWN_TYPE) || (valueType == ValueType::Type::NONSTD)) - return (withoutSideEffects = false); + withoutSideEffects = isVariableWithoutSideEffects(var); + if (!withoutSideEffects) + { + return withoutSideEffects; } } @@ -1520,6 +1550,24 @@ bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type) return (withoutSideEffects = true); } +bool CheckUnusedVar::isVariableWithoutSideEffects(const Variable& var) +{ + if (var.isPointer()) + return true; + + const Type* variableType = var.type(); + if (variableType) { + if (!isRecordTypeWithoutSideEffects(variableType)) + return false; + } else { + ValueType::Type valueType = var.valueType()->type; + if ((valueType == ValueType::Type::UNKNOWN_TYPE) || (valueType == ValueType::Type::NONSTD)) + return false; + } + + return true; +} + bool CheckUnusedVar::isEmptyType(const Type* type) { // a type that has no variables and no constructor diff --git a/lib/checkunusedvar.h b/lib/checkunusedvar.h index d31519a32..16cf93894 100644 --- a/lib/checkunusedvar.h +++ b/lib/checkunusedvar.h @@ -34,6 +34,7 @@ class Token; class Tokenizer; class Type; class Variables; +class Variable; /// @addtogroup Checks /// @{ @@ -70,6 +71,7 @@ public: private: bool isRecordTypeWithoutSideEffects(const Type* type); + bool isVariableWithoutSideEffects(const Variable& var); bool isEmptyType(const Type* type); // Error messages.. diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 19ec9cf5a..96d393109 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -286,12 +286,94 @@ private: "}"); ASSERT_EQUALS("[test.cpp:6]: (style) Unused variable: e\n", errout.str()); + functionVariableUsage( + "class F {\n" + "public:\n" + " F() : x(0) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (style) Unused variable: f\n", errout.str()); + + functionVariableUsage( + "class F {\n" + "public:\n" + " F() : x{0} {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (style) Unused variable: f\n", errout.str()); + + functionVariableUsage( + "int y = 0;\n" + "class F {\n" + "public:\n" + " F() : x(y) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("[test.cpp:8]: (style) Unused variable: f\n", errout.str()); + + functionVariableUsage( + "int y = 0;" + "class F {\n" + "public:\n" + " F() : x(++y) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (style) Unused variable: f\n", errout.str()); + + functionVariableUsage( + "int y = 0;" + "class F {\n" + "public:\n" + " F() : x(--y) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (style) Unused variable: f\n", errout.str()); + + functionVariableUsage( + "int y = 0;" + "class F {\n" + "public:\n" + " F() : x(y+=1) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (style) Unused variable: f\n", errout.str()); + + functionVariableUsage( + "int y = 0;" + "class F {\n" + "public:\n" + " F() : x(y-=1) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (style) Unused variable: f\n", errout.str()); + // non-empty constructor functionVariableUsage( "class F {\n" "public:\n" " F() {\n" - " int i =0;\n" + " int i = 0;\n" " (void) i;" " }\n" "};\n" @@ -305,7 +387,7 @@ private: "class F {\n" "public:\n" " F() {\n" - " int i =0;\n" + " int i = 0;\n" " (void) i;" " }\n" "};\n" @@ -318,6 +400,25 @@ private: "}"); ASSERT_EQUALS("", errout.str()); + // side-effect variable in initialization list + functionVariableUsage( + "class F {\n" + "public:\n" + " F() {\n" + " int i = 0;\n" + " (void) i;" + " }\n" + "};\n" + "class G {\n" + "public:\n" + " G() : f(F()) {}\n" + " F f;" + "};\n" + "void f() {\n" + " G g;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + // unknown variable type functionVariableUsage( "class H {\n" @@ -329,7 +430,98 @@ private: "}"); ASSERT_EQUALS("", errout.str()); + // unknown variable type in initialization list + functionVariableUsage( + "class H {\n" + "public:\n" + " H() : x{0}, u(1) {}\n" + " int x;" + " unknown_type u;\n" + "};\n" + "void f() {\n" + " H h;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + // unknown variable type used for initialization + functionVariableUsage( + "unknown_type y = 0;\n" + "class F {\n" + "public:\n" + " F() : x(y) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage( + "int sideEffectFunc();\n" + "class F {\n" + "public:\n" + " F() : x(sideEffectFunc()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage( + "class F {\n" + "public:\n" + " F() : x(unknownFunc()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage( + "class F {\n" + "public:\n" + " F() : x(++unknownValue) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage( + "class F {\n" + "public:\n" + " F() : x(--unknownValue) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage( + "class F {\n" + "public:\n" + " F() : x(unknownValue+=1) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage( + "class F {\n" + "public:\n" + " F() : x(unknownValue-=1) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " F f;\n" + "}"); + ASSERT_EQUALS("", errout.str()); } // #5355 - False positive: Variable is not assigned a value. @@ -4033,7 +4225,7 @@ private: " std::cout << \"test\" << std::endl;\n" " delete fred;\n" "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) Variable 'fred' is allocated memory that is never used.\n", errout.str()); functionVariableUsage("void foo()\n" "{\n"