From d2e8ab9c8670789c4d40ac74019dcc60db016b8e Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Tue, 1 Jan 2013 09:53:40 +0100 Subject: [PATCH] Fixed #4302 (Member variable not initialized in public delegate constructor) --- lib/checkclass.cpp | 96 +++++++++++++++++++++++---------------- lib/checkclass.h | 2 +- lib/symboldatabase.cpp | 61 +++++++++++++++---------- lib/symboldatabase.h | 6 +++ test/testconstructors.cpp | 40 +++++++++++++++- 5 files changed, 140 insertions(+), 65 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 545e0b3d5..e22345271 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -105,7 +105,7 @@ void CheckClass::constructors() // Mark all variables not used clearAllVar(usage); - std::list callstack; + std::list callstack; initializeVarList(*func, callstack, &(*scope), usage); // Check if any variables are uninitialized @@ -354,7 +354,7 @@ bool CheckClass::isBaseClassFunc(const Token *tok, const Scope *scope) return false; } -void CheckClass::initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage) +void CheckClass::initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage) { bool initList = func.type == Function::eConstructor || func.type == Function::eCopyConstructor; const Token *ftok = func.arg->link()->next(); @@ -364,8 +364,35 @@ void CheckClass::initializeVarList(const Function &func, std::list // clKalle::clKalle() : var(value) { } if (initList) { if (level == 0 && Token::Match(ftok, "%var% (")) { - if (ftok->strAt(2) != ")") - initVar(ftok->str(), scope, usage); + if (ftok->str() != func.name()) { + if (ftok->strAt(2) != ")") + initVar(ftok->str(), scope, usage); + } else { // c++11 delegate constructor + const Function *member = symbolDatabase->findFunctionByNameAndArgsInScope(ftok, scope); + // member function found + if (member) { + // recursive call + // assume that all variables are initialized + if (std::find(callstack.begin(), callstack.end(), member) != callstack.end()) { + /** @todo false negative: just bail */ + assignAllVar(usage); + return; + } + + // member function has implementation + if (member->hasBody) { + // initialize variable use list using member function + callstack.push_back(member); + initializeVarList(*member, callstack, scope, usage); + callstack.pop_back(); + } + + // there is a called member function, but it has no implementation, so we assume it initializes everything + else { + assignAllVar(usage); + } + } + } } else if (level == 0 && Token::Match(ftok, "%var% {") && ftok->str() != "const" && Token::Match(ftok->next()->link()->next(), ",|{|%type%")) { initVar(ftok->str(), scope, usage); ftok = ftok->linkAt(1); @@ -469,29 +496,22 @@ void CheckClass::initializeVarList(const Function &func, std::list // Calling member function? else if (Token::simpleMatch(ftok, "operator= (") && ftok->previous()->str() != "::") { - // recursive call / calling overloaded function - // assume that all variables are initialized - if (std::find(callstack.begin(), callstack.end(), ftok->str()) != callstack.end()) { - /** @todo false negative: just bail */ - assignAllVar(usage); - return; - } - - /** @todo check function parameters for overloaded function so we check the right one */ - // check if member function exists - std::list::const_iterator it; - for (it = scope->functionList.begin(); it != scope->functionList.end(); ++it) { - if (ftok->str() == it->tokenDef->str() && it->type != Function::eConstructor) - break; - } - + const Function *member = symbolDatabase->findFunctionByNameAndArgsInScope(ftok, scope); // member function found - if (it != scope->functionList.end()) { + if (member) { + // recursive call + // assume that all variables are initialized + if (std::find(callstack.begin(), callstack.end(), member) != callstack.end()) { + /** @todo false negative: just bail */ + assignAllVar(usage); + return; + } + // member function has implementation - if (it->hasBody) { + if (member->hasBody) { // initialize variable use list using member function - callstack.push_back(ftok->str()); - initializeVarList(*it, callstack, scope, usage); + callstack.push_back(member); + initializeVarList(*member, callstack, scope, usage); callstack.pop_back(); } @@ -517,27 +537,23 @@ void CheckClass::initializeVarList(const Function &func, std::list } } - // recursive call / calling overloaded function - // assume that all variables are initialized - if (std::find(callstack.begin(), callstack.end(), ftok->str()) != callstack.end()) { - assignAllVar(usage); - return; - } - // check if member function - std::list::const_iterator it; - for (it = scope->functionList.begin(); it != scope->functionList.end(); ++it) { - if (ftok->str() == it->tokenDef->str() && it->type != Function::eConstructor) - break; - } + const Function *member = symbolDatabase->findFunctionByNameAndArgsInScope(ftok, scope); // member function found - if (it != scope->functionList.end()) { + if (member && member->type != Function::eConstructor) { + // recursive call + // assume that all variables are initialized + if (std::find(callstack.begin(), callstack.end(), member) != callstack.end()) { + assignAllVar(usage); + return; + } + // member function has implementation - if (it->hasBody) { + if (member->hasBody) { // initialize variable use list using member function - callstack.push_back(ftok->str()); - initializeVarList(*it, callstack, scope, usage); + callstack.push_back(member); + initializeVarList(*member, callstack, scope, usage); callstack.pop_back(); // Assume that variables that are passed to it are initialized.. diff --git a/lib/checkclass.h b/lib/checkclass.h index 26d4a4c8f..97a9db9da 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -244,7 +244,7 @@ private: * @param scope pointer to variable Scope * @param usage reference to usage vector */ - void initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage); + void initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage); static bool canNotCopy(const Scope *scope); }; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 6f04f2081..9d27ce9f3 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -2299,6 +2299,8 @@ const Function *SymbolDatabase::findFunctionByToken(const Token *tok) const return 0; } +//--------------------------------------------------------------------------- + const Function* SymbolDatabase::findFunctionByName(const std::string& str, const Scope* startScope) const { const Scope* currScope = startScope; @@ -2318,10 +2320,43 @@ const Function* SymbolDatabase::findFunctionByName(const std::string& str, const return 0; } +//--------------------------------------------------------------------------- + /** @todo This function only counts the number of arguments in the function call. It does not take into account functions with default arguments. It does not take into account argument types. This can be difficult because of promotion and conversion operators and casts and because the argument can also be a function call. */ +const Function* SymbolDatabase::findFunctionByNameAndArgsInScope(const Token *tok, const Scope *scope) const +{ + for (std::list::const_iterator i = scope->functionList.begin(); i != scope->functionList.end(); ++i) { + if (i->tokenDef->str() == tok->str()) { + const Function *func = &*i; + if (tok->strAt(1) == "(" && tok->tokAt(2)) { + // check if function has no arguments + if (tok->strAt(2) == ")" && (func->argCount() == 0 || func->argCount() == func->initializedArgCount())) + return func; + + // check the arguments + unsigned int args = 0; + const Token *arg = tok->tokAt(2); + while (arg && arg->str() != ")") { + /** @todo check argument type for match */ + /** @todo check for default arguments */ + args++; + arg = arg->nextArgument(); + } + + if (args == func->argCount()) + return func; + } + } + } + + return 0; +} + +//--------------------------------------------------------------------------- + const Function* SymbolDatabase::findFunctionByNameAndArgs(const Token *tok, const Scope *startScope) const { const Scope* currScope = startScope; @@ -2332,29 +2367,9 @@ const Function* SymbolDatabase::findFunctionByNameAndArgs(const Token *tok, cons currScope = currScope->nestedIn; } while (currScope) { - for (std::list::const_iterator i = currScope->functionList.begin(); i != currScope->functionList.end(); ++i) { - if (i->tokenDef->str() == tok->str()) { - const Function *func = &*i; - if (tok->strAt(1) == "(" && tok->tokAt(2)) { - // check if function has no arguments - if (tok->strAt(2) == ")" && (func->argCount() == 0 || func->argCount() == func->initializedArgCount())) - return func; - - // check the arguments - unsigned int args = 0; - const Token *arg = tok->tokAt(2); - while (arg && arg->str() != ")") { - /** @todo check argument type for match */ - /** @todo check for default arguments */ - args++; - arg = arg->nextArgument(); - } - - if (args == func->argCount()) - return func; - } - } - } + const Function *func = findFunctionByNameAndArgsInScope(tok, currScope); + if (func) + return func; currScope = currScope->nestedIn; } return 0; diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 6b635789e..caa23cd25 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -394,6 +394,10 @@ public: retFuncPtr(false) { } + const std::string &name() const { + return tokenDef->str(); + } + std::size_t argCount() const { return argumentList.size(); } @@ -605,6 +609,8 @@ public: */ const Function* findFunctionByNameAndArgs(const Token *tok, const Scope *startScope) const; + const Function* findFunctionByNameAndArgsInScope(const Token *tok, const Scope *scope) const; + const Scope* findScopeByName(const std::string& name) const; const Scope *findScope(const Token *tok, const Scope *startScope) const; diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index 5e4812e25..f85b58021 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -73,6 +73,7 @@ private: TEST_CASE(initvar_constvar); TEST_CASE(initvar_staticvar); TEST_CASE(initvar_union); + TEST_CASE(initvar_delegate); // ticket #4302 TEST_CASE(initvar_private_constructor); // BUG 2354171 - private constructor TEST_CASE(initvar_copy_constructor); // ticket #1611 @@ -688,6 +689,43 @@ private: } + void initvar_delegate() { + check("class A {\n" + " int number;\n" + "public:\n" + " A(int n) { }\n" + " A() : A(42) {}\n" + "};"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'A::number' is not initialized in the constructor.\n" + "[test.cpp:5]: (warning) Member variable 'A::number' is not initialized in the constructor.\n", errout.str()); + + check("class A {\n" + " int number;\n" + "public:\n" + " A(int n) { number = n; }\n" + " A() : A(42) {}\n" + "};"); + ASSERT_EQUALS("", errout.str()); + + check("class A {\n" + " int number;\n" + "public:\n" + " A(int n) : A() { }\n" + " A() {}\n" + "};"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'A::number' is not initialized in the constructor.\n" + "[test.cpp:5]: (warning) Member variable 'A::number' is not initialized in the constructor.\n", errout.str()); + + check("class A {\n" + " int number;\n" + "public:\n" + " A(int n) : A() { }\n" + " A() { number = 42; }\n" + "};"); + ASSERT_EQUALS("", errout.str()); + } + + void initvar_private_constructor() { check("class Fred\n" "{\n" @@ -2465,7 +2503,7 @@ private: " void init(int value)\n" " { }\n" "};"); - TODO_ASSERT_EQUALS("[test.cpp:7]: (warning) Member variable 'A::i' is not initialized in the constructor.\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (warning) Member variable 'A::i' is not initialized in the constructor.\n", errout.str()); } void uninitVarOperatorEqual() { // ticket #2415