From 66be74a5afcc0077823e5e52683788fa514d44c8 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Sun, 16 Jan 2011 16:37:11 +0100 Subject: [PATCH] Symbol database: Refactorings. Move check-specific code to check. Ticket: #2468 --- lib/checkclass.cpp | 371 ++++++++++++++++++++++++++++++++++++++++- lib/checkclass.h | 52 ++++++ lib/symboldatabase.cpp | 367 ---------------------------------------- lib/symboldatabase.h | 41 +---- 4 files changed, 420 insertions(+), 411 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 9b211cc34..bc9b6be7a 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -94,6 +94,7 @@ void CheckClass::constructors() } std::list::const_iterator func; + std::vector usage(info->varlist.size()); for (func = info->functionList.begin(); func != info->functionList.end(); ++func) { @@ -103,16 +104,17 @@ void CheckClass::constructors() continue; // Mark all variables not used - info->clearAllVar(); + clearAllVar(usage); std::list callstack; - info->initializeVarList(*func, callstack); + initializeVarList(*func, callstack, info, usage); // Check if any variables are uninitialized std::list::const_iterator var; - for (var = info->varlist.begin(); var != info->varlist.end(); ++var) + unsigned int count = 0; + for (var = info->varlist.begin(); var != info->varlist.end(); ++var, ++count) { - if (var->assign || var->init || var->isStatic) + if (usage[count].assign || usage[count].init || var->isStatic) continue; if (var->isConst && var->token->previous()->str() != "*") @@ -160,6 +162,367 @@ void CheckClass::constructors() } } +void CheckClass::assignVar(const std::string &varname, const SymbolDatabase::SpaceInfo *info, std::vector &usage) +{ + std::list::const_iterator var; + unsigned int count = 0; + + for (var = info->varlist.begin(); var != info->varlist.end(); ++var, ++count) + { + if (var->token->str() == varname) + { + usage[count].assign = true; + return; + } + } +} + +void CheckClass::initVar(const std::string &varname, const SymbolDatabase::SpaceInfo *info, std::vector &usage) +{ + std::list::const_iterator var; + unsigned int count = 0; + + for (var = info->varlist.begin(); var != info->varlist.end(); ++var, ++count) + { + if (var->token->str() == varname) + { + usage[count].init = true; + return; + } + } +} + +void CheckClass::assignAllVar(std::vector &usage) +{ + for (size_t i = 0; i < usage.size(); ++i) + usage[i].assign = true; +} + +void CheckClass::clearAllVar(std::vector &usage) +{ + for (size_t i = 0; i < usage.size(); ++i) + { + usage[i].assign = false; + usage[i].init = false; + } +} + +bool CheckClass::isBaseClassFunc(const Token *tok, const SymbolDatabase::SpaceInfo *info) +{ + // Iterate through each base class... + for (size_t i = 0; i < info->derivedFrom.size(); ++i) + { + const SymbolDatabase::SpaceInfo *derivedFrom = info->derivedFrom[i].spaceInfo; + + // Check if base class exists in database + if (derivedFrom) + { + std::list::const_iterator func; + + for (func = derivedFrom->functionList.begin(); func != derivedFrom->functionList.end(); ++func) + { + if (func->tokenDef->str() == tok->str()) + return true; + } + } + + // Base class not found so assume it is in it. + else + return true; + } + + return false; +} + +void CheckClass::initializeVarList(const SymbolDatabase::Func &func, std::list &callstack, const SymbolDatabase::SpaceInfo *info, std::vector &usage) +{ + bool Assign = false; + unsigned int indentlevel = 0; + const Token *ftok = func.token; + + for (; ftok; ftok = ftok->next()) + { + if (!ftok->next()) + break; + + // Class constructor.. initializing variables like this + // clKalle::clKalle() : var(value) { } + if (indentlevel == 0) + { + if (Assign && Token::Match(ftok, "%var% (")) + { + initVar(ftok->str(), info, usage); + + // assignment in the initializer.. + // : var(value = x) + if (Token::Match(ftok->tokAt(2), "%var% =")) + assignVar(ftok->strAt(2), info, usage); + } + + Assign |= (ftok->str() == ":"); + } + + + if (ftok->str() == "{") + { + ++indentlevel; + Assign = false; + } + + else if (ftok->str() == "}") + { + if (indentlevel <= 1) + break; + --indentlevel; + } + + if (indentlevel < 1) + continue; + + // Variable getting value from stream? + if (Token::Match(ftok, ">> %var%")) + { + assignVar(ftok->strAt(1), info, usage); + } + + // Before a new statement there is "[{};)=]" + if (! Token::Match(ftok, "[{};()=]")) + continue; + + if (Token::simpleMatch(ftok, "( !")) + ftok = ftok->next(); + + // Using the operator= function to initialize all variables.. + if (Token::simpleMatch(ftok->next(), "* this = ")) + { + assignAllVar(usage); + break; + } + + // Calling member variable function? + if (Token::Match(ftok->next(), "%var% . %var% (")) + { + std::list::const_iterator var; + for (var = info->varlist.begin(); var != info->varlist.end(); ++var) + { + if (var->token->varId() == ftok->next()->varId()) + { + /** @todo false negative: we assume function changes variable state */ + assignVar(ftok->next()->str(), info, usage); + continue; + } + } + + ftok = ftok->tokAt(2); + } + + if (!Token::Match(ftok->next(), "%var%") && + !Token::Match(ftok->next(), "this . %var%") && + !Token::Match(ftok->next(), "* %var% =") && + !Token::Match(ftok->next(), "( * this ) . %var%")) + continue; + + // Goto the first token in this statement.. + ftok = ftok->next(); + + // Skip "( * this )" + if (Token::simpleMatch(ftok, "( * this ) .")) + { + ftok = ftok->tokAt(5); + } + + // Skip "this->" + if (Token::simpleMatch(ftok, "this .")) + ftok = ftok->tokAt(2); + + // Skip "classname :: " + if (Token::Match(ftok, "%var% ::")) + ftok = ftok->tokAt(2); + + // Clearing all variables.. + if (Token::simpleMatch(ftok, "memset ( this ,")) + { + assignAllVar(usage); + return; + } + + // Clearing array.. + else if (Token::Match(ftok, "memset ( %var% ,")) + { + assignVar(ftok->strAt(2), info, usage); + ftok = ftok->next()->link(); + continue; + } + + // Calling member function? + else if (Token::simpleMatch(ftok, "operator = (") && + ftok->previous()->str() != "::") + { + // check if member function exists + std::list::const_iterator it; + for (it = info->functionList.begin(); it != info->functionList.end(); ++it) + { + if (ftok->next()->str() == it->tokenDef->str() && it->type != SymbolDatabase::Func::Constructor) + break; + } + + // member function found + if (it != info->functionList.end()) + { + // member function has implementation + if (it->hasBody) + { + // initialize variable use list using member function + callstack.push_back(ftok->str()); + initializeVarList(*it, callstack, info, usage); + callstack.pop_back(); + } + + // there is a called member function, but it has no implementation, so we assume it initializes everything + else + { + assignAllVar(usage); + } + } + + // using default operator =, assume everything initialized + else + { + assignAllVar(usage); + } + } + else if (Token::Match(ftok, "%var% (") && ftok->str() != "if") + { + // Passing "this" => assume that everything is initialized + for (const Token *tok2 = ftok->next()->link(); tok2 && tok2 != ftok; tok2 = tok2->previous()) + { + if (tok2->str() == "this") + { + assignAllVar(usage); + return; + } + } + + // 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 = info->functionList.begin(); it != info->functionList.end(); ++it) + { + if (ftok->str() == it->tokenDef->str() && it->type != SymbolDatabase::Func::Constructor) + break; + } + + // member function found + if (it != info->functionList.end()) + { + // member function has implementation + if (it->hasBody) + { + // initialize variable use list using member function + callstack.push_back(ftok->str()); + initializeVarList(*it, callstack, info, usage); + callstack.pop_back(); + } + + // there is a called member function, but it has no implementation, so we assume it initializes everything + else + { + assignAllVar(usage); + } + } + + // not member function + else + { + // could be a base class virtual function, so we assume it initializes everything + if (func.type != SymbolDatabase::Func::Constructor && isBaseClassFunc(ftok, info)) + { + /** @todo False Negative: we should look at the base class functions to see if they + * call any derived class virtual functions that change the derived class state + */ + assignAllVar(usage); + } + + // has friends, so we assume it initializes everything + if (!info->friendList.empty()) + assignAllVar(usage); + + // the function is external and it's neither friend nor inherited virtual function. + // assume all variables that are passed to it are initialized.. + else + { + unsigned int indentlevel2 = 0; + for (const Token *tok = ftok->tokAt(2); tok; tok = tok->next()) + { + if (tok->str() == "(") + ++indentlevel2; + else if (tok->str() == ")") + { + if (indentlevel2 == 0) + break; + --indentlevel2; + } + if (tok->isName()) + { + assignVar(tok->str(), info, usage); + } + } + } + } + } + + // Assignment of member variable? + else if (Token::Match(ftok, "%var% =")) + { + assignVar(ftok->str(), info, usage); + } + + // Assignment of array item of member variable? + else if (Token::Match(ftok, "%var% [ %any% ] =")) + { + assignVar(ftok->str(), info, usage); + } + + // Assignment of member of array item of member variable? + else if (Token::Match(ftok, "%var% [ %any% ] . %var% =") || + Token::Match(ftok, "%var% [ %any% ] . %var% . %var% =")) + { + assignVar(ftok->str(), info, usage); + } + + // Assignment of array item of member variable? + else if (Token::Match(ftok, "%var% [ %any% ] [ %any% ] =")) + { + assignVar(ftok->str(), info, usage); + } + + // Assignment of array item of member variable? + else if (Token::Match(ftok, "* %var% =")) + { + assignVar(ftok->next()->str(), info, usage); + } + + // Assignment of struct member of member variable? + else if (Token::Match(ftok, "%var% . %any% =")) + { + assignVar(ftok->str(), info, usage); + } + + // The functions 'clear' and 'Clear' are supposed to initialize variable. + if (Token::Match(ftok, "%var% . clear|Clear (")) + { + assignVar(ftok->str(), info, usage); + } + } +} + void CheckClass::noConstructorError(const Token *tok, const std::string &classname, bool isStruct) { // For performance reasons the constructor might be intentionally missing. Therefore this is not a "warning" diff --git a/lib/checkclass.h b/lib/checkclass.h index c87b4fdd9..23edb4dc0 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -175,6 +175,58 @@ private: bool checkConstFunc(const SymbolDatabase::SpaceInfo *info, const Token *tok); /** @brief check if this function is virtual in the base classes */ bool isVirtualFunc(const SymbolDatabase::SpaceInfo *info, const Token *functionToken) const; + + // constructors helper function + /** @brief Information about a member variable. Used when checking for uninitialized variables */ + struct Usage + { + Usage() : assign(false), init(false) { } + + /** @brief has this variable been assigned? */ + bool assign; + + /** @brief has this variable been initialized? */ + bool init; + }; + + bool isBaseClassFunc(const Token *tok, const SymbolDatabase::SpaceInfo *info); + + /** + * @brief assign a variable in the varlist + * @param varname name of variable to mark assigned + * @param info pointer to variable SpaceInfo + * @param usage reference to usage vector + */ + void assignVar(const std::string &varname, const SymbolDatabase::SpaceInfo *info, std::vector &usage); + + /** + * @brief initialize a variable in the varlist + * @param varname name of variable to mark initialized + * @param info pointer to variable SpaceInfo + * @param usage reference to usage vector + */ + void initVar(const std::string &varname, const SymbolDatabase::SpaceInfo *info, std::vector &usage); + + /** + * @brief set all variables in list assigned + * @param usage reference to usage vector + */ + void assignAllVar(std::vector &usage); + + /** + * @brief set all variables in list not assigned and not initialized + * @param usage reference to usage vector + */ + void clearAllVar(std::vector &usage); + + /** + * @brief parse a scope for a constructor or member function and set the "init" flags in the provided varlist + * @param func reference to the function that should be checked + * @param callstack the function doesn't look into recursive function calls. + * @param info pointer to variable SpaceInfo + * @param usage reference to usage vector + */ + void initializeVarList(const SymbolDatabase::Func &func, std::list &callstack, const SymbolDatabase::SpaceInfo *info, std::vector &usage); }; /// @} //--------------------------------------------------------------------------- diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 2319c945f..ea1401e7c 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1417,370 +1417,3 @@ unsigned int SymbolDatabase::SpaceInfo::getNestedNonFunctions() const } return nested; } - -//--------------------------------------------------------------------------- - -void SymbolDatabase::SpaceInfo::assignVar(const std::string &varname) -{ - std::list::iterator i; - - for (i = varlist.begin(); i != varlist.end(); ++i) - { - if (i->token->str() == varname) - { - i->assign = true; - return; - } - } -} - -void SymbolDatabase::SpaceInfo::initVar(const std::string &varname) -{ - std::list::iterator i; - - for (i = varlist.begin(); i != varlist.end(); ++i) - { - if (i->token->str() == varname) - { - i->init = true; - return; - } - } -} - -void SymbolDatabase::SpaceInfo::assignAllVar() -{ - std::list::iterator i; - - for (i = varlist.begin(); i != varlist.end(); ++i) - i->assign = true; -} - -void SymbolDatabase::SpaceInfo::clearAllVar() -{ - std::list::iterator i; - - for (i = varlist.begin(); i != varlist.end(); ++i) - { - i->assign = false; - i->init = false; - } -} - -//--------------------------------------------------------------------------- - -bool SymbolDatabase::SpaceInfo::isBaseClassFunc(const Token *tok) -{ - // Iterate through each base class... - for (unsigned int i = 0; i < derivedFrom.size(); ++i) - { - const SpaceInfo *info = derivedFrom[i].spaceInfo; - - // Check if base class exists in database - if (info) - { - std::list::const_iterator it; - - for (it = info->functionList.begin(); it != info->functionList.end(); ++it) - { - if (it->tokenDef->str() == tok->str()) - return true; - } - } - - // Base class not found so assume it is in it. - else - return true; - } - - return false; -} - -void SymbolDatabase::SpaceInfo::initializeVarList(const Func &func, std::list &callstack) -{ - bool Assign = false; - unsigned int indentlevel = 0; - const Token *ftok = func.token; - - for (; ftok; ftok = ftok->next()) - { - if (!ftok->next()) - break; - - // Class constructor.. initializing variables like this - // clKalle::clKalle() : var(value) { } - if (indentlevel == 0) - { - if (Assign && Token::Match(ftok, "%var% (")) - { - initVar(ftok->str()); - - // assignment in the initializer.. - // : var(value = x) - if (Token::Match(ftok->tokAt(2), "%var% =")) - assignVar(ftok->strAt(2)); - } - - Assign |= (ftok->str() == ":"); - } - - - if (ftok->str() == "{") - { - ++indentlevel; - Assign = false; - } - - else if (ftok->str() == "}") - { - if (indentlevel <= 1) - break; - --indentlevel; - } - - if (indentlevel < 1) - continue; - - // Variable getting value from stream? - if (Token::Match(ftok, ">> %var%")) - { - assignVar(ftok->strAt(1)); - } - - // Before a new statement there is "[{};)=]" - if (! Token::Match(ftok, "[{};()=]")) - continue; - - if (Token::simpleMatch(ftok, "( !")) - ftok = ftok->next(); - - // Using the operator= function to initialize all variables.. - if (Token::simpleMatch(ftok->next(), "* this = ")) - { - assignAllVar(); - break; - } - - // Calling member variable function? - if (Token::Match(ftok->next(), "%var% . %var% (")) - { - std::list::const_iterator var; - for (var = varlist.begin(); var != varlist.end(); ++var) - { - if (var->token->varId() == ftok->next()->varId()) - { - /** @todo false negative: we assume function changes variable state */ - assignVar(ftok->next()->str()); - continue; - } - } - - ftok = ftok->tokAt(2); - } - - if (!Token::Match(ftok->next(), "%var%") && - !Token::Match(ftok->next(), "this . %var%") && - !Token::Match(ftok->next(), "* %var% =") && - !Token::Match(ftok->next(), "( * this ) . %var%")) - continue; - - // Goto the first token in this statement.. - ftok = ftok->next(); - - // Skip "( * this )" - if (Token::simpleMatch(ftok, "( * this ) .")) - { - ftok = ftok->tokAt(5); - } - - // Skip "this->" - if (Token::simpleMatch(ftok, "this .")) - ftok = ftok->tokAt(2); - - // Skip "classname :: " - if (Token::Match(ftok, "%var% ::")) - ftok = ftok->tokAt(2); - - // Clearing all variables.. - if (Token::simpleMatch(ftok, "memset ( this ,")) - { - assignAllVar(); - return; - } - - // Clearing array.. - else if (Token::Match(ftok, "memset ( %var% ,")) - { - assignVar(ftok->strAt(2)); - ftok = ftok->next()->link(); - continue; - } - - // Calling member function? - else if (Token::simpleMatch(ftok, "operator = (") && - ftok->previous()->str() != "::") - { - // check if member function exists - std::list::const_iterator it; - for (it = functionList.begin(); it != functionList.end(); ++it) - { - if (ftok->next()->str() == it->tokenDef->str() && it->type != Func::Constructor) - break; - } - - // member function found - if (it != functionList.end()) - { - // member function has implementation - if (it->hasBody) - { - // initialize variable use list using member function - callstack.push_back(ftok->str()); - initializeVarList(*it, callstack); - callstack.pop_back(); - } - - // there is a called member function, but it has no implementation, so we assume it initializes everything - else - { - assignAllVar(); - } - } - - // using default operator =, assume everything initialized - else - { - assignAllVar(); - } - } - else if (Token::Match(ftok, "%var% (") && ftok->str() != "if") - { - // Passing "this" => assume that everything is initialized - for (const Token *tok2 = ftok->next()->link(); tok2 && tok2 != ftok; tok2 = tok2->previous()) - { - if (tok2->str() == "this") - { - assignAllVar(); - return; - } - } - - // recursive call / calling overloaded function - // assume that all variables are initialized - if (std::find(callstack.begin(), callstack.end(), ftok->str()) != callstack.end()) - { - assignAllVar(); - return; - } - - // check if member function - std::list::const_iterator it; - for (it = functionList.begin(); it != functionList.end(); ++it) - { - if (ftok->str() == it->tokenDef->str() && it->type != Func::Constructor) - break; - } - - // member function found - if (it != functionList.end()) - { - // member function has implementation - if (it->hasBody) - { - // initialize variable use list using member function - callstack.push_back(ftok->str()); - initializeVarList(*it, callstack); - callstack.pop_back(); - } - - // there is a called member function, but it has no implementation, so we assume it initializes everything - else - { - assignAllVar(); - } - } - - // not member function - else - { - // could be a base class virtual function, so we assume it initializes everything - if (func.type != Func::Constructor && isBaseClassFunc(ftok)) - { - /** @todo False Negative: we should look at the base class functions to see if they - * call any derived class virtual functions that change the derived class state - */ - assignAllVar(); - } - - // has friends, so we assume it initializes everything - if (!friendList.empty()) - assignAllVar(); - - // the function is external and it's neither friend nor inherited virtual function. - // assume all variables that are passed to it are initialized.. - else - { - unsigned int indentlevel2 = 0; - for (const Token *tok = ftok->tokAt(2); tok; tok = tok->next()) - { - if (tok->str() == "(") - ++indentlevel2; - else if (tok->str() == ")") - { - if (indentlevel2 == 0) - break; - --indentlevel2; - } - if (tok->isName()) - { - assignVar(tok->str()); - } - } - } - } - } - - // Assignment of member variable? - else if (Token::Match(ftok, "%var% =")) - { - assignVar(ftok->str()); - } - - // Assignment of array item of member variable? - else if (Token::Match(ftok, "%var% [ %any% ] =")) - { - assignVar(ftok->str()); - } - - // Assignment of member of array item of member variable? - else if (Token::Match(ftok, "%var% [ %any% ] . %var% =") || - Token::Match(ftok, "%var% [ %any% ] . %var% . %var% =")) - { - assignVar(ftok->str()); - } - - // Assignment of array item of member variable? - else if (Token::Match(ftok, "%var% [ %any% ] [ %any% ] =")) - { - assignVar(ftok->str()); - } - - // Assignment of array item of member variable? - else if (Token::Match(ftok, "* %var% =")) - { - assignVar(ftok->next()->str()); - } - - // Assignment of struct member of member variable? - else if (Token::Match(ftok, "%var% . %any% =")) - { - assignVar(ftok->str()); - } - - // The functions 'clear' and 'Clear' are supposed to initialize variable. - if (Token::Match(ftok, "%var% . clear|Clear (")) - { - assignVar(ftok->str()); - } - } -} diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 268aa3a95..177c981be 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -43,15 +43,13 @@ public: class SpaceInfo; - /** @brief Information about a member variable. Used when checking for uninitialized variables */ + /** @brief Information about a member variable. */ class Var { public: Var(const Token *token_, std::size_t index_, AccessControl access_, bool mutable_, bool static_, bool const_, bool class_, const SpaceInfo *type_) : token(token_), index(index_), - assign(false), - init(false), access(access_), isMutable(mutable_), isStatic(static_), @@ -67,12 +65,6 @@ public: /** @brief order declared */ std::size_t index; - /** @brief has this variable been assigned? */ - bool assign; - - /** @brief has this variable been initialized? */ - bool init; - /** @brief what section is this variable declared in? */ AccessControl access; // public/protected/private @@ -187,43 +179,14 @@ public: */ SpaceInfo * findInNestedList(const std::string & name); - /** - * @brief assign a variable in the varlist - * @param varname name of variable to mark assigned - */ - void assignVar(const std::string &varname); - - /** - * @brief initialize a variable in the varlist - * @param varname name of variable to mark initialized - */ - void initVar(const std::string &varname); - void addVar(const Token *token_, AccessControl access_, bool mutable_, bool static_, bool const_, bool class_, const SpaceInfo *type_) { varlist.push_back(Var(token_, varlist.size(), access_, mutable_, static_, const_, class_, type_)); } - /** - * @brief set all variables in list assigned - */ - void assignAllVar(); - - /** - * @brief set all variables in list not assigned and not initialized - */ - void clearAllVar(); - /** @brief initialize varlist */ void getVarList(); - /** - * @brief parse a scope for a constructor or member function and set the "init" flags in the provided varlist - * @param func reference to the function that should be checked - * @param callstack the function doesn't look into recursive function calls. - */ - void initializeVarList(const Func &func, std::list &callstack); - const Func *getDestructor() const; /** @@ -234,8 +197,6 @@ public: */ unsigned int getNestedNonFunctions() const; - bool isBaseClassFunc(const Token *tok); - bool hasDefaultConstructor() const; private: