From 1ee980184eb8caa46b0a166bb76074be26622a30 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Tue, 16 Oct 2012 06:11:28 +0200 Subject: [PATCH] Fixed #3190 (SymbolDatabase: Parse of sub class constructor fails) --- lib/checkmemoryleak.cpp | 4 ++ lib/symboldatabase.cpp | 135 ++++++++++++++++++++++++++++++++---- lib/symboldatabase.h | 21 ++++++ test/testconstructors.cpp | 19 +++++ test/testio.cpp | 6 +- test/testsymboldatabase.cpp | 2 +- 6 files changed, 169 insertions(+), 18 deletions(-) diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 30b9bc3c5..6b73161ba 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -2220,6 +2220,10 @@ void CheckMemoryLeakInFunction::check() if (!var->isPointer() && var->typeStartToken()->str() != "int") continue; + // check for known class without implementation (forward declaration) + if (var->isPointer() && var->type() && var->type()->isForwardDeclaration()) + continue; + unsigned int sz = _tokenizer->sizeOfType(var->typeStartToken()); if (sz < 1) sz = 1; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index ef782e1eb..59abbce21 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -52,17 +52,33 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti // find all scopes for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { // Locate next class - if (Token::Match(tok, "class|struct|union|namespace %var% [{:]")) { - scopeList.push_back(Scope(this, tok, scope)); + if (Token::Match(tok, "class|struct|union|namespace ::| %var% {|:|::")) { + const Token *tok2; + std::list names; - Scope *new_scope = &scopeList.back(); + if (tok->strAt(1) == "::") { + names.push_front(tok->strAt(2)); + tok2 = tok->tokAt(3); + } else { + tok2 = tok->tokAt(2); + names.push_front(tok->strAt(1)); + } + + while (tok2 && tok2->str() == "::") { + names.push_front(tok2->strAt(1)); + tok2 = tok2->tokAt(2); + } + + // make sure we have valid code + if (!tok2) + break; + + Scope *new_scope = getScope(names, tok, scope); if (tok->str() == "class") access[new_scope] = Private; else if (tok->str() == "struct") access[new_scope] = Public; - const Token *tok2 = tok->tokAt(2); - // only create base list for classes and structures if (new_scope->isClassOrStruct()) { // goto initial '{' @@ -89,8 +105,7 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti classAndStructTypes.insert(new_scope->className); // make the new scope the current scope - scope = &scopeList.back(); - scope->nestedIn->nestedList.push_back(scope); + scope = applyScope(new_scope); tok = tok2; } @@ -124,13 +139,30 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti } // forward declaration - else if (Token::Match(tok, "class|struct %var% ;")) { + else if (Token::Match(tok, "class|struct %var% ;") && + tok->strAt(-1) != "friend") { // fill the classAndStructTypes set.. classAndStructTypes.insert(tok->next()->str()); - /** @todo save forward declarations in database someday */ + scopeList.push_back(Scope(this, tok, scope)); + + Scope *new_scope = &scopeList.back(); + if (tok->str() == "class") + access[new_scope] = Private; + else if (tok->str() == "struct") + access[new_scope] = Public; + + // make the new scope the current scope + scope = &scopeList.back(); + scope->nestedIn->nestedList.push_back(scope); + + // save the forward declaration + _forwardDecls.push_back(scope); + + // leave the scope + scope = scope->nestedIn; + tok = tok->tokAt(2); - continue; } // using namespace @@ -205,7 +237,11 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti else { // check for end of scope if (tok == scope->classEnd) { - scope = scope->nestedIn; + if (!_back.empty() && _back.top().forward == scope) { + scope = _back.top().back; + _back.pop(); + } else + scope = scope->nestedIn; continue; } @@ -817,6 +853,68 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti } } +Scope * SymbolDatabase::getScope(const std::list &names, const Token *tok, Scope *scope) +{ + // check for forward declarations + if (_forwardDecls.empty()) { + // none so just return end of scope list + scopeList.push_back(Scope(this, tok, scope)); + + return &scopeList.back(); + } + + // got forward declarations so check for possible match + std::list::iterator decls; + for (decls = _forwardDecls.begin(); decls != _forwardDecls.end(); ++decls) { + Scope * decl = *decls; + Scope * next = decl; + std::list::const_iterator it; + for (it = names.begin(); next && it != names.end(); ++it) { + if (next->className == *it) + next = next->nestedIn; + else + break; + } + + // fully qualified name match + if (it == names.end()) { + _back.push(Back(decl, scope)); + decl->classDef = tok; + for (std::list::iterator nli = decl->nestedIn->nestedList.begin(); + nli != decl->nestedIn->nestedList.end(); ++nli) { + Scope *b = *nli; + if (b == decl) { + // remove forward declration + _forwardDecls.erase(decls); + break; + } + } + return decl; + } + } + + // no forward declaration found so just return end of scope list + scopeList.push_back(Scope(this, tok, scope)); + + return &scopeList.back(); +} + +Scope * SymbolDatabase::applyScope(Scope * scope) +{ + // check if there was a forward declaration + std::list::iterator it; + for (it = scope->nestedIn->nestedList.begin(); it != scope->nestedIn->nestedList.end(); ++it) { + if ((*it) == scope) + break; + } + + // add it if there was no forward declaration + if (it == scope->nestedIn->nestedList.end()) + scope->nestedIn->nestedList.push_back(scope); + + return scope; +} + bool SymbolDatabase::isFunction(const Token *tok, const Scope* outerScope, const Token **funcStart, const Token **argStart) { // function returning function pointer? '... ( ... %var% ( ... ))( ... ) {' @@ -1522,7 +1620,7 @@ void SymbolDatabase::printOut(const char *title) const count = scope->nestedList.size(); for (nsi = scope->nestedList.begin(); nsi != scope->nestedList.end(); ++nsi) { - std::cout << " " << &(*nsi) << " " << (*nsi)->type << " " << (*nsi)->className; + std::cout << " " << (*nsi) << " " << (*nsi)->type << " " << (*nsi)->className; if (count-- > 1) std::cout << ","; } @@ -1752,11 +1850,20 @@ Scope::Scope(SymbolDatabase *check_, const Token *classDef_, Scope *nestedIn_) : type = Scope::eGlobal; } else if (classDef->str() == "class") { type = Scope::eClass; - className = classDef->next()->str(); + const Token * tok = classDef->next(); + while (tok->strAt(1) == "::") + tok = tok->tokAt(2); + className = tok->str(); } else if (classDef->str() == "struct") { type = Scope::eStruct; + if (classDef->next()->str() == "::") { + const Token * tok = classDef->next(); + while (tok->strAt(1) == "::") + tok = tok->tokAt(2); + className = tok->str(); + } // anonymous and unnamed structs don't have a name - if (classDef->next()->str() != "{") + else if (classDef->next()->str() != "{") className = classDef->next()->str(); } else if (classDef->str() == "union") { type = Scope::eUnion; diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 46e1d2252..59a508440 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -25,6 +25,7 @@ #include #include #include +#include #include "config.h" #include "token.h" @@ -490,6 +491,10 @@ public: type == eTry || type == eCatch); } + bool isForwardDeclaration() const { + return isClassOrStruct() && classStart == NULL; + } + /** * @brief find if name is in nested list * @param name name of nested scope @@ -625,6 +630,9 @@ private: void addNewFunction(Scope **info, const Token **tok); static bool isFunction(const Token *tok, const Scope* outerScope, const Token **funcStart, const Token **argStart); + Scope * getScope(const std::list &names, const Token *tok, Scope *scope); + Scope * applyScope(Scope * scope); + /** class/struct types */ std::set classAndStructTypes; @@ -634,6 +642,19 @@ private: /** variable symbol table */ std::vector _variableList; + + /** Forward declarations list for fast searching. + Forward declarations are removed if implementation is found. */ + std::list _forwardDecls; + + /** Scope to jump back to when forward declaration implementation found. + Implementation information updates forward declaration. */ + struct Back { + Scope * forward; // forward declaration + Scope * back; // scope to jump back to + Back(Scope * f, Scope * b) : forward(f), back(b) { } + }; + std::stack _back; }; #endif diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index 3ab2b1e8d..f09168037 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -107,6 +107,7 @@ private: TEST_CASE(uninitVar20); // ticket #2867 TEST_CASE(uninitVar21); // ticket #2947 TEST_CASE(uninitVar22); // ticket #3043 + TEST_CASE(uninitVar23); // ticket #3190 TEST_CASE(uninitVarEnum); TEST_CASE(uninitVarStream); TEST_CASE(uninitVarTypedef); @@ -1561,6 +1562,24 @@ private: ASSERT_EQUALS("[test.cpp:7]: (warning) Member variable 'Fred::x' is not assigned a value in 'Fred::operator='.\n", errout.str()); } + void uninitVar23() { // ticket #3190 + check("class Foo { class Sub; };\n" + "class Bar { class Sub; };\n" + "class Bar::Sub {\n" + "public:\n" + " Sub() { }\n" + " int n;\n" + "};\n" + "class ::Foo::Sub {\n" + "public:\n" + " Sub() { }\n" + " int n;\n" + "};"); + + ASSERT_EQUALS("[test.cpp:10]: (warning) Member variable 'Sub::n' is not initialized in the constructor.\n" + "[test.cpp:5]: (warning) Member variable 'Sub::n' is not initialized in the constructor.\n", errout.str()); + } + void uninitVarArray1() { check("class John\n" "{\n" diff --git a/test/testio.cpp b/test/testio.cpp index d2e42f516..d2c18b9bc 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -637,9 +637,9 @@ private: " printf(\"%f\", f);\n" " printf(\"%p\", f);\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:3]: (warning) %u in format string (no. 1) requires an integer given in the argument list.\n" - "[test.cpp:4]: (warning) %f in format string (no. 1) requires an integer given in the argument list.\n" - "[test.cpp:5]: (warning) %p in format string (no. 1) requires an integer given in the argument list.\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (warning) %u in format string (no. 1) requires an unsigned integer given in the argument list.\n" + "[test.cpp:4]: (warning) %f in format string (no. 1) requires a floating point number given in the argument list.\n" + "[test.cpp:5]: (warning) %p in format string (no. 1) requires an address given in the argument list.\n", errout.str()); } }; diff --git a/test/testsymboldatabase.cpp b/test/testsymboldatabase.cpp index 8471905bd..4320fa266 100644 --- a/test/testsymboldatabase.cpp +++ b/test/testsymboldatabase.cpp @@ -672,7 +672,7 @@ private: seen_something = true; } } - TODO_ASSERT_EQUALS("works", "doesn't work", seen_something ? "works" : "doesn't work"); + ASSERT_EQUALS(true, seen_something); } }