Skip to content

Commit 457a0cf

Browse files
chrchr-githubchrchr-github
authored
Fix #11016 FP unusedStructMember when used through iterator (regression) (#4289)
* Format * Fix #11016 FP unusedStructMember when used through iterator (regression) * Format * Fix test * Format * Nullptr check
1 parent f644938 commit 457a0cf

5 files changed

Lines changed: 59 additions & 27 deletions

File tree

‎lib/symboldatabase.cpp‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,14 +1205,23 @@ void SymbolDatabase::createSymbolDatabaseSetVariablePointers()
12051205
{
12061206
VarIdMap varIds;
12071207

1208+
auto setMemberVar = [&](const Variable* membervar, Token* membertok, const Token* vartok) -> void {
1209+
if (membervar) {
1210+
membertok->variable(membervar);
1211+
if (vartok && (membertok->varId() == 0 || mVariableList[membertok->varId()] == nullptr))
1212+
fixVarId(varIds, vartok, membertok, membervar);
1213+
}
1214+
};
1215+
12081216
// Set variable pointers
12091217
for (const Token* tok = mTokenizer->list.front(); tok != mTokenizer->list.back(); tok = tok->next()) {
12101218
if (tok->varId())
1211-
const_cast<Token *>(tok)->variable(getVariableFromVarId(tok->varId()));
1219+
const_cast<Token*>(tok)->variable(getVariableFromVarId(tok->varId()));
12121220

12131221
// Set Token::variable pointer for array member variable
12141222
// Since it doesn't point at a fixed location it doesn't have varid
1215-
const bool isVar = tok->variable() && (tok->variable()->typeScope() || tok->variable()->isSmartPointer() || (tok->valueType() && tok->valueType()->type == ValueType::CONTAINER));
1223+
const bool isVar = tok->variable() && (tok->variable()->typeScope() || tok->variable()->isSmartPointer() ||
1224+
(tok->valueType() && (tok->valueType()->type == ValueType::CONTAINER || tok->valueType()->type == ValueType::ITERATOR)));
12161225
const bool isArrayAccess = isVar && Token::simpleMatch(tok->astParent(), "[");
12171226
const bool isDirectAccess = isVar && !isArrayAccess && Token::simpleMatch(tok->astParent(), ".");
12181227
const bool isDerefAccess = isVar && !isDirectAccess && Token::simpleMatch(tok->astParent(), "*") && Token::simpleMatch(tok->astParent()->astParent(), ".");
@@ -1247,19 +1256,11 @@ void SymbolDatabase::createSymbolDatabaseSetVariablePointers()
12471256
const Variable *var = tok->variable();
12481257
if (var->typeScope()) {
12491258
const Variable *membervar = var->typeScope()->getVariable(membertok->str());
1250-
if (membervar) {
1251-
membertok->variable(membervar);
1252-
if (membertok->varId() == 0 || mVariableList[membertok->varId()] == nullptr)
1253-
fixVarId(varIds, tok, const_cast<Token *>(membertok), membervar);
1254-
}
1259+
setMemberVar(membervar, membertok, tok);
12551260
} else if (const ::Type *type = var->smartPointerType()) {
12561261
const Scope *classScope = type->classScope;
12571262
const Variable *membervar = classScope ? classScope->getVariable(membertok->str()) : nullptr;
1258-
if (membervar) {
1259-
membertok->variable(membervar);
1260-
if (membertok->varId() == 0 || mVariableList[membertok->varId()] == nullptr)
1261-
fixVarId(varIds, tok, const_cast<Token *>(membertok), membervar);
1262-
}
1263+
setMemberVar(membervar, membertok, tok);
12631264
} else if (tok->valueType() && tok->valueType()->type == ValueType::CONTAINER) {
12641265
if (Token::Match(var->typeStartToken(), "std :: %type% < %name%")) {
12651266
const Token* type2tok = var->typeStartToken()->tokAt(4);
@@ -1268,13 +1269,14 @@ void SymbolDatabase::createSymbolDatabaseSetVariablePointers()
12681269
const Type* type2 = type2tok ? type2tok->type() : nullptr;
12691270
if (type2 && type2->classScope && type2->classScope->definedType) {
12701271
const Variable *membervar = type2->classScope->getVariable(membertok->str());
1271-
if (membervar) {
1272-
membertok->variable(membervar);
1273-
if (membertok->varId() == 0 || mVariableList[membertok->varId()] == nullptr)
1274-
fixVarId(varIds, tok, const_cast<Token *>(membertok), membervar);
1275-
}
1272+
setMemberVar(membervar, membertok, tok);
12761273
}
12771274
}
1275+
} else if (const Type* iterType = var->iteratorType()) {
1276+
if (iterType->classScope && iterType->classScope->definedType) {
1277+
const Variable *membervar = iterType->classScope->getVariable(membertok->str());
1278+
setMemberVar(membervar, membertok, tok);
1279+
}
12781280
}
12791281
}
12801282
}
@@ -1296,13 +1298,7 @@ void SymbolDatabase::createSymbolDatabaseSetVariablePointers()
12961298
if (!membervar) {
12971299
if (type->classScope) {
12981300
membervar = type->classScope->getVariable(membertok->str());
1299-
if (membervar) {
1300-
membertok->variable(membervar);
1301-
if (membertok->varId() == 0 || mVariableList[membertok->varId()] == nullptr) {
1302-
if (tok->function()->retDef)
1303-
fixVarId(varIds, tok->function()->retDef, const_cast<Token *>(membertok), membervar);
1304-
}
1305-
}
1301+
setMemberVar(membervar, membertok, tok->function()->retDef);
13061302
}
13071303
}
13081304
}
@@ -2262,6 +2258,17 @@ const Type *Variable::smartPointerType() const
22622258
return nullptr;
22632259
}
22642260

2261+
const Type* Variable::iteratorType() const
2262+
{
2263+
if (!mValueType || mValueType->type != ValueType::ITERATOR)
2264+
return nullptr;
2265+
2266+
if (mValueType->containerTypeToken)
2267+
return mValueType->containerTypeToken->type();
2268+
2269+
return nullptr;
2270+
}
2271+
22652272
std::string Variable::getTypeName() const
22662273
{
22672274
std::string ret;

‎lib/symboldatabase.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,8 @@ class CPPCHECKLIB Variable {
593593
return getFlag(fIsSmartPointer);
594594
}
595595

596-
const Type *smartPointerType() const;
596+
const Type* smartPointerType() const;
597+
const Type* iteratorType() const;
597598

598599
/**
599600
* Checks if the variable is of any of the STL types passed as arguments ('std::')

‎test/testautovariables.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,7 +1556,7 @@ class TestAutoVariables : public TestFixture {
15561556
void returnReference25()
15571557
{
15581558
check("int& f();\n" // #10983
1559-
" auto g() -> decltype(f()) {\n"
1559+
"auto g() -> decltype(f()) {\n"
15601560
" return f();\n"
15611561
"}\n"
15621562
"int& h() {\n"
@@ -2252,7 +2252,7 @@ class TestAutoVariables : public TestFixture {
22522252
" return &it->foo;\n"
22532253
"}");
22542254
ASSERT_EQUALS(
2255-
"[test.cpp:3] -> [test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning object that points to local variable 'v' that will be invalid when returning.\n",
2255+
"[test.cpp:3] -> [test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning pointer to local variable 'v' that will be invalid when returning.\n",
22562256
errout.str());
22572257

22582258
check("auto f(std::vector<int> x) {\n"

‎test/testsymboldatabase.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ class TestSymbolDatabase : public TestFixture {
361361
TEST_CASE(symboldatabase98); // #10451
362362
TEST_CASE(symboldatabase99); // #10864
363363
TEST_CASE(symboldatabase100); // #10174
364+
TEST_CASE(symboldatabase101);
364365

365366
TEST_CASE(createSymbolDatabaseFindAllScopes1);
366367
TEST_CASE(createSymbolDatabaseFindAllScopes2);
@@ -4974,6 +4975,19 @@ class TestSymbolDatabase : public TestFixture {
49744975
}
49754976
}
49764977

4978+
void symboldatabase101() {
4979+
GET_SYMBOL_DB("struct A { bool b; };\n"
4980+
"void f(const std::vector<A>& v) {\n"
4981+
" std::vector<A>::const_iterator it = b.begin();\n"
4982+
" if (it->b) {}\n"
4983+
"}\n");
4984+
ASSERT(db);
4985+
const Token* it = Token::findsimplematch(tokenizer.tokens(), "it . b");
4986+
ASSERT(it);
4987+
ASSERT(it->tokAt(2));
4988+
ASSERT(it->tokAt(2)->variable());
4989+
}
4990+
49774991
void createSymbolDatabaseFindAllScopes1() {
49784992
GET_SYMBOL_DB("void f() { union {int x; char *p;} a={0}; }");
49794993
ASSERT(db->scopeList.size() == 3);

‎test/testunusedvar.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ class TestUnusedVar : public TestFixture {
6969
TEST_CASE(structmember19); // #10826, #10848, #10852
7070
TEST_CASE(structmember20); // #10737
7171
TEST_CASE(structmember21); // #4759
72+
TEST_CASE(structmember22); // #11016
7273

7374
TEST_CASE(localvar1);
7475
TEST_CASE(localvar2);
@@ -1796,6 +1797,15 @@ class TestUnusedVar : public TestFixture {
17961797
errout.str());
17971798
}
17981799

1800+
void structmember22() { // #11016
1801+
checkStructMemberUsage("struct A { bool b; };\n"
1802+
"void f(const std::vector<A>& v) {\n"
1803+
" std::vector<A>::const_iterator it = b.begin();\n"
1804+
" if (it->b) {}\n"
1805+
"}\n");
1806+
ASSERT_EQUALS("", errout.str());
1807+
}
1808+
17991809
void functionVariableUsage_(const char* file, int line, const char code[], const char filename[] = "test.cpp") {
18001810
// Clear the error buffer..
18011811
errout.str("");

0 commit comments

Comments
 (0)