Skip to content

Commit 1f5265c

Browse files
zingsheimdanmar
authored andcommitted
Fixed cppcheck-opensource#6253 ([False Positive] Variable not initialized in the constructor)
1 parent bacc5ac commit 1f5265c

6 files changed

Lines changed: 200 additions & 129 deletions

File tree

lib/checkclass.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
485485
// Class constructor.. initializing variables like this
486486
// clKalle::clKalle() : var(value) { }
487487
if (initList) {
488-
if (level == 0 && Token::Match(ftok, "%name% {|(")) {
488+
if (level == 0 && Token::Match(ftok, "%name% {|(") && Token::Match(ftok->linkAt(1), "}|) ,|{")) {
489489
if (ftok->str() != func.name()) {
490490
initVar(ftok->str(), scope, usage);
491491
} else { // c++11 delegate constructor
@@ -514,21 +514,20 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
514514
}
515515
}
516516
}
517-
ftok = ftok->next();
518-
level++;
519517
} else if (level != 0 && Token::Match(ftok, "%name% =")) // assignment in the initializer: var(value = x)
520518
assignVar(ftok->str(), scope, usage);
521519

522-
else if (ftok->str() == "(")
520+
// Level handling
521+
if (ftok->link() && Token::Match(ftok, "(|<"))
523522
level++;
524-
else if (ftok->str() == ")" || ftok->str() == "}")
525-
level--;
526523
else if (ftok->str() == "{") {
527-
if (level == 0)
528-
initList = false;
524+
if (level != 0 ||
525+
(Token::Match(ftok->previous(), "%name%|>") && Token::Match(ftok->link(), "} ,|{")))
526+
level++;
529527
else
530-
ftok = ftok->link();
531-
}
528+
initList = false;
529+
} else if (ftok->link() && Token::Match(ftok, ")|>|}"))
530+
level--;
532531
}
533532

534533
if (initList)

lib/symboldatabase.cpp

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -572,8 +572,21 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
572572
}
573573

574574
// find start of function '{'
575-
while (end && end->str() != "{" && end->str() != ";")
576-
end = end->next();
575+
bool foundInitList = false;
576+
while (end && end->str() != "{" && end->str() != ";") {
577+
if (end->link() && Token::Match(end, "(|<")) {
578+
end = end->link();
579+
} else if (foundInitList &&
580+
Token::Match(end, "%name%|> {") &&
581+
Token::Match(end->linkAt(1), "} ,|{")) {
582+
end = end->linkAt(1);
583+
} else {
584+
if (end->str() == ":")
585+
foundInitList = true;
586+
end = end->next();
587+
}
588+
}
589+
577590
if (!end || end->str() == ";")
578591
continue;
579592

@@ -1843,17 +1856,23 @@ void SymbolDatabase::addNewFunction(Scope **scope, const Token **tok)
18431856
scopeList.push_back(Scope(this, tok1, *scope));
18441857
Scope *new_scope = &scopeList.back();
18451858

1846-
// skip to start of function
1847-
bool foundInitLit = false;
1848-
while (tok1 && (tok1->str() != "{" || (foundInitLit && tok1->previous()->isName()))) {
1849-
if (Token::Match(tok1, "(|{"))
1859+
// find start of function '{'
1860+
bool foundInitList = false;
1861+
while (tok1 && tok1->str() != "{" && tok1->str() != ";") {
1862+
if (tok1->link() && Token::Match(tok1, "(|<")) {
18501863
tok1 = tok1->link();
1851-
if (tok1->str() == ":")
1852-
foundInitLit = true;
1853-
tok1 = tok1->next();
1864+
} else if (foundInitList &&
1865+
Token::Match(tok1, "%name%|> {") &&
1866+
Token::Match(tok1->linkAt(1), "} ,|{")) {
1867+
tok1 = tok1->linkAt(1);
1868+
} else {
1869+
if (tok1->str() == ":")
1870+
foundInitList = true;
1871+
tok1 = tok1->next();
1872+
}
18541873
}
18551874

1856-
if (tok1) {
1875+
if (tok1 && tok1->str() == "{") {
18571876
new_scope->classStart = tok1;
18581877
new_scope->classEnd = tok1->link();
18591878

lib/tokenize.cpp

Lines changed: 80 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,37 @@
3636

3737
//---------------------------------------------------------------------------
3838

39+
namespace {
40+
// local struct used in setVarId
41+
// in order to store information about the scope
42+
struct scopeStackEntryType {
43+
scopeStackEntryType()
44+
:isExecutable(false), startVarid(0) {
45+
}
46+
scopeStackEntryType(bool _isExecutable, unsigned int _startVarid)
47+
:isExecutable(_isExecutable), startVarid(_startVarid) {
48+
}
49+
50+
const bool isExecutable;
51+
const unsigned int startVarid;
52+
};
53+
}
54+
3955
/**
4056
* is token pointing at function head?
4157
* @param tok A '(' or ')' token in a possible function head
4258
* @param endsWith string after function head
43-
* @return true if syntax seems to be a function head
59+
* @return token matching with endsWith if syntax seems to be a function head else nullptr
4460
*/
45-
static bool isFunctionHead(const Token *tok, const std::string &endsWith)
61+
static const Token * isFunctionHead(const Token *tok, const std::string &endsWith)
4662
{
4763
if (tok->str() == "(")
4864
tok = tok->link();
49-
if (Token::Match(tok, ") const| [;{]")) {
65+
if (Token::Match(tok, ") const| [;:{]")) {
5066
tok = tok->next();
5167
if (tok->isName())
5268
tok = tok->next();
53-
return endsWith.find(tok->str()) != std::string::npos;
69+
return (endsWith.find(tok->str()) != std::string::npos) ? tok : nullptr;
5470
}
5571
if (Token::Match(tok, ") const| throw (")) {
5672
tok = tok->next();
@@ -59,9 +75,9 @@ static bool isFunctionHead(const Token *tok, const std::string &endsWith)
5975
tok = tok->link()->next();
6076
while (tok && tok->isName())
6177
tok = tok->next();
62-
return tok && endsWith.find(tok->str()) != std::string::npos;
78+
return (tok && endsWith.find(tok->str()) != std::string::npos) ? tok : nullptr;
6379
}
64-
return false;
80+
return nullptr;
6581
}
6682
//---------------------------------------------------------------------------
6783

@@ -2422,43 +2438,6 @@ static void setVarIdStructMembers(Token **tok1,
24222438
}
24232439

24242440

2425-
static const Token * findInitListEndToken(const Token *tok)
2426-
{
2427-
if (!Token::Match(tok, ") noexcept|:") && !Token::simpleMatch(tok, "noexcept :"))
2428-
return nullptr;
2429-
2430-
if (tok->strAt(1) != ":") {
2431-
tok = tok->next();
2432-
if (tok->strAt(1) == "(")
2433-
tok = tok->linkAt(1);
2434-
}
2435-
2436-
tok = tok->tokAt(2);
2437-
2438-
while (tok) {
2439-
if (tok && tok->str()=="::")
2440-
tok = tok->next();
2441-
2442-
while (tok && Token::Match(tok, "%type% ::"))
2443-
tok = tok->tokAt(2);
2444-
2445-
if (Token::Match(tok, "%name% [({]")) {
2446-
tok = tok->linkAt(1);
2447-
if (!tok)
2448-
return nullptr;
2449-
tok = tok->next();
2450-
}
2451-
if (tok && tok->str()==",")
2452-
tok = tok->next();
2453-
else if (tok && tok->str()=="{")
2454-
return tok; // End of init list found
2455-
else
2456-
return nullptr;
2457-
}
2458-
2459-
return nullptr;
2460-
}
2461-
24622441
static void setVarIdClassDeclaration(Token * const startToken,
24632442
const std::map<std::string, unsigned int> &variableId,
24642443
const unsigned int scopeStartVarId,
@@ -2481,15 +2460,24 @@ static void setVarIdClassDeclaration(Token * const startToken,
24812460

24822461
// replace varids..
24832462
unsigned int indentlevel = 0;
2484-
const Token * initListEndToken = nullptr;
2463+
bool initList = false;
2464+
const Token *initListArgLastToken = nullptr;
24852465
for (Token *tok = startToken->next(); tok != endToken; tok = tok->next()) {
2466+
if (initList) {
2467+
if (tok == initListArgLastToken)
2468+
initListArgLastToken = nullptr;
2469+
else if (!initListArgLastToken &&
2470+
Token::Match(tok->previous(), "%name%|>|>> {|(") &&
2471+
Token::Match(tok->link(), "}|) ,|{"))
2472+
initListArgLastToken = tok->link();
2473+
}
24862474
if (tok->str() == "{") {
2487-
if (tok == initListEndToken)
2488-
initListEndToken = nullptr;
2475+
if (initList && !initListArgLastToken)
2476+
initList = false;
24892477
++indentlevel;
24902478
} else if (tok->str() == "}")
24912479
--indentlevel;
2492-
else if (initListEndToken && indentlevel == 0 && Token::Match(tok->previous(), "[,:] %name% [({]")) {
2480+
else if (initList && indentlevel == 0 && Token::Match(tok->previous(), "[,:] %name% [({]")) {
24932481
const std::map<std::string, unsigned int>::const_iterator it = variableId.find(tok->str());
24942482
if (it != variableId.end()) {
24952483
tok->varId(it->second);
@@ -2511,8 +2499,8 @@ static void setVarIdClassDeclaration(Token * const startToken,
25112499
setVarIdStructMembers(&tok, structMembers, _varId);
25122500
}
25132501
}
2514-
} else if (indentlevel == 0 && tok->str() == ":" && !initListEndToken)
2515-
initListEndToken = findInitListEndToken(tok->previous());
2502+
} else if (indentlevel == 0 && tok->str() == ":" && !initListArgLastToken)
2503+
initList = true;
25162504
}
25172505
}
25182506

@@ -2572,86 +2560,78 @@ void Tokenizer::setVarId()
25722560
std::map<std::string, unsigned int> variableId;
25732561
std::map<unsigned int, std::map<std::string, unsigned int> > structMembers;
25742562
std::stack< std::map<std::string, unsigned int> > scopeInfo;
2575-
std::stack<bool> executableScope;
2576-
executableScope.push(false);
2577-
std::stack<unsigned int> scopestartvarid; // varid when scope starts
2578-
scopestartvarid.push(0);
2579-
const Token * initListEndToken = nullptr;
2580-
for (Token *tok = list.front(); tok; tok = tok->next()) {
25812563

2582-
// scope info to handle shadow variables..
2583-
bool newScope = false;
2584-
if (!initListEndToken && tok->str() == "(") {
2585-
if (Token::Match(tok->tokAt(-2), ")|const throw (") && Token::simpleMatch(tok->link(), ") {")) {
2586-
tok = tok->link();
2587-
continue;
2588-
}
2589-
if (isFunctionHead(tok, "{"))
2590-
newScope = true;
2564+
std::stack<scopeStackEntryType> scopeStack;
2565+
2566+
scopeStack.push(scopeStackEntryType());
2567+
const Token * functionDeclEnd = nullptr;
2568+
bool initlist = false;
2569+
for (Token *tok = list.front(); tok; tok = tok->next()) {
2570+
if (tok == functionDeclEnd) {
2571+
functionDeclEnd = nullptr;
2572+
if (tok->str() == ":")
2573+
initlist = true;
2574+
else if (tok->str() == ";") {
2575+
if (scopeInfo.empty())
2576+
cppcheckError(tok);
2577+
variableId.swap(scopeInfo.top());
2578+
scopeInfo.pop();
2579+
} else if (tok->str() == "{")
2580+
scopeStack.push(scopeStackEntryType(true, _varId));
2581+
} else if (!functionDeclEnd && !initlist && tok->str()=="(") {
2582+
if (!scopeStack.top().isExecutable)
2583+
functionDeclEnd = isFunctionHead(tok, "{:;");
25912584
else {
2592-
initListEndToken = findInitListEndToken(tok->link());
2593-
if (initListEndToken)
2594-
newScope = true;
2585+
Token const * tokenLinkNext = tok->link()->next();
2586+
if (tokenLinkNext->str() == "{") // might be for- or while-loop or if-statement
2587+
functionDeclEnd = tokenLinkNext;
25952588
}
2596-
}
2597-
if (newScope) {
2598-
scopeInfo.push(variableId);
2599-
2600-
// function declarations
2601-
} else if (!executableScope.top() && tok->str() == "(" && isFunctionHead(tok, ";")) {
2602-
scopeInfo.push(variableId);
2603-
} else if (!executableScope.top() && tok->str() == ")" && isFunctionHead(tok, ";")) {
2604-
if (scopeInfo.empty())
2605-
cppcheckError(tok);
2606-
variableId.swap(scopeInfo.top());
2607-
scopeInfo.pop();
2589+
if (functionDeclEnd)
2590+
scopeInfo.push(variableId);
26082591
} else if (tok->str() == "{") {
26092592
// parse anonymous unions as part of the current scope
2610-
if (!(tok->strAt(-1) == "union" && Token::simpleMatch(tok->link(), "} ;"))) {
2611-
scopestartvarid.push(_varId);
2593+
if (!(tok->strAt(-1) == "union" && Token::simpleMatch(tok->link(), "} ;")) &&
2594+
!(initlist && Token::Match(tok->previous(), "%name%|>|>>") && Token::Match(tok->link(), "} ,|{"))) {
2595+
bool isExecutable;
26122596
if (tok->strAt(-1) == ")" || Token::Match(tok->tokAt(-2), ") %type%") ||
2613-
tok == initListEndToken) {
2614-
executableScope.push(true);
2597+
(initlist && tok->strAt(-1) == "}")) {
2598+
isExecutable = true;
26152599
} else {
2616-
executableScope.push(tok->strAt(-1) == "else");
2600+
isExecutable = (initlist || tok->strAt(-1) == "else");
26172601
scopeInfo.push(variableId);
26182602
}
2603+
initlist = false;
2604+
scopeStack.push(scopeStackEntryType(isExecutable, _varId));
26192605
}
2620-
if (tok == initListEndToken)
2621-
initListEndToken= nullptr;
26222606
} else if (tok->str() == "}") {
26232607
// parse anonymous unions as part of the current scope
2624-
if (!(Token::simpleMatch(tok, "} ;") && tok->link() && Token::simpleMatch(tok->link()->previous(), "union {"))) {
2608+
if (!(Token::simpleMatch(tok, "} ;") && tok->link() && Token::simpleMatch(tok->link()->previous(), "union {")) &&
2609+
!(initlist && Token::Match(tok, "} ,|{") && Token::Match(tok->link()->previous(), "%name%|>|>> {"))) {
26252610
// Set variable ids in class declaration..
2626-
if (!initListEndToken && !isC() && !executableScope.top() && tok->link()) {
2611+
if (!initlist && !isC() && !scopeStack.top().isExecutable && tok->link()) {
26272612
setVarIdClassDeclaration(tok->link(),
26282613
variableId,
2629-
scopestartvarid.top(),
2614+
scopeStack.top().startVarid,
26302615
&structMembers,
26312616
&_varId);
26322617
}
26332618

2634-
scopestartvarid.pop();
2635-
if (scopestartvarid.empty()) { // should be impossible
2636-
scopestartvarid.push(0);
2637-
}
2638-
26392619
if (scopeInfo.empty()) {
26402620
variableId.clear();
26412621
} else {
26422622
variableId.swap(scopeInfo.top());
26432623
scopeInfo.pop();
26442624
}
26452625

2646-
executableScope.pop();
2647-
if (executableScope.empty()) { // should not possibly happen
2648-
executableScope.push(false);
2626+
scopeStack.pop();
2627+
if (scopeStack.empty()) { // should be impossible
2628+
scopeStack.push(scopeStackEntryType());
26492629
}
26502630
}
26512631
}
26522632

26532633
if (tok == list.front() || Token::Match(tok, "[;{}]") ||
2654-
(Token::Match(tok, "[(,]") && (!executableScope.top() || Token::simpleMatch(tok->link(), ") {"))) ||
2634+
(Token::Match(tok, "[(,]") && (!scopeStack.top().isExecutable || Token::simpleMatch(tok->link(), ") {"))) ||
26552635
(tok->isName() && tok->str().at(tok->str().length()-1U) == ':')) {
26562636

26572637
// No variable declarations in sizeof
@@ -2679,7 +2659,7 @@ void Tokenizer::setVarId()
26792659
if (!isC() && Token::simpleMatch(tok2, "const new"))
26802660
continue;
26812661

2682-
bool decl = setVarIdParseDeclaration(&tok2, variableId, executableScope.top(), isCPP(), isC());
2662+
bool decl = setVarIdParseDeclaration(&tok2, variableId, scopeStack.top().isExecutable, isCPP(), isC());
26832663
if (decl) {
26842664
const Token* prev2 = tok2->previous();
26852665
if (Token::Match(prev2, "%type% [;[=,)]") && tok2->previous()->str() != "const")
@@ -2695,7 +2675,7 @@ void Tokenizer::setVarId()
26952675

26962676
const Token *tok3 = tok2->next();
26972677
if (!tok3->isStandardType() && tok3->str() != "void" && !Token::Match(tok3, "struct|union|class %type%") && tok3->str() != "." && !Token::Match(tok2->link()->previous(), "[&*]")) {
2698-
if (!executableScope.top()) {
2678+
if (!scopeStack.top().isExecutable) {
26992679
// Detecting initializations with () in non-executable scope is hard and often impossible to be done safely. Thus, only treat code as a variable that definitly is one.
27002680
decl = false;
27012681
bool rhs = false;

0 commit comments

Comments
 (0)