Fixed #6253 ([False Positive] Variable not initialized in the constructor)

This commit is contained in:
Frank Zingsheim 2015-04-06 19:47:21 +02:00 committed by Daniel Marjamäki
parent bacc5ac1cc
commit 1f5265c1bd
6 changed files with 200 additions and 129 deletions

View File

@ -485,7 +485,7 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
// Class constructor.. initializing variables like this
// clKalle::clKalle() : var(value) { }
if (initList) {
if (level == 0 && Token::Match(ftok, "%name% {|(")) {
if (level == 0 && Token::Match(ftok, "%name% {|(") && Token::Match(ftok->linkAt(1), "}|) ,|{")) {
if (ftok->str() != func.name()) {
initVar(ftok->str(), scope, usage);
} else { // c++11 delegate constructor
@ -514,21 +514,20 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
}
}
}
ftok = ftok->next();
level++;
} else if (level != 0 && Token::Match(ftok, "%name% =")) // assignment in the initializer: var(value = x)
assignVar(ftok->str(), scope, usage);
else if (ftok->str() == "(")
// Level handling
if (ftok->link() && Token::Match(ftok, "(|<"))
level++;
else if (ftok->str() == ")" || ftok->str() == "}")
level--;
else if (ftok->str() == "{") {
if (level == 0)
initList = false;
if (level != 0 ||
(Token::Match(ftok->previous(), "%name%|>") && Token::Match(ftok->link(), "} ,|{")))
level++;
else
ftok = ftok->link();
}
initList = false;
} else if (ftok->link() && Token::Match(ftok, ")|>|}"))
level--;
}
if (initList)

View File

@ -572,8 +572,21 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
}
// find start of function '{'
while (end && end->str() != "{" && end->str() != ";")
end = end->next();
bool foundInitList = false;
while (end && end->str() != "{" && end->str() != ";") {
if (end->link() && Token::Match(end, "(|<")) {
end = end->link();
} else if (foundInitList &&
Token::Match(end, "%name%|> {") &&
Token::Match(end->linkAt(1), "} ,|{")) {
end = end->linkAt(1);
} else {
if (end->str() == ":")
foundInitList = true;
end = end->next();
}
}
if (!end || end->str() == ";")
continue;
@ -1843,17 +1856,23 @@ void SymbolDatabase::addNewFunction(Scope **scope, const Token **tok)
scopeList.push_back(Scope(this, tok1, *scope));
Scope *new_scope = &scopeList.back();
// skip to start of function
bool foundInitLit = false;
while (tok1 && (tok1->str() != "{" || (foundInitLit && tok1->previous()->isName()))) {
if (Token::Match(tok1, "(|{"))
// find start of function '{'
bool foundInitList = false;
while (tok1 && tok1->str() != "{" && tok1->str() != ";") {
if (tok1->link() && Token::Match(tok1, "(|<")) {
tok1 = tok1->link();
if (tok1->str() == ":")
foundInitLit = true;
tok1 = tok1->next();
} else if (foundInitList &&
Token::Match(tok1, "%name%|> {") &&
Token::Match(tok1->linkAt(1), "} ,|{")) {
tok1 = tok1->linkAt(1);
} else {
if (tok1->str() == ":")
foundInitList = true;
tok1 = tok1->next();
}
}
if (tok1) {
if (tok1 && tok1->str() == "{") {
new_scope->classStart = tok1;
new_scope->classEnd = tok1->link();

View File

@ -36,21 +36,37 @@
//---------------------------------------------------------------------------
namespace {
// local struct used in setVarId
// in order to store information about the scope
struct scopeStackEntryType {
scopeStackEntryType()
:isExecutable(false), startVarid(0) {
}
scopeStackEntryType(bool _isExecutable, unsigned int _startVarid)
:isExecutable(_isExecutable), startVarid(_startVarid) {
}
const bool isExecutable;
const unsigned int startVarid;
};
}
/**
* is token pointing at function head?
* @param tok A '(' or ')' token in a possible function head
* @param endsWith string after function head
* @return true if syntax seems to be a function head
* @return token matching with endsWith if syntax seems to be a function head else nullptr
*/
static bool isFunctionHead(const Token *tok, const std::string &endsWith)
static const Token * isFunctionHead(const Token *tok, const std::string &endsWith)
{
if (tok->str() == "(")
tok = tok->link();
if (Token::Match(tok, ") const| [;{]")) {
if (Token::Match(tok, ") const| [;:{]")) {
tok = tok->next();
if (tok->isName())
tok = tok->next();
return endsWith.find(tok->str()) != std::string::npos;
return (endsWith.find(tok->str()) != std::string::npos) ? tok : nullptr;
}
if (Token::Match(tok, ") const| throw (")) {
tok = tok->next();
@ -59,9 +75,9 @@ static bool isFunctionHead(const Token *tok, const std::string &endsWith)
tok = tok->link()->next();
while (tok && tok->isName())
tok = tok->next();
return tok && endsWith.find(tok->str()) != std::string::npos;
return (tok && endsWith.find(tok->str()) != std::string::npos) ? tok : nullptr;
}
return false;
return nullptr;
}
//---------------------------------------------------------------------------
@ -2422,43 +2438,6 @@ static void setVarIdStructMembers(Token **tok1,
}
static const Token * findInitListEndToken(const Token *tok)
{
if (!Token::Match(tok, ") noexcept|:") && !Token::simpleMatch(tok, "noexcept :"))
return nullptr;
if (tok->strAt(1) != ":") {
tok = tok->next();
if (tok->strAt(1) == "(")
tok = tok->linkAt(1);
}
tok = tok->tokAt(2);
while (tok) {
if (tok && tok->str()=="::")
tok = tok->next();
while (tok && Token::Match(tok, "%type% ::"))
tok = tok->tokAt(2);
if (Token::Match(tok, "%name% [({]")) {
tok = tok->linkAt(1);
if (!tok)
return nullptr;
tok = tok->next();
}
if (tok && tok->str()==",")
tok = tok->next();
else if (tok && tok->str()=="{")
return tok; // End of init list found
else
return nullptr;
}
return nullptr;
}
static void setVarIdClassDeclaration(Token * const startToken,
const std::map<std::string, unsigned int> &variableId,
const unsigned int scopeStartVarId,
@ -2481,15 +2460,24 @@ static void setVarIdClassDeclaration(Token * const startToken,
// replace varids..
unsigned int indentlevel = 0;
const Token * initListEndToken = nullptr;
bool initList = false;
const Token *initListArgLastToken = nullptr;
for (Token *tok = startToken->next(); tok != endToken; tok = tok->next()) {
if (initList) {
if (tok == initListArgLastToken)
initListArgLastToken = nullptr;
else if (!initListArgLastToken &&
Token::Match(tok->previous(), "%name%|>|>> {|(") &&
Token::Match(tok->link(), "}|) ,|{"))
initListArgLastToken = tok->link();
}
if (tok->str() == "{") {
if (tok == initListEndToken)
initListEndToken = nullptr;
if (initList && !initListArgLastToken)
initList = false;
++indentlevel;
} else if (tok->str() == "}")
--indentlevel;
else if (initListEndToken && indentlevel == 0 && Token::Match(tok->previous(), "[,:] %name% [({]")) {
else if (initList && indentlevel == 0 && Token::Match(tok->previous(), "[,:] %name% [({]")) {
const std::map<std::string, unsigned int>::const_iterator it = variableId.find(tok->str());
if (it != variableId.end()) {
tok->varId(it->second);
@ -2511,8 +2499,8 @@ static void setVarIdClassDeclaration(Token * const startToken,
setVarIdStructMembers(&tok, structMembers, _varId);
}
}
} else if (indentlevel == 0 && tok->str() == ":" && !initListEndToken)
initListEndToken = findInitListEndToken(tok->previous());
} else if (indentlevel == 0 && tok->str() == ":" && !initListArgLastToken)
initList = true;
}
}
@ -2572,70 +2560,62 @@ void Tokenizer::setVarId()
std::map<std::string, unsigned int> variableId;
std::map<unsigned int, std::map<std::string, unsigned int> > structMembers;
std::stack< std::map<std::string, unsigned int> > scopeInfo;
std::stack<bool> executableScope;
executableScope.push(false);
std::stack<unsigned int> scopestartvarid; // varid when scope starts
scopestartvarid.push(0);
const Token * initListEndToken = nullptr;
std::stack<scopeStackEntryType> scopeStack;
scopeStack.push(scopeStackEntryType());
const Token * functionDeclEnd = nullptr;
bool initlist = false;
for (Token *tok = list.front(); tok; tok = tok->next()) {
// scope info to handle shadow variables..
bool newScope = false;
if (!initListEndToken && tok->str() == "(") {
if (Token::Match(tok->tokAt(-2), ")|const throw (") && Token::simpleMatch(tok->link(), ") {")) {
tok = tok->link();
continue;
}
if (isFunctionHead(tok, "{"))
newScope = true;
if (tok == functionDeclEnd) {
functionDeclEnd = nullptr;
if (tok->str() == ":")
initlist = true;
else if (tok->str() == ";") {
if (scopeInfo.empty())
cppcheckError(tok);
variableId.swap(scopeInfo.top());
scopeInfo.pop();
} else if (tok->str() == "{")
scopeStack.push(scopeStackEntryType(true, _varId));
} else if (!functionDeclEnd && !initlist && tok->str()=="(") {
if (!scopeStack.top().isExecutable)
functionDeclEnd = isFunctionHead(tok, "{:;");
else {
initListEndToken = findInitListEndToken(tok->link());
if (initListEndToken)
newScope = true;
Token const * tokenLinkNext = tok->link()->next();
if (tokenLinkNext->str() == "{") // might be for- or while-loop or if-statement
functionDeclEnd = tokenLinkNext;
}
}
if (newScope) {
scopeInfo.push(variableId);
// function declarations
} else if (!executableScope.top() && tok->str() == "(" && isFunctionHead(tok, ";")) {
scopeInfo.push(variableId);
} else if (!executableScope.top() && tok->str() == ")" && isFunctionHead(tok, ";")) {
if (scopeInfo.empty())
cppcheckError(tok);
variableId.swap(scopeInfo.top());
scopeInfo.pop();
if (functionDeclEnd)
scopeInfo.push(variableId);
} else if (tok->str() == "{") {
// parse anonymous unions as part of the current scope
if (!(tok->strAt(-1) == "union" && Token::simpleMatch(tok->link(), "} ;"))) {
scopestartvarid.push(_varId);
if (!(tok->strAt(-1) == "union" && Token::simpleMatch(tok->link(), "} ;")) &&
!(initlist && Token::Match(tok->previous(), "%name%|>|>>") && Token::Match(tok->link(), "} ,|{"))) {
bool isExecutable;
if (tok->strAt(-1) == ")" || Token::Match(tok->tokAt(-2), ") %type%") ||
tok == initListEndToken) {
executableScope.push(true);
(initlist && tok->strAt(-1) == "}")) {
isExecutable = true;
} else {
executableScope.push(tok->strAt(-1) == "else");
isExecutable = (initlist || tok->strAt(-1) == "else");
scopeInfo.push(variableId);
}
initlist = false;
scopeStack.push(scopeStackEntryType(isExecutable, _varId));
}
if (tok == initListEndToken)
initListEndToken= nullptr;
} else if (tok->str() == "}") {
// parse anonymous unions as part of the current scope
if (!(Token::simpleMatch(tok, "} ;") && tok->link() && Token::simpleMatch(tok->link()->previous(), "union {"))) {
if (!(Token::simpleMatch(tok, "} ;") && tok->link() && Token::simpleMatch(tok->link()->previous(), "union {")) &&
!(initlist && Token::Match(tok, "} ,|{") && Token::Match(tok->link()->previous(), "%name%|>|>> {"))) {
// Set variable ids in class declaration..
if (!initListEndToken && !isC() && !executableScope.top() && tok->link()) {
if (!initlist && !isC() && !scopeStack.top().isExecutable && tok->link()) {
setVarIdClassDeclaration(tok->link(),
variableId,
scopestartvarid.top(),
scopeStack.top().startVarid,
&structMembers,
&_varId);
}
scopestartvarid.pop();
if (scopestartvarid.empty()) { // should be impossible
scopestartvarid.push(0);
}
if (scopeInfo.empty()) {
variableId.clear();
} else {
@ -2643,15 +2623,15 @@ void Tokenizer::setVarId()
scopeInfo.pop();
}
executableScope.pop();
if (executableScope.empty()) { // should not possibly happen
executableScope.push(false);
scopeStack.pop();
if (scopeStack.empty()) { // should be impossible
scopeStack.push(scopeStackEntryType());
}
}
}
if (tok == list.front() || Token::Match(tok, "[;{}]") ||
(Token::Match(tok, "[(,]") && (!executableScope.top() || Token::simpleMatch(tok->link(), ") {"))) ||
(Token::Match(tok, "[(,]") && (!scopeStack.top().isExecutable || Token::simpleMatch(tok->link(), ") {"))) ||
(tok->isName() && tok->str().at(tok->str().length()-1U) == ':')) {
// No variable declarations in sizeof
@ -2679,7 +2659,7 @@ void Tokenizer::setVarId()
if (!isC() && Token::simpleMatch(tok2, "const new"))
continue;
bool decl = setVarIdParseDeclaration(&tok2, variableId, executableScope.top(), isCPP(), isC());
bool decl = setVarIdParseDeclaration(&tok2, variableId, scopeStack.top().isExecutable, isCPP(), isC());
if (decl) {
const Token* prev2 = tok2->previous();
if (Token::Match(prev2, "%type% [;[=,)]") && tok2->previous()->str() != "const")
@ -2695,7 +2675,7 @@ void Tokenizer::setVarId()
const Token *tok3 = tok2->next();
if (!tok3->isStandardType() && tok3->str() != "void" && !Token::Match(tok3, "struct|union|class %type%") && tok3->str() != "." && !Token::Match(tok2->link()->previous(), "[&*]")) {
if (!executableScope.top()) {
if (!scopeStack.top().isExecutable) {
// 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.
decl = false;
bool rhs = false;

View File

@ -63,6 +63,7 @@ private:
TEST_CASE(simple11); // ticket #4536, #6214
TEST_CASE(simple12); // ticket #4620
TEST_CASE(simple13); // #5498 - no constructor, c++11 assignments
TEST_CASE(simple14); // #6253 template base
TEST_CASE(noConstructor1);
TEST_CASE(noConstructor2);
@ -423,6 +424,30 @@ private:
ASSERT_EQUALS("", errout.str());
}
void simple14() { // #6253 template base
check("class Fred : public Base<A, B> {"
"public:"
" Fred()\n"
" :Base<A, B>(1),\n"
" x(1)\n"
" {}\n"
"private:\n"
" int x;\n"
"};\n");
ASSERT_EQUALS("", errout.str());
check("class Fred : public Base<A, B> {"
"public:"
" Fred()\n"
" :Base<A, B>{1},\n"
" x{1}\n"
" {}\n"
"private:\n"
" int x;\n"
"};\n");
ASSERT_EQUALS("", errout.str());
}
void noConstructor1() {
// There are nonstatic member variables - constructor is needed
check("class Fred\n"

View File

@ -377,10 +377,10 @@ private:
void garbageCode28() {
// 5702
ASSERT_THROW(checkCode("struct R1 {\n"
" int a;\n"
" R1 () : a { }\n"
"};\n"), InternalError);
checkCode("struct R1 {\n"
" int a;\n"
" R1 () : a { }\n"
"};\n");
}
void garbageCode29() {

View File

@ -115,6 +115,7 @@ private:
TEST_CASE(varid_in_class16);
TEST_CASE(varid_in_class17); // #6056 - no varid for member functions
TEST_CASE(varid_initList);
TEST_CASE(varid_initListWithBaseTemplate);
TEST_CASE(varid_operator);
TEST_CASE(varid_throw);
TEST_CASE(varid_unknown_macro); // #2638 - unknown macro is not type
@ -1688,6 +1689,53 @@ private:
tokenize(code7));
}
void varid_initListWithBaseTemplate() {
const char code1[] = "class A : B<C,D> {\n"
" A() : B<C,D>(), x(0) {}\n"
" int x;\n"
"};";
ASSERT_EQUALS("\n\n##file 0\n"
"1: class A : B < C , D > {\n"
"2: A ( ) : B < C , D > ( ) , x@1 ( 0 ) { }\n"
"3: int x@1 ;\n"
"4: } ;\n",
tokenize(code1));
const char code2[] = "class A : B<C,D> {\n"
" A(int x) : x(x) {}\n"
" int x;\n"
"};";
ASSERT_EQUALS("\n\n##file 0\n1: class A : B < C , D > {\n"
"2: A ( int x@1 ) : x@2 ( x@1 ) { }\n"
"3: int x@2 ;\n"
"4: } ;\n",
tokenize(code2));
const char code3[] = "class A : B<C,D> {\n"
" A(int x);\n"
" int x;\n"
"};\n"
"A::A(int x) : x(x) {}";
ASSERT_EQUALS("\n\n##file 0\n"
"1: class A : B < C , D > {\n"
"2: A ( int x@1 ) ;\n"
"3: int x@2 ;\n"
"4: } ;\n"
"5: A :: A ( int x@3 ) : x@2 ( x@3 ) { }\n",
tokenize(code3));
const char code4[] = "struct A : B<C,D> {\n"
" int x;\n"
" A(int x) : x(x) {}\n"
"};\n";
ASSERT_EQUALS("\n\n##file 0\n"
"1: struct A : B < C , D > {\n"
"2: int x@1 ;\n"
"3: A ( int x@2 ) : x@1 ( x@2 ) { }\n"
"4: } ;\n",
tokenize(code4));
}
void varid_operator() {
{
const std::string actual = tokenize(
@ -1923,17 +1971,17 @@ private:
"1: class Scope { } ;\n",
tokenize("class CPPCHECKLIB Scope { };"));
// #6073
// #6073 #6253
ASSERT_EQUALS("\n\n##file 0\n"
"1: class A : public B , public C :: D {\n"
"1: class A : public B , public C :: D , public E < F > :: G < H > {\n"
"2: int i@1 ;\n"
"3: A ( int i@2 ) : B { i@2 } , C :: D { i@2 } , i@1 { i@2 } {\n"
"3: A ( int i@2 ) : B { i@2 } , C :: D { i@2 } , E < F > :: G < H > { i@2 } , i@1 { i@2 } {\n"
"4: int j@3 { i@2 } ;\n"
"5: }\n"
"6: } ;\n",
tokenize("class A: public B, public C::D {\n"
tokenize("class A: public B, public C::D, public E<F>::G<H> {\n"
" int i;\n"
" A(int i): B{i}, C::D{i}, i{i} {\n"
" A(int i): B{i}, C::D{i}, E<F>::G<H>{i} ,i{i} {\n"
" int j{i};\n"
" }\n"
"};"));