Skip to content

Commit 35c2b80

Browse files
IOBYTEdanmar
authored andcommitted
Fixed danmar#3190 (SymbolDatabase: Parse of sub class constructor fails)
1 parent a219ed3 commit 35c2b80

6 files changed

Lines changed: 200 additions & 45 deletions

File tree

lib/checkmemoryleak.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2222,6 +2222,10 @@ void CheckMemoryLeakInFunction::check()
22222222
if (!var->isPointer() && var->typeStartToken()->str() != "int")
22232223
continue;
22242224

2225+
// check for known class without implementation (forward declaration)
2226+
if (var->isPointer() && var->type() && var->type()->isForwardDeclaration())
2227+
continue;
2228+
22252229
unsigned int sz = _tokenizer->sizeOfType(var->typeStartToken());
22262230
if (sz < 1)
22272231
sz = 1;

lib/symboldatabase.cpp

Lines changed: 150 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -49,50 +49,83 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
4949
// Store current access in each scope (depends on evaluation progress)
5050
std::map<const Scope*, AccessControl> access;
5151

52+
std::map<const Token *, Scope *> back;
53+
5254
// find all scopes
5355
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
5456
// Locate next class
55-
if (Token::Match(tok, "class|struct|union|namespace %var% [{:]")) {
56-
scopeList.push_back(Scope(this, tok, scope));
57+
if (Token::Match(tok, "class|struct|union|namespace ::| %var% {|:|::")) {
58+
const Token *tok2 = tok->tokAt(2);
5759

58-
Scope *new_scope = &scopeList.back();
59-
if (tok->str() == "class")
60-
access[new_scope] = Private;
61-
else if (tok->str() == "struct")
62-
access[new_scope] = Public;
60+
if (tok->strAt(1) == "::")
61+
tok2 = tok2->next();
6362

64-
const Token *tok2 = tok->tokAt(2);
63+
while (tok2 && tok2->str() == "::")
64+
tok2 = tok2->tokAt(2);
65+
66+
// make sure we have valid code
67+
if (!tok2 || !Token::Match(tok2, "{|:"))
68+
break;
69+
70+
Scope *new_scope = findScope(tok->next(), scope);
6571

66-
// only create base list for classes and structures
67-
if (new_scope->isClassOrStruct()) {
68-
// goto initial '{'
69-
tok2 = new_scope->initBaseInfo(tok);
72+
if (new_scope) {
73+
// only create base list for classes and structures
74+
if (new_scope->isClassOrStruct()) {
75+
// goto initial '{'
76+
tok2 = new_scope->initBaseInfo(tok, tok2);
77+
78+
// make sure we have valid code
79+
if (!tok2) {
80+
break;
81+
}
82+
}
83+
back[tok2->link()] = scope;
84+
new_scope->classDef = tok;
85+
new_scope->classStart = tok2;
86+
new_scope->classEnd = tok2->link();
87+
scope = new_scope;
88+
tok = tok2;
89+
} else {
90+
scopeList.push_back(Scope(this, tok, scope));
91+
new_scope = &scopeList.back();
92+
93+
if (tok->str() == "class")
94+
access[new_scope] = Private;
95+
else if (tok->str() == "struct")
96+
access[new_scope] = Public;
97+
98+
// only create base list for classes and structures
99+
if (new_scope->isClassOrStruct()) {
100+
// goto initial '{'
101+
tok2 = new_scope->initBaseInfo(tok, tok2);
102+
103+
// make sure we have valid code
104+
if (!tok2) {
105+
scopeList.pop_back();
106+
break;
107+
}
108+
}
109+
110+
new_scope->classStart = tok2;
111+
new_scope->classEnd = tok2->link();
70112

71113
// make sure we have valid code
72-
if (!tok2) {
114+
if (!new_scope->classEnd) {
73115
scopeList.pop_back();
74116
break;
75117
}
76-
}
77118

78-
new_scope->classStart = tok2;
79-
new_scope->classEnd = tok2->link();
119+
// fill the classAndStructTypes set..
120+
if (new_scope->isClassOrStruct())
121+
classAndStructTypes.insert(new_scope->className);
80122

81-
// make sure we have valid code
82-
if (!new_scope->classEnd) {
83-
scopeList.pop_back();
84-
break;
85-
}
123+
// make the new scope the current scope
124+
scope = &scopeList.back();
125+
scope->nestedIn->nestedList.push_back(scope);
86126

87-
// fill the classAndStructTypes set..
88-
if (new_scope->isClassOrStruct())
89-
classAndStructTypes.insert(new_scope->className);
90-
91-
// make the new scope the current scope
92-
scope = &scopeList.back();
93-
scope->nestedIn->nestedList.push_back(scope);
94-
95-
tok = tok2;
127+
tok = tok2;
128+
}
96129
}
97130

98131
// Namespace and unknown macro (#3854)
@@ -124,13 +157,20 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
124157
}
125158

126159
// forward declaration
127-
else if (Token::Match(tok, "class|struct %var% ;")) {
128-
// fill the classAndStructTypes set..
129-
classAndStructTypes.insert(tok->next()->str());
160+
else if (Token::Match(tok, "class|struct %var% ;") &&
161+
tok->strAt(-1) != "friend") {
162+
if (!findScope(tok->next(), scope)) {
163+
// fill the classAndStructTypes set..
164+
classAndStructTypes.insert(tok->next()->str());
165+
166+
scopeList.push_back(Scope(this, tok, scope));
130167

131-
/** @todo save forward declarations in database someday */
168+
Scope *new_scope = &scopeList.back();
169+
170+
// add scope
171+
scope->nestedList.push_back(new_scope);
172+
}
132173
tok = tok->tokAt(2);
133-
continue;
134174
}
135175

136176
// using namespace
@@ -205,7 +245,11 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
205245
else {
206246
// check for end of scope
207247
if (tok == scope->classEnd) {
208-
scope = scope->nestedIn;
248+
if (back.find(tok) != back.end()) {
249+
scope = back[tok];
250+
back.erase(tok);
251+
} else
252+
scope = scope->nestedIn;
209253
continue;
210254
}
211255

@@ -1065,7 +1109,8 @@ void SymbolDatabase::addClassFunction(Scope **scope, const Token **tok, const To
10651109
return;
10661110

10671111
// back up to head of path
1068-
while (tok1 && tok1->previous() && tok1->previous()->str() == "::") {
1112+
while (tok1 && tok1->previous() && tok1->previous()->str() == "::" &&
1113+
tok1->tokAt(-2) && tok1->tokAt(-2)->isName()) {
10691114
path = tok1->str() + " :: " + path;
10701115
tok1 = tok1->tokAt(-2);
10711116
count++;
@@ -1203,10 +1248,10 @@ void SymbolDatabase::addNewFunction(Scope **scope, const Token **tok)
12031248
}
12041249
}
12051250

1206-
const Token *Scope::initBaseInfo(const Token *tok)
1251+
const Token *Scope::initBaseInfo(const Token *tok, const Token *tok1)
12071252
{
12081253
// goto initial '{'
1209-
const Token *tok2 = tok->tokAt(2);
1254+
const Token *tok2 = tok1;
12101255
while (tok2 && tok2->str() != "{") {
12111256
// skip unsupported templates
12121257
if (tok2->str() == "<")
@@ -1542,7 +1587,7 @@ void SymbolDatabase::printOut(const char *title) const
15421587

15431588
count = scope->nestedList.size();
15441589
for (nsi = scope->nestedList.begin(); nsi != scope->nestedList.end(); ++nsi) {
1545-
std::cout << " " << &(*nsi) << " " << (*nsi)->type << " " << (*nsi)->className;
1590+
std::cout << " " << (*nsi) << " " << (*nsi)->type << " " << (*nsi)->className;
15461591
if (count-- > 1)
15471592
std::cout << ",";
15481593
}
@@ -2233,6 +2278,19 @@ Scope * Scope::findInNestedList(const std::string & name)
22332278

22342279
//---------------------------------------------------------------------------
22352280

2281+
Scope * Scope::findRecordInNestedList(const std::string & name)
2282+
{
2283+
std::list<Scope *>::const_iterator it;
2284+
2285+
for (it = nestedList.begin(); it != nestedList.end(); ++it) {
2286+
if ((*it)->className == name && (*it)->type != eFunction)
2287+
return (*it);
2288+
}
2289+
return 0;
2290+
}
2291+
2292+
//---------------------------------------------------------------------------
2293+
22362294
Scope * Scope::findInNestedListRecursive(const std::string & name)
22372295
{
22382296
std::list<Scope *>::iterator it;
@@ -2306,3 +2364,55 @@ bool SymbolDatabase::isCPP() const
23062364
{
23072365
return _tokenizer->isCPP();
23082366
}
2367+
2368+
//---------------------------------------------------------------------------
2369+
2370+
const Scope * SymbolDatabase::findScope(const Token *tok, const Scope *startScope) const
2371+
{
2372+
return findScope(tok, startScope);
2373+
}
2374+
2375+
Scope * SymbolDatabase::findScope(const Token *tok, Scope *startScope)
2376+
{
2377+
// absolute path
2378+
if (tok->str() == "::") {
2379+
tok = tok->next();
2380+
Scope *scope = &scopeList.front();
2381+
2382+
while (scope && tok && tok->isName()) {
2383+
scope = scope->findRecordInNestedList(tok->str());
2384+
2385+
if (scope) {
2386+
if (tok->strAt(1) == "::")
2387+
tok = tok->tokAt(2);
2388+
else
2389+
break;
2390+
}
2391+
2392+
}
2393+
2394+
return scope;
2395+
}
2396+
2397+
// relative path
2398+
else if (tok->isName()) {
2399+
Scope *scope = startScope;
2400+
2401+
while (scope && tok && tok->isName()) {
2402+
scope = scope->findRecordInNestedList(tok->str());
2403+
2404+
if (scope) {
2405+
if (tok->strAt(1) == "::")
2406+
tok = tok->tokAt(2);
2407+
else
2408+
break;
2409+
}
2410+
}
2411+
2412+
return scope;
2413+
}
2414+
2415+
// not a valid path
2416+
else
2417+
return 0;
2418+
}

lib/symboldatabase.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,12 +490,18 @@ class CPPCHECKLIB Scope {
490490
type == eTry || type == eCatch);
491491
}
492492

493+
bool isForwardDeclaration() const {
494+
return isClassOrStruct() && classStart == NULL;
495+
}
496+
493497
/**
494498
* @brief find if name is in nested list
495499
* @param name name of nested scope
496500
*/
497501
Scope * findInNestedList(const std::string & name);
498502

503+
Scope * findRecordInNestedList(const std::string & name);
504+
499505
/**
500506
* @brief find if name is in nested list
501507
* @param name name of nested scope
@@ -512,7 +518,7 @@ class CPPCHECKLIB Scope {
512518
type_, scope_));
513519
}
514520

515-
const Token *initBaseInfo(const Token *tok);
521+
const Token *initBaseInfo(const Token *tok, const Token *tok1);
516522

517523
/** @brief initialize varlist */
518524
void getVariableList();
@@ -594,6 +600,8 @@ class CPPCHECKLIB SymbolDatabase {
594600

595601
const Scope* findScopeByName(const std::string& name) const;
596602

603+
const Scope *findScope(const Token *tok, const Scope *startScope) const;
604+
597605
bool isClassOrStruct(const std::string &type) const {
598606
return bool(classAndStructTypes.find(type) != classAndStructTypes.end());
599607
}
@@ -627,6 +635,8 @@ class CPPCHECKLIB SymbolDatabase {
627635
void addNewFunction(Scope **info, const Token **tok);
628636
static bool isFunction(const Token *tok, const Scope* outerScope, const Token **funcStart, const Token **argStart);
629637

638+
Scope *findScope(const Token *tok, Scope *startScope);
639+
630640
/** class/struct types */
631641
std::set<std::string> classAndStructTypes;
632642

test/testconstructors.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ class TestConstructors : public TestFixture {
108108
TEST_CASE(uninitVar21); // ticket #2947
109109
TEST_CASE(uninitVar22); // ticket #3043
110110
TEST_CASE(uninitVar23); // ticket #3702
111+
TEST_CASE(uninitVar24); // ticket #3190
111112
TEST_CASE(uninitVarEnum);
112113
TEST_CASE(uninitVarStream);
113114
TEST_CASE(uninitVarTypedef);
@@ -1585,6 +1586,36 @@ class TestConstructors : public TestFixture {
15851586
"[test.cpp:14]: (warning) Member variable 'Fred::x' is not initialized in the constructor.\n", errout.str());
15861587
}
15871588

1589+
void uninitVar24() { // ticket #3190
1590+
check("class Foo;\n"
1591+
"class Bar;\n"
1592+
"class Sub;\n"
1593+
"class Foo { class Sub; };\n"
1594+
"class Bar { class Sub; };\n"
1595+
"class Bar::Sub {\n"
1596+
" int b;\n"
1597+
"public:\n"
1598+
" Sub() { }\n"
1599+
" Sub(int);\n"
1600+
"};\n"
1601+
"Bar::Sub::Sub(int) { };\n"
1602+
"class ::Foo::Sub {\n"
1603+
" int f;\n"
1604+
"public:\n"
1605+
" ~Sub();\n"
1606+
" Sub();\n"
1607+
"};\n"
1608+
"::Foo::Sub::~Sub() { }\n"
1609+
"::Foo::Sub::Sub() { }\n"
1610+
"class Foo;\n"
1611+
"class Bar;\n"
1612+
"class Sub;\n");
1613+
1614+
ASSERT_EQUALS("[test.cpp:20]: (warning) Member variable 'Sub::f' is not initialized in the constructor.\n"
1615+
"[test.cpp:9]: (warning) Member variable 'Sub::b' is not initialized in the constructor.\n"
1616+
"[test.cpp:12]: (warning) Member variable 'Sub::b' is not initialized in the constructor.\n", errout.str());
1617+
}
1618+
15881619
void uninitVarArray1() {
15891620
check("class John\n"
15901621
"{\n"

test/testio.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -637,9 +637,9 @@ class TestIO : public TestFixture {
637637
" printf(\"%f\", f);\n"
638638
" printf(\"%p\", f);\n"
639639
"}");
640-
TODO_ASSERT_EQUALS("[test.cpp:3]: (warning) %u in format string (no. 1) requires an integer given in the argument list.\n"
641-
"[test.cpp:4]: (warning) %f in format string (no. 1) requires an integer given in the argument list.\n"
642-
"[test.cpp:5]: (warning) %p in format string (no. 1) requires an integer given in the argument list.\n", "", errout.str());
640+
ASSERT_EQUALS("[test.cpp:3]: (warning) %u in format string (no. 1) requires an unsigned integer given in the argument list.\n"
641+
"[test.cpp:4]: (warning) %f in format string (no. 1) requires a floating point number given in the argument list.\n"
642+
"[test.cpp:5]: (warning) %p in format string (no. 1) requires an address given in the argument list.\n", errout.str());
643643

644644
// Ticket #4189 (Improve check (printf("%l") not detected)) tests (according to C99 7.19.6.1.7)
645645
// False positive tests

test/testsymboldatabase.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ class TestSymbolDatabase: public TestFixture {
675675
seen_something = true;
676676
}
677677
}
678-
TODO_ASSERT_EQUALS("works", "doesn't work", seen_something ? "works" : "doesn't work");
678+
ASSERT_EQUALS(true, seen_something);
679679
}
680680
}
681681

0 commit comments

Comments
 (0)