diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 69e77ed48..ffcc55633 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -811,7 +811,7 @@ void CheckClass::virtualDestructor() for (unsigned int j = 0; j < info->derivedFrom.size(); ++j) { // Check if base class is public and exists in database - if (info->derivedFrom[j].access == SymbolDatabase::Public && info->derivedFrom[j].spaceInfo) + if (info->derivedFrom[j].access != SymbolDatabase::Private && info->derivedFrom[j].spaceInfo) { const SymbolDatabase::SpaceInfo *spaceInfo = info->derivedFrom[j].spaceInfo; @@ -977,12 +977,12 @@ void CheckClass::checkConst() // check if base class function is virtual if (!info->derivedFrom.empty()) { - if (symbolDatabase->isVirtualFunc(info, func->tokenDef)) + if (isVirtualFunc(info, func->tokenDef)) continue; } // if nothing non-const was found. write error.. - if (symbolDatabase->checkConstFunc(info, paramEnd)) + if (checkConstFunc(info, paramEnd)) { std::string classname = info->className; SymbolDatabase::SpaceInfo *nest = info->nestedIn; @@ -1010,6 +1010,249 @@ void CheckClass::checkConst() } } +bool CheckClass::isMemberVar(const SymbolDatabase::SpaceInfo *info, const Token *tok) +{ + const Token *tok1 = tok; + + while (tok->previous() && !Token::Match(tok->previous(), "}|{|;|public:|protected:|private:|return|:|?")) + { + if (Token::Match(tok->previous(), "* this")) + return true; + + tok = tok->previous(); + } + + if (tok->str() == "this") + return true; + + if (Token::Match(tok, "( * %var% ) [") || (Token::Match(tok, "( * %var% ) <<") && tok1->next()->str() == "<<")) + tok = tok->tokAt(2); + + // ignore class namespace + if (tok->str() == info->className && tok->next()->str() == "::") + tok = tok->tokAt(2); + + std::list::const_iterator var; + for (var = info->varlist.begin(); var != info->varlist.end(); ++var) + { + if (var->token->str() == tok->str()) + { + return !var->isMutable; + } + } + + // not found in this class + if (!info->derivedFrom.empty()) + { + // check each base class + for (unsigned int i = 0; i < info->derivedFrom.size(); ++i) + { + // find the base class + const SymbolDatabase::SpaceInfo *spaceInfo = info->derivedFrom[i].spaceInfo; + + // find the function in the base class + if (spaceInfo) + { + if (isMemberVar(spaceInfo, tok)) + return true; + } + } + } + + return false; +} + +bool CheckClass::isConstMemberFunc(const SymbolDatabase::SpaceInfo *info, const Token *tok) +{ + std::list::const_iterator func; + + for (func = info->functionList.begin(); func != info->functionList.end(); ++func) + { + if (func->tokenDef->str() == tok->str() && func->isConst) + return true; + } + + // not found in this class + if (!info->derivedFrom.empty()) + { + // check each base class + for (unsigned int i = 0; i < info->derivedFrom.size(); ++i) + { + // find the base class + const SymbolDatabase::SpaceInfo *spaceInfo = info->derivedFrom[i].spaceInfo; + + // find the function in the base class + if (spaceInfo) + { + if (isConstMemberFunc(spaceInfo, tok)) + return true; + } + } + } + + return false; +} + +bool CheckClass::checkConstFunc(const SymbolDatabase::SpaceInfo *info, const Token *tok) +{ + // if the function doesn't have any assignment nor function call, + // it can be a const function.. + unsigned int indentlevel = 0; + bool isconst = true; + for (const Token *tok1 = tok; tok1; tok1 = tok1->next()) + { + if (tok1->str() == "{") + ++indentlevel; + else if (tok1->str() == "}") + { + if (indentlevel <= 1) + break; + --indentlevel; + } + + // assignment.. = += |= .. + else if (tok1->str() == "=" || + (tok1->str().find("=") == 1 && + tok1->str().find_first_of("") == std::string::npos)) + { + if (tok1->previous()->varId() == 0 && !info->derivedFrom.empty()) + { + isconst = false; + break; + } + else if (isMemberVar(info, tok1->previous())) + { + isconst = false; + break; + } + else if (tok1->previous()->str() == "]") + { + // TODO: I assume that the assigned variable is a member variable + // don't assume it + isconst = false; + break; + } + else if (tok1->next()->str() == "this") + { + isconst = false; + break; + } + + // FIXME: I assume that a member union/struct variable is assigned. + else if (Token::Match(tok1->tokAt(-2), ". %var%")) + { + isconst = false; + break; + } + } + + // streaming: << + else if (tok1->str() == "<<" && isMemberVar(info, tok1->previous())) + { + isconst = false; + break; + } + + // increment/decrement (member variable?).. + else if (Token::Match(tok1, "++|--")) + { + isconst = false; + break; + } + + // function call.. + else if (Token::Match(tok1, "%var% (") && + !(Token::Match(tok1, "return|c_str|if|string") || tok1->isStandardType())) + { + if (!isConstMemberFunc(info, tok1)) + { + isconst = false; + break; + } + } + else if (Token::Match(tok1, "%var% < %any% > (")) + { + isconst = false; + break; + } + + // delete.. + else if (tok1->str() == "delete") + { + isconst = false; + break; + } + } + + return isconst; +} + +//--------------------------------------------------------------------------- + +// check if this function is defined virtual in the base classes +bool CheckClass::isVirtualFunc(const SymbolDatabase::SpaceInfo *info, const Token *functionToken) const +{ + // check each base class + for (unsigned int i = 0; i < info->derivedFrom.size(); ++i) + { + // check if base class exists in database + if (info->derivedFrom[i].spaceInfo) + { + const SymbolDatabase::SpaceInfo *derivedFrom = info->derivedFrom[i].spaceInfo; + + std::list::const_iterator func; + + // check if function defined in base class + for (func = derivedFrom->functionList.begin(); func != derivedFrom->functionList.end(); ++func) + { + if (func->isVirtual) + { + const Token *tok = func->tokenDef; + + if (tok->str() == functionToken->str()) + { + const Token *temp1 = tok->previous(); + const Token *temp2 = functionToken->previous(); + bool returnMatch = true; + + // check for matching return parameters + while (temp1->str() != "virtual") + { + if (temp1->str() != temp2->str()) + { + returnMatch = false; + break; + } + + temp1 = temp1->previous(); + temp2 = temp2->previous(); + } + + // check for matching function parameters + if (returnMatch && symbolDatabase->argsMatch(info, tok->tokAt(2), functionToken->tokAt(2), std::string(""), 0)) + { + return true; + } + } + } + } + + if (!derivedFrom->derivedFrom.empty()) + { + if (isVirtualFunc(derivedFrom, functionToken)) + return true; + } + } + else + { + // unable to find base class so assume it has a virtual function + return true; + } + } + + return false; +} + void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname) { reportError(tok, Severity::information, "functionConst", diff --git a/lib/checkclass.h b/lib/checkclass.h index 42603cf0e..c87b4fdd9 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -169,7 +169,12 @@ private: bool hasDeallocation(const Token *first, const Token *last); bool hasAssignSelf(const Token *first, const Token *last, const Token *rhs); - + // checkConst helper functions + bool isMemberVar(const SymbolDatabase::SpaceInfo *info, const Token *tok); + bool isConstMemberFunc(const SymbolDatabase::SpaceInfo *info, const Token *tok); + 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; }; /// @} //--------------------------------------------------------------------------- diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index e541101f7..2319c945f 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1784,247 +1784,3 @@ void SymbolDatabase::SpaceInfo::initializeVarList(const Func &func, std::listprevious() && !Token::Match(tok->previous(), "}|{|;|public:|protected:|private:|return|:|?")) - { - if (Token::Match(tok->previous(), "* this")) - return true; - - tok = tok->previous(); - } - - if (tok->str() == "this") - return true; - - if (Token::Match(tok, "( * %var% ) [") || (Token::Match(tok, "( * %var% ) <<") && tok1->next()->str() == "<<")) - tok = tok->tokAt(2); - - // ignore class namespace - if (tok->str() == info->className && tok->next()->str() == "::") - tok = tok->tokAt(2); - - std::list::const_iterator var; - for (var = info->varlist.begin(); var != info->varlist.end(); ++var) - { - if (var->token->str() == tok->str()) - { - return !var->isMutable; - } - } - - // not found in this class - if (!info->derivedFrom.empty()) - { - // check each base class - for (unsigned int i = 0; i < info->derivedFrom.size(); ++i) - { - // find the base class - const SpaceInfo *spaceInfo = info->derivedFrom[i].spaceInfo; - - // find the function in the base class - if (spaceInfo) - { - if (isMemberVar(spaceInfo, tok)) - return true; - } - } - } - - return false; -} - -bool SymbolDatabase::isConstMemberFunc(const SymbolDatabase::SpaceInfo *info, const Token *tok) -{ - std::list::const_iterator func; - - for (func = info->functionList.begin(); func != info->functionList.end(); ++func) - { - if (func->tokenDef->str() == tok->str() && func->isConst) - return true; - } - - // not found in this class - if (!info->derivedFrom.empty()) - { - // check each base class - for (unsigned int i = 0; i < info->derivedFrom.size(); ++i) - { - // find the base class - const SymbolDatabase::SpaceInfo *spaceInfo = info->derivedFrom[i].spaceInfo; - - // find the function in the base class - if (spaceInfo) - { - if (isConstMemberFunc(spaceInfo, tok)) - return true; - } - } - } - - return false; -} - -bool SymbolDatabase::checkConstFunc(const SymbolDatabase::SpaceInfo *info, const Token *tok) -{ - // if the function doesn't have any assignment nor function call, - // it can be a const function.. - unsigned int indentlevel = 0; - bool isconst = true; - for (const Token *tok1 = tok; tok1; tok1 = tok1->next()) - { - if (tok1->str() == "{") - ++indentlevel; - else if (tok1->str() == "}") - { - if (indentlevel <= 1) - break; - --indentlevel; - } - - // assignment.. = += |= .. - else if (tok1->str() == "=" || - (tok1->str().find("=") == 1 && - tok1->str().find_first_of("") == std::string::npos)) - { - if (tok1->previous()->varId() == 0 && !info->derivedFrom.empty()) - { - isconst = false; - break; - } - else if (isMemberVar(info, tok1->previous())) - { - isconst = false; - break; - } - else if (tok1->previous()->str() == "]") - { - // TODO: I assume that the assigned variable is a member variable - // don't assume it - isconst = false; - break; - } - else if (tok1->next()->str() == "this") - { - isconst = false; - break; - } - - // FIXME: I assume that a member union/struct variable is assigned. - else if (Token::Match(tok1->tokAt(-2), ". %var%")) - { - isconst = false; - break; - } - } - - // streaming: << - else if (tok1->str() == "<<" && isMemberVar(info, tok1->previous())) - { - isconst = false; - break; - } - - // increment/decrement (member variable?).. - else if (Token::Match(tok1, "++|--")) - { - isconst = false; - break; - } - - // function call.. - else if (Token::Match(tok1, "%var% (") && - !(Token::Match(tok1, "return|c_str|if|string") || tok1->isStandardType())) - { - if (!isConstMemberFunc(info, tok1)) - { - isconst = false; - break; - } - } - else if (Token::Match(tok1, "%var% < %any% > (")) - { - isconst = false; - break; - } - - // delete.. - else if (tok1->str() == "delete") - { - isconst = false; - break; - } - } - - return isconst; -} - -//--------------------------------------------------------------------------- - -// check if this function is defined virtual in the base classes -bool SymbolDatabase::isVirtualFunc(const SymbolDatabase::SpaceInfo *info, const Token *functionToken) const -{ - // check each base class - for (unsigned int i = 0; i < info->derivedFrom.size(); ++i) - { - // check if base class exists in database - if (info->derivedFrom[i].spaceInfo) - { - const SymbolDatabase::SpaceInfo *derivedFrom = info->derivedFrom[i].spaceInfo; - - std::list::const_iterator func; - - // check if function defined in base class - for (func = derivedFrom->functionList.begin(); func != derivedFrom->functionList.end(); ++func) - { - if (func->isVirtual) - { - const Token *tok = func->tokenDef; - - if (tok->str() == functionToken->str()) - { - const Token *temp1 = tok->previous(); - const Token *temp2 = functionToken->previous(); - bool returnMatch = true; - - // check for matching return parameters - while (temp1->str() != "virtual") - { - if (temp1->str() != temp2->str()) - { - returnMatch = false; - break; - } - - temp1 = temp1->previous(); - temp2 = temp2->previous(); - } - - // check for matching function parameters - if (returnMatch && argsMatch(info, tok->tokAt(2), functionToken->tokAt(2), std::string(""), 0)) - { - return true; - } - } - } - } - - if (!derivedFrom->derivedFrom.empty()) - { - if (isVirtualFunc(derivedFrom, functionToken)) - return true; - } - } - else - { - // unable to find base class so assume it has a virtual function - return true; - } - } - - return false; -} - diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 0445b35ba..268aa3a95 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -249,15 +249,6 @@ public: bool isVariableDeclaration(const Token* tok, const Token*& vartok, const Token*& typetok) const; }; - bool isMemberVar(const SpaceInfo *info, const Token *tok); - bool isConstMemberFunc(const SpaceInfo *info, const Token *tok); - bool checkConstFunc(const SpaceInfo *info, const Token *tok); - - const Token *initBaseInfo(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; - /** @brief Information about all namespaces/classes/structrues */ std::list spaceInfoList; @@ -269,6 +260,8 @@ public: */ const SpaceInfo *findVarType(const SpaceInfo *start, const Token *type) const; + bool argsMatch(const SpaceInfo *info, const Token *first, const Token *second, const std::string &path, unsigned int depth) const; + private: // Needed by Borland C++: @@ -276,9 +269,8 @@ private: void addFunction(SpaceInfo **info, const Token **tok, const Token *argStart); void addNewFunction(SpaceInfo **info, const Token **tok); - + const Token *initBaseInfo(SpaceInfo *info, const Token *tok); bool isFunction(const Token *tok, const Token **funcStart, const Token **argStart) const; - bool argsMatch(const SpaceInfo *info, const Token *first, const Token *second, const std::string &path, unsigned int depth) const; const Tokenizer *_tokenizer; const Settings *_settings; diff --git a/test/testclass.cpp b/test/testclass.cpp index f683dc502..a36dcd13a 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -1316,8 +1316,7 @@ private: checkVirtualDestructor("class Base { };\n" "class Derived : protected Base { public: ~Derived() { (void)11; } };"); - TODO_ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str()); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str()); checkVirtualDestructor("class Base { };\n" "class Derived : private Base { public: ~Derived() { (void)11; } };"); @@ -1338,8 +1337,7 @@ private: checkVirtualDestructor("class Base { public: ~Base(); };\n" "class Derived : protected Base { public: ~Derived() { (void)11; } };"); - TODO_ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str()); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (error) Class Base which is inherited by class Derived does not have a virtual destructor\n", errout.str()); checkVirtualDestructor("class Base { public: ~Base(); };\n" "class Derived : private Fred, public Base { public: ~Derived() { (void)11; } };");