From 1842a122daa2d1321470c75e0344122b10195324 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Tue, 23 Nov 2010 18:41:07 +0100 Subject: [PATCH] reuse symbol database in checkmemoryleak.cpp. ticket: #2219 --- lib/checkclass.cpp | 13 ++- lib/checkclass.h | 3 +- lib/checkmemoryleak.cpp | 115 ++++++++--------------- lib/checkmemoryleak.h | 4 +- lib/symboldatabase.cpp | 28 +++++- lib/tokenize.cpp | 196 ++++------------------------------------ lib/tokenize.h | 14 +-- test/testmemleak.cpp | 20 ++++ test/testtokenize.cpp | 110 ---------------------- 9 files changed, 119 insertions(+), 384 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 53aa59ce4..22576ac58 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -43,14 +43,15 @@ CheckClass instance; CheckClass::CheckClass(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) : Check(tokenizer, settings, errorLogger), - symbolDatabase(NULL) + symbolDatabase(NULL), ownSymbolDatabase(false) { } CheckClass::~CheckClass() { - delete symbolDatabase; + if (ownSymbolDatabase) + delete symbolDatabase; } void CheckClass::createSymbolDatabase() @@ -59,7 +60,13 @@ void CheckClass::createSymbolDatabase() if (symbolDatabase) return; - symbolDatabase = new SymbolDatabase(_tokenizer, _settings, _errorLogger); + if (_tokenizer->_symbolDatabase) + symbolDatabase = _tokenizer->_symbolDatabase; + else + { + symbolDatabase = new SymbolDatabase(_tokenizer, _settings, _errorLogger); + ownSymbolDatabase = true; + } } //--------------------------------------------------------------------------- diff --git a/lib/checkclass.h b/lib/checkclass.h index 1d6b24938..39cd9e7a6 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -36,7 +36,7 @@ class CheckClass : public Check { public: /** @brief This constructor is used when registering the CheckClass */ - CheckClass() : Check(), symbolDatabase(NULL) + CheckClass() : Check(), symbolDatabase(NULL), ownSymbolDatabase(false) { } /** @brief This constructor is used when running checks. */ @@ -113,6 +113,7 @@ private: void createSymbolDatabase(); SymbolDatabase *symbolDatabase; + bool ownSymbolDatabase; // Reporting errors.. void noConstructorError(const Token *tok, const std::string &classname, bool isStruct); diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 561de989d..cef03ce50 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -2573,81 +2573,49 @@ void CheckMemoryLeakInFunction::check() void CheckMemoryLeakInClass::check() { - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) - { - if (tok->str() == "{") - tok = tok->link(); + SymbolDatabase * symbolDatabase = _tokenizer->_symbolDatabase; + bool ownSymbolDatabase = false; - else if (Token::Match(tok, "class %var% [{:]")) + if (symbolDatabase == NULL) + { + symbolDatabase = new SymbolDatabase(_tokenizer, _settings, _errorLogger); + ownSymbolDatabase = true; + } + + std::list::iterator i; + + for (i = symbolDatabase->spaceInfoList.begin(); i != symbolDatabase->spaceInfoList.end(); ++i) + { + const SymbolDatabase::SpaceInfo *info = *i; + + // only check classes and structures + if (info->type == SymbolDatabase::SpaceInfo::Class) { - std::vector classname; - classname.push_back(tok->strAt(1)); - parseClass(tok, classname); + std::list::const_iterator var; + for (var = info->varlist.begin(); var != info->varlist.end(); ++var) + { + if (!var->isStatic && var->token->previous()->str() == "*") + { + // allocation but no deallocation of private variables in public function.. + if (var->access == SymbolDatabase::Private && var->token->tokAt(-2)->isStandardType()) + checkPublicFunctions(var->token, var->token->varId()); + + if (var->token->tokAt(-2)->isStandardType()) + variable(info, var->token); + } + } } } + + if (ownSymbolDatabase) + delete symbolDatabase; } -void CheckMemoryLeakInClass::parseClass(const Token *tok1, std::vector &classname) -{ - // Go into class. - while (tok1 && tok1->str() != "{") - tok1 = tok1->next(); - tok1 = tok1 ? tok1->next() : 0; - - // are we parsing the private scope of the class? - bool privateScope = true; - - unsigned int indentlevel = 0; - for (const Token *tok = tok1; tok; tok = tok->next()) - { - if (tok->str() == "{") - ++indentlevel; - - else if (tok->str() == "}") - { - if (indentlevel == 0) - return; - --indentlevel; - } - - else if (tok->isName() && tok->str().find(":") != std::string::npos) - privateScope = bool(tok->str() == "private:"); - - // Only parse this particular class.. not subclasses - if (indentlevel > 0) - continue; - - // skip static variables.. - if (Token::Match(tok, "static %type% * %var% ;")) - { - tok = tok->tokAt(4); - } - - // Declaring subclass.. recursive checking - if (Token::Match(tok, "class %var% [{:]")) - { - classname.push_back(tok->strAt(1)); - parseClass(tok, classname); - classname.pop_back(); - } - - // Declaring member variable.. check allocations and deallocations - if (Token::Match(tok->previous(), ";|{|}|private:|protected:|public: %type% * %var% ;")) - { - // allocation but no deallocation of private variables in public function.. - if (privateScope && tok->isStandardType()) - checkPublicFunctions(tok1, tok->tokAt(2)->varId()); - - if (!isclass(_tokenizer, tok)) - variable(classname.back(), tok->tokAt(2)); - } - } -} - -void CheckMemoryLeakInClass::variable(const std::string &classname, const Token *tokVarname) +void CheckMemoryLeakInClass::variable(const SymbolDatabase::SpaceInfo *classinfo, const Token *tokVarname) { const std::string varname = tokVarname->strAt(0); + const std::string classname = classinfo->className; // Check if member variable has been allocated and deallocated.. CheckMemoryLeak::AllocType Alloc = CheckMemoryLeak::No; @@ -2656,14 +2624,13 @@ void CheckMemoryLeakInClass::variable(const std::string &classname, const Token bool allocInConstructor = false; bool deallocInDestructor = false; - // Loop through all tokens. Inspect member functions - int indent_ = 0; - const Token *functionToken = _tokenizer->findClassFunction(_tokenizer->tokens(), classname.c_str(), "~| %var%", indent_); - while (functionToken) + // Inspect member functions + std::list::const_iterator func; + for (func = classinfo->functionList.begin(); func != classinfo->functionList.end(); ++func) { - const bool constructor(Token::Match(functionToken, (classname + " (").c_str()) || Token::Match(functionToken, (classname + " :: " + classname + " (").c_str())); - const bool destructor(functionToken->str() == "~" || functionToken->tokAt(2)->str() == "~"); - + const Token *functionToken = func->token; + const bool constructor = func->type == SymbolDatabase::Func::Constructor; + const bool destructor = func->type == SymbolDatabase::Func::Destructor; unsigned int indent = 0; bool initlist = false; for (const Token *tok = functionToken; tok; tok = tok->next()) @@ -2765,8 +2732,6 @@ void CheckMemoryLeakInClass::variable(const std::string &classname, const Token } } } - - functionToken = _tokenizer->Tokenizer::findClassFunction(functionToken->next(), classname.c_str(), "~| %var%", indent_); } if (allocInConstructor && !deallocInDestructor) diff --git a/lib/checkmemoryleak.h b/lib/checkmemoryleak.h index d77ed655a..da3d1eca3 100644 --- a/lib/checkmemoryleak.h +++ b/lib/checkmemoryleak.h @@ -35,6 +35,7 @@ */ #include "check.h" +#include "symboldatabase.h" #include #include @@ -376,8 +377,7 @@ public: void check(); private: - void parseClass(const Token *tok1, std::vector &classname); - void variable(const std::string &classname, const Token *tokVarname); + void variable(const SymbolDatabase::SpaceInfo *spaceinfo, const Token *tokVarname); /** Public functions: possible double-allocation */ void checkPublicFunctions(const Token *classtok, const unsigned int varid); diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index ff81e69bf..4cfa2bfe5 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -256,6 +256,11 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti if (tok->previous() && tok->previous()->str() == "::") addFunction(&info, &tok, argStart); + // class destructor + else if (tok->previous() && tok->previous()->str() == "~" && + tok->previous()->previous() && tok->previous()->previous()->str() == "::") + addFunction(&info, &tok, argStart); + // regular function else { @@ -493,11 +498,17 @@ bool SymbolDatabase::argsMatch(const Token *first, const Token *second, const st void SymbolDatabase::addFunction(SpaceInfo **info, const Token **tok, const Token *argStart) { - const Token *tok1 = (*tok)->tokAt(-2); // skip class/struct name int count = 0; bool added = false; std::string path; unsigned int path_length = 0; + const Token *tok1; + + // skip class/struct name + if ((*tok)->previous()->str() == "~") + tok1 = (*tok)->tokAt(-3); + else + tok1 = (*tok)->tokAt(-2); // back up to head of path while (tok1 && tok1->previous() && tok1->previous()->str() == "::") @@ -565,6 +576,17 @@ void SymbolDatabase::addFunction(SpaceInfo **info, const Token **tok, const Toke func->arg = argStart; } } + else if (func->type == SymbolDatabase::Func::Destructor && + (*tok)->previous()->str() == "~" && + func->tokenDef->str() == (*tok)->str()) + { + if (argsMatch(func->tokenDef->next(), (*tok)->next(), path, path_length)) + { + func->hasBody = true; + func->token = *tok; + func->arg = argStart; + } + } else if (func->tokenDef->str() == (*tok)->str()) { if (argsMatch(func->tokenDef->next(), (*tok)->next(), path, path_length)) @@ -576,7 +598,7 @@ void SymbolDatabase::addFunction(SpaceInfo **info, const Token **tok, const Toke (!func->isConst && (*tok)->next()->link()->next()->str() != "const")) { func->hasBody = true; - func->token = (*tok); + func->token = *tok; func->arg = argStart; } } @@ -586,7 +608,7 @@ void SymbolDatabase::addFunction(SpaceInfo **info, const Token **tok, const Toke { // todo check for const func->hasBody = true; - func->token = (*tok); + func->token = *tok; func->arg = argStart; } } diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 8cc1ef174..31a200f05 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -30,6 +30,7 @@ #include "errorlogger.h" #include "check.h" #include "path.h" +#include "symboldatabase.h" #include #include @@ -52,6 +53,7 @@ Tokenizer::Tokenizer() _tokens = 0; _tokensBack = 0; _codeWithTemplates = false; + _symbolDatabase = NULL; } Tokenizer::Tokenizer(const Settings *settings, ErrorLogger *errorLogger) @@ -60,11 +62,13 @@ Tokenizer::Tokenizer(const Settings *settings, ErrorLogger *errorLogger) _tokens = 0; _tokensBack = 0; _codeWithTemplates = false; + _symbolDatabase = NULL; } Tokenizer::~Tokenizer() { deallocateTokens(); + delete _symbolDatabase; } //--------------------------------------------------------------------------- @@ -3737,7 +3741,8 @@ void Tokenizer::simplifySizeof() bool Tokenizer::simplifyTokenList() { // clear the _functionList so it can't contain dead pointers - _functionList.clear(); + delete _symbolDatabase; + _symbolDatabase = NULL; for (Token *tok = _tokens; tok; tok = tok->next()) { @@ -7461,11 +7466,19 @@ void Tokenizer::simplifyStd() const Token *Tokenizer::getFunctionTokenByName(const char funcname[]) const { - for (unsigned int i = 0; i < _functionList.size(); ++i) + if (_symbolDatabase == NULL) + return NULL; + + std::list::iterator i; + + for (i = _symbolDatabase->spaceInfoList.begin(); i != _symbolDatabase->spaceInfoList.end(); ++i) { - if (_functionList[i]->str() == funcname) + SymbolDatabase::SpaceInfo *info = *i; + + if (info->type == SymbolDatabase::SpaceInfo::Function) { - return _functionList[i]; + if (info->classDef->str() == funcname) + return info->classDef; } } return NULL; @@ -7474,80 +7487,7 @@ const Token *Tokenizer::getFunctionTokenByName(const char funcname[]) const void Tokenizer::fillFunctionList() { - _functionList.clear(); - - for (const Token *tok = _tokens; tok; tok = tok->next()) - { - if (tok->str() == "{") - { - tok = tok->link(); - if (!tok) - break; - continue; - } - - if (Token::Match(tok, "%var% (")) - { - // Check if this is the first token of a function implementation.. - for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) - { - if (tok2->str() == ";") - { - tok = tok2; - break; - } - - else if (tok2->str() == "{") - { - break; - } - - else if (tok2->str() == ")") - { - if (Token::Match(tok2, ") const| {")) - { - _functionList.push_back(tok); - tok = tok2; - } - else - { - tok = tok2; - while (tok->next() && !Token::Match(tok->next(), "[;{]")) - tok = tok->next(); - } - break; - } - } - } - } - - // If the _functionList functions with duplicate names, remove them - /** @todo handle when functions with the same name */ - for (unsigned int func1 = 0; func1 < _functionList.size();) - { - bool hasDuplicates = false; - for (unsigned int func2 = func1 + 1; func2 < _functionList.size();) - { - if (_functionList[func1]->str() == _functionList[func2]->str()) - { - hasDuplicates = true; - _functionList.erase(_functionList.begin() + static_cast(func2)); - } - else - { - ++func2; - } - } - - if (! hasDuplicates) - { - ++func1; - } - else - { - _functionList.erase(_functionList.begin() + static_cast(func1)); - } - } + _symbolDatabase = new SymbolDatabase(this, _settings, _errorLogger); } //--------------------------------------------------------------------------- @@ -7604,106 +7544,6 @@ std::string Tokenizer::file(const Token *tok) const //--------------------------------------------------------------------------- -const Token * Tokenizer::findClassFunction(const Token *tok, const std::string &classname, const std::string &funcname, int &indentlevel, bool isStruct) const -{ - if (indentlevel < 0 || tok == NULL) - return NULL; - /* - // TODO: This is currently commented out as updateClassList doesn't - // fully work yet and call to updateClassList is currently also - // commented out. - - //std::cout << tok->str()<<"--\n"; - if( tok == _tokens || tok->str() == "class" || tok->str() == "struct") - tok = 0; - - std::map::const_iterator iter; - iter = _classInfoList.find(classname); - if( iter == _classInfoList.end() ) - return NULL; - - for( size_t i = 0; i < iter->second._memberFunctions.size(); i++ ) - if( Token::Match( iter->second._memberFunctions[i]._declaration, funcname ) ) - { - if( tok != 0 ) - { - if( tok == iter->second._memberFunctions[i]._implementation || - tok->previous() == iter->second._memberFunctions[i]._implementation) - tok = 0; - continue; - } - - return iter->second._memberFunctions[i]._implementation; - } - - return NULL; - */ - const std::string classPattern(std::string(isStruct ? "struct " : "class ") + classname + " :|{"); - const std::string internalPattern(std::string("!!~ ") + funcname + " ("); - const std::string externalPattern(classname + " :: " + funcname + " ("); - - for (; tok; tok = tok->next()) - { - if (indentlevel == 0 && Token::Match(tok, classPattern.c_str())) - { - while (tok && tok->str() != "{") - tok = tok->next(); - if (tok) - tok = tok->next(); - if (! tok) - break; - indentlevel = 1; - } - - if (tok->str() == "{") - { - // If indentlevel==0 don't go to indentlevel 1. Skip the block. - if (indentlevel > 0) - ++indentlevel; - - else - { - // skip the block - tok = tok->link(); - if (tok == NULL) - return NULL; - - continue; - } - } - - if (tok->str() == "}") - { - --indentlevel; - if (indentlevel < 0) - return NULL; - } - - if (indentlevel == 1) - { - // Member function implemented in the class declaration? - if (Token::Match(tok->previous(), internalPattern.c_str())) - { - const Token *tok2 = tok; - while (tok2 && tok2->str() != "{" && tok2->str() != ";") - tok2 = tok2->next(); - if (tok2 && tok2->str() == "{") - return tok; - } - } - - else if (indentlevel == 0 && Token::Match(tok, externalPattern.c_str())) - { - if (!Token::simpleMatch(tok->previous(), "::")) - return tok; - } - } - - // Not found - return NULL; -} -//--------------------------------------------------------------------------- - void Tokenizer::syntaxError(const Token *tok) { std::list locationList; diff --git a/lib/tokenize.h b/lib/tokenize.h index 3cc3509fb..8cd5c6466 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -29,6 +29,7 @@ class Token; class ErrorLogger; class Settings; +class SymbolDatabase; /// @addtogroup Core /// @{ @@ -124,17 +125,6 @@ public: std::string file(const Token *tok) const; - /** - * Find a class or struct member function - * @param tok where to begin the search - * @param classname name of class - * @param funcname name of function ("~ Fred" => destructor for fred, "%var%" => any function) - * @param indentlevel Just an integer that you initialize to 0 before the first call. - * @param isStruct is it a struct - * @return First matching token or NULL. - */ - const Token *findClassFunction(const Token *tok, const std::string &classname, const std::string &funcname, int &indentlevel, bool isStruct = false) const; - /** * get error messages */ @@ -285,7 +275,7 @@ public: /** Simplify "if else" */ void elseif(); - std::vector _functionList; + SymbolDatabase * _symbolDatabase; void addtoken(const char str[], const unsigned int lineno, const unsigned int fileno, bool split = false); void addtoken(const Token *tok, const unsigned int lineno, const unsigned int fileno); diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index e50988c63..be43055c9 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -3139,6 +3139,7 @@ private: TEST_CASE(class16); TEST_CASE(class17); TEST_CASE(class18); + TEST_CASE(class19); // ticket #2219 TEST_CASE(staticvar); @@ -3528,6 +3529,25 @@ private: ASSERT_EQUALS("", errout.str()); } + void class19() + { + // Ticket #2219 + check("class Foo\n" + "{\n" + "private:\n" + " TRadioButton* rp1;\n" + " TRadioButton* rp2;\n" + "public:\n" + " Foo();\n" + "};\n" + "Foo::Foo()\n" + "{\n" + " rp1 = new TRadioButton(this);\n" + " rp2 = new TRadioButton(this);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void staticvar() { check("class A\n" diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 296e355e3..ce530e8d5 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -67,10 +67,6 @@ private: TEST_CASE(inlineasm); - TEST_CASE(dupfuncname); - - TEST_CASE(const_and_volatile_functions); - TEST_CASE(ifAddBraces1); TEST_CASE(ifAddBraces2); TEST_CASE(ifAddBraces3); @@ -204,9 +200,6 @@ private: TEST_CASE(simplify_constants); TEST_CASE(simplify_constants2); - TEST_CASE(findClassFunction1); - TEST_CASE(findClassFunction2); - TEST_CASE(vardecl1); TEST_CASE(vardecl2); TEST_CASE(vardecl3); @@ -587,61 +580,6 @@ private: } - void dupfuncname() - { - const char code[] = "void a()\n" - "{ }\n" - "void a(int i)\n" - "{ }\n" - "void b()\n" - "{ }\n"; - // tokenize.. - Tokenizer tokenizer; - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.cpp"); - - tokenizer.fillFunctionList(); - - ASSERT_EQUALS(1, static_cast(tokenizer._functionList.size())); - ASSERT_EQUALS("b", tokenizer._functionList[0]->str()); - } - - void const_and_volatile_functions() - { - const char code[] = "class B\n\ - {\n\ - public:\n\ - void a();\n\ - void b() const;\n\ - void c() volatile;\n\ - };\n\ - \n\ - void B::a()\n\ - {}\n\ - \n\ - void B::b() const\n\ - {}\n\ - \n\ - void B::c() volatile\n\ - {}\n"; - - - // tokenize.. - Tokenizer tokenizer; - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.cpp"); - - tokenizer.fillFunctionList(); - - ASSERT_EQUALS(3, static_cast(tokenizer._functionList.size())); - if (tokenizer._functionList.size() == 3) - { - ASSERT_EQUALS("a", tokenizer._functionList[0]->str()); - ASSERT_EQUALS("b", tokenizer._functionList[1]->str()); - ASSERT_EQUALS("c", tokenizer._functionList[2]->str()); - } - } - void pointers_condition() { ASSERT_EQUALS("( p )", tokenizeAndStringify("( p != NULL )", true)); @@ -3531,54 +3469,6 @@ private: ASSERT_EQUALS(oss.str(), ostr.str()); } - - void findClassFunction1() - { - const char code[] = - "class Fred" - "{\n" - "public:\n" - " Fred()\n" - " { }\n" - "};\n"; - - // tokenize.. - Tokenizer tokenizer; - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.cpp"); - - int i; - - i = 0; - const Token *tok = tokenizer.findClassFunction(tokenizer.tokens(), "Fred", "%var%", i); - ASSERT_EQUALS(true, Token::simpleMatch(tok, "Fred ( ) {")); - tok = tokenizer.findClassFunction(tok->next(), "Fred", "%var%", i); - ASSERT_EQUALS(0, tok ? 1 : 0); - } - - void findClassFunction2() - { - const char code[] = - "struct Fred" - "{\n" - " Fred()\n" - " { }\n" - "};\n"; - - // tokenize.. - Tokenizer tokenizer; - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.cpp"); - - int i; - - i = 0; - const Token *tok = tokenizer.findClassFunction(tokenizer.tokens(), "Fred", "%var%", i, true); - ASSERT_EQUALS(true, Token::simpleMatch(tok, "Fred ( ) {")); - tok = tokenizer.findClassFunction(tok->next(), "Fred", "%var%", i, false); - ASSERT_EQUALS(0, tok ? 1 : 0); - } - void vardecl1() { const char code[] = "unsigned int a, b;";