Skip to content

Commit aebdb37

Browse files
zingsheimzingsheim
committed
Fixed cppcheck-opensource#4369 (false positive: Variable 'i' is assigned a value that is never used)
1 parent 590704a commit aebdb37

3 files changed

Lines changed: 179 additions & 37 deletions

File tree

‎lib/checkunusedvar.cpp‎

Lines changed: 100 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ class Variables {
3535
public:
3636
enum VariableType { standard, array, pointer, reference, pointerArray, referenceArray, pointerPointer, none };
3737

38+
typedef std::set<unsigned int> VariableSet;
39+
typedef std::list<VariableSet> VariableListOfSets;
40+
3841
/** Store information about variable usage */
3942
class VariableUsage {
4043
public:
@@ -54,7 +57,8 @@ class Variables {
5457
}
5558

5659
/** variable is used.. set both read+write */
57-
void use() {
60+
void use(VariableListOfSets & varReadInScope) {
61+
varReadInScope.back().insert(_var->varId());
5862
_read = true;
5963
_write = true;
6064
}
@@ -78,6 +82,27 @@ class Variables {
7882

7983
typedef std::map<unsigned int, VariableUsage> VariableMap;
8084

85+
class ScopeGuard
86+
{
87+
public:
88+
ScopeGuard(Variables & guarded,
89+
bool insideLoop)
90+
:_guarded(guarded),
91+
_insideLoop(insideLoop)
92+
{
93+
_guarded.enterScope();
94+
}
95+
96+
~ScopeGuard()
97+
{
98+
_guarded.leaveScope(_insideLoop);
99+
}
100+
101+
private:
102+
Variables & _guarded;
103+
bool _insideLoop;
104+
};
105+
81106
void clear() {
82107
_varUsage.clear();
83108
}
@@ -103,8 +128,17 @@ class Variables {
103128
void eraseAll(unsigned int varid);
104129
void clearAliases(unsigned int varid);
105130

131+
ScopeGuard newScope(bool insideLoop) {
132+
return ScopeGuard(*this, insideLoop);
133+
}
134+
106135
private:
136+
void enterScope();
137+
void leaveScope(bool insideLoop);
138+
107139
VariableMap _varUsage;
140+
VariableListOfSets _varAddedInScope;
141+
VariableListOfSets _varReadInScope;
108142
};
109143

110144

@@ -123,7 +157,7 @@ void Variables::alias(unsigned int varid1, unsigned int varid2, bool replace)
123157
// alias to self
124158
if (varid1 == varid2) {
125159
if (var1)
126-
var1->use();
160+
var1->use(_varReadInScope);
127161
return;
128162
}
129163

@@ -150,8 +184,10 @@ void Variables::alias(unsigned int varid1, unsigned int varid2, bool replace)
150184
var2->_aliases.insert(varid1);
151185
var1->_aliases.insert(varid2);
152186

153-
if (var2->_type == Variables::pointer)
187+
if (var2->_type == Variables::pointer) {
188+
_varReadInScope.back().insert(varid2);
154189
var2->_read = true;
190+
}
155191
}
156192

157193
void Variables::clearAliases(unsigned int varid)
@@ -196,8 +232,10 @@ void Variables::addVar(const Variable *var,
196232
VariableType type,
197233
bool write_)
198234
{
199-
if (var->varId() > 0)
235+
if (var->varId() > 0) {
236+
_varAddedInScope.back().insert(var->varId());
200237
_varUsage.insert(std::make_pair(var->varId(), VariableUsage(var, type, false, write_, false)));
238+
}
201239
}
202240

203241
void Variables::allocateMemory(unsigned int varid, const Token* tok)
@@ -215,8 +253,10 @@ void Variables::read(unsigned int varid, const Token* tok)
215253
VariableUsage *usage = find(varid);
216254

217255
if (usage) {
256+
_varReadInScope.back().insert(varid);
218257
usage->_read = true;
219-
usage->_lastAccess = tok;
258+
if (tok)
259+
usage->_lastAccess = tok;
220260
}
221261
}
222262

@@ -231,6 +271,7 @@ void Variables::readAliases(unsigned int varid, const Token* tok)
231271
VariableUsage *aliased = find(*aliases);
232272

233273
if (aliased) {
274+
_varReadInScope.back().insert(*aliases);
234275
aliased->_read = true;
235276
aliased->_lastAccess = tok;
236277
}
@@ -285,7 +326,7 @@ void Variables::use(unsigned int varid, const Token* tok)
285326
VariableUsage *usage = find(varid);
286327

287328
if (usage) {
288-
usage->use();
329+
usage->use(_varReadInScope);
289330
usage->_lastAccess = tok;
290331

291332
std::set<unsigned int>::iterator aliases;
@@ -294,7 +335,7 @@ void Variables::use(unsigned int varid, const Token* tok)
294335
VariableUsage *aliased = find(*aliases);
295336

296337
if (aliased) {
297-
aliased->use();
338+
aliased->use(_varReadInScope);
298339
aliased->_lastAccess = tok;
299340
}
300341
}
@@ -332,6 +373,47 @@ Variables::VariableUsage *Variables::find(unsigned int varid)
332373
return 0;
333374
}
334375

376+
void Variables::enterScope()
377+
{
378+
_varAddedInScope.push_back(VariableSet());
379+
_varReadInScope.push_back(VariableSet());
380+
}
381+
382+
void Variables::leaveScope(bool insideLoop)
383+
{
384+
if (insideLoop) {
385+
// read variables are read again in subsequent run through loop
386+
VariableSet const & currentVarReadInScope = _varReadInScope.back();
387+
for (VariableSet::const_iterator readIter = currentVarReadInScope.begin();
388+
readIter != currentVarReadInScope.end();
389+
++readIter)
390+
{
391+
read(*readIter, NULL);
392+
}
393+
}
394+
395+
VariableListOfSets::reverse_iterator reverseReadIter = _varReadInScope.rbegin();
396+
++reverseReadIter;
397+
if (reverseReadIter != _varReadInScope.rend())
398+
{
399+
// Transfer read variables into previous scope
400+
401+
VariableSet const & currentVarAddedInScope = _varAddedInScope.back();
402+
VariableSet & currentVarReadInScope = _varReadInScope.back();
403+
for (VariableSet::const_iterator addedIter = currentVarAddedInScope.begin();
404+
addedIter != currentVarAddedInScope.end();
405+
++addedIter)
406+
{
407+
currentVarReadInScope.erase(*addedIter);
408+
}
409+
VariableSet & previousVarReadInScope = *reverseReadIter;
410+
previousVarReadInScope.insert(currentVarReadInScope.begin(),
411+
currentVarReadInScope.end());
412+
}
413+
_varReadInScope.pop_back();
414+
_varAddedInScope.pop_back();
415+
}
416+
335417
static const Token* doAssignment(Variables &variables, const Token *tok, bool dereference, const Scope *scope)
336418
{
337419
// a = a + b;
@@ -589,8 +671,10 @@ static const Token * skipBracketsAndMembers(const Token *tok)
589671
//---------------------------------------------------------------------------
590672
// Usage of function variables
591673
//---------------------------------------------------------------------------
592-
void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const scope, Variables& variables, bool insideLoop, std::vector<unsigned int> &usedVariables)
674+
void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const scope, Variables& variables, bool insideLoop)
593675
{
676+
Variables::ScopeGuard scopeGuard=variables.newScope(insideLoop);
677+
594678
// Find declarations if the scope is executable..
595679
if (scope->isExecutable()) {
596680
// Find declarations
@@ -648,12 +732,7 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
648732
if (tok->str() == "for" || tok->str() == "while" || tok->str() == "do") {
649733
for (std::list<Scope*>::const_iterator i = scope->nestedList.begin(); i != scope->nestedList.end(); ++i) {
650734
if ((*i)->classDef == tok) { // Find associated scope
651-
checkFunctionVariableUsage_iterateScopes(*i, variables, true, usedVariables); // Scan child scope
652-
insideLoop = false;
653-
std::vector<unsigned int>::iterator it;
654-
for (it = usedVariables.begin(); it != usedVariables.end(); ++it) {
655-
variables.read((*it), tok);
656-
}
735+
checkFunctionVariableUsage_iterateScopes(*i, variables, true); // Scan child scope
657736
tok = (*i)->classStart->link();
658737
break;
659738
}
@@ -664,7 +743,7 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
664743
if (tok->str() == "{") {
665744
for (std::list<Scope*>::const_iterator i = scope->nestedList.begin(); i != scope->nestedList.end(); ++i) {
666745
if ((*i)->classStart == tok) { // Find associated scope
667-
checkFunctionVariableUsage_iterateScopes(*i, variables, insideLoop, usedVariables); // Scan child scope
746+
checkFunctionVariableUsage_iterateScopes(*i, variables, false); // Scan child scope
668747
tok = tok->link();
669748
break;
670749
}
@@ -827,10 +906,12 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
827906

828907
// checked for chained assignments
829908
if (tok != start && equal && equal->str() == "=") {
830-
Variables::VariableUsage *var = variables.find(tok->varId());
909+
unsigned int varId = tok->varId();
910+
Variables::VariableUsage *var = variables.find(varId);
831911

832-
if (var && var->_type != Variables::reference)
833-
var->_read = true;
912+
if (var && var->_type != Variables::reference) {
913+
variables.read(varId,tok);
914+
}
834915

835916
tok = tok->previous();
836917
}
@@ -868,78 +949,63 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
868949
variables.read(tok->next()->varId(), tok);
869950
} else // addressof
870951
variables.use(tok->next()->varId(), tok); // use = read + write
871-
usedVariables.push_back(tok->next()->varId());
872952
} else if (Token::Match(tok, ">> %var%")) {
873953
variables.use(tok->next()->varId(), tok); // use = read + write
874-
usedVariables.push_back(tok->next()->varId());
875954
} else if (Token::Match(tok, "%var% >>|&") && Token::Match(tok->previous(), "[{};:]")) {
876955
variables.read(tok->varId(), tok);
877-
usedVariables.push_back(tok->next()->varId());
878956
}
879957

880958
// function parameter
881959
else if (Token::Match(tok, "[(,] %var% [")) {
882960
variables.use(tok->next()->varId(), tok); // use = read + write
883-
usedVariables.push_back(tok->next()->varId());
884961
} else if (Token::Match(tok, "[(,] %var% [,)]") && tok->previous()->str() != "*") {
885962
variables.use(tok->next()->varId(), tok); // use = read + write
886-
usedVariables.push_back(tok->next()->varId());
887963
} else if (Token::Match(tok, "[(,] (") &&
888964
Token::Match(tok->next()->link(), ") %var% [,)]")) {
889965
variables.use(tok->next()->link()->next()->varId(), tok); // use = read + write
890-
usedVariables.push_back(tok->next()->link()->next()->varId());
891966
}
892967

893968
// function
894969
else if (Token::Match(tok, "%var% (")) {
895970
variables.read(tok->varId(), tok);
896-
usedVariables.push_back(tok->varId());
897971
if (Token::Match(tok->tokAt(2), "%var% =")) {
898972
variables.read(tok->tokAt(2)->varId(), tok);
899-
usedVariables.push_back(tok->tokAt(2)->varId());
900973
}
901974
}
902975

903976
else if (Token::Match(tok, "[{,] %var% [,}]")) {
904977
variables.read(tok->next()->varId(), tok);
905-
usedVariables.push_back(tok->next()->varId());
906978
}
907979

908980
else if (Token::Match(tok, "%var% .")) {
909981
variables.use(tok->varId(), tok); // use = read + write
910-
usedVariables.push_back(tok->varId());
911982
}
912983

913984
else if (tok->isExtendedOp() &&
914985
Token::Match(tok->next(), "%var%") && !Token::Match(tok->next(), "true|false|new") && tok->strAt(2) != "=") {
915986
variables.readAll(tok->next()->varId(), tok);
916-
usedVariables.push_back(tok->next()->varId());
917987
}
918988

919989
else if (Token::Match(tok, "%var%") && tok->next() && (tok->next()->str() == ")" || tok->next()->isExtendedOp())) {
920990
variables.readAll(tok->varId(), tok);
921-
usedVariables.push_back(tok->varId());
922991
}
923992

924993
else if (Token::Match(tok, "%var% ;") && Token::Match(tok->previous(), "[;{}:]")) {
925994
variables.readAll(tok->varId(), tok);
926-
usedVariables.push_back(tok->varId());
927995
}
928996

929997
else if (Token::Match(tok, "++|-- %var%")) {
930998
if (!Token::Match(tok->previous(), "[;{}:]"))
931999
variables.use(tok->next()->varId(), tok);
9321000
else
9331001
variables.modified(tok->next()->varId(), tok);
934-
usedVariables.push_back(tok->next()->varId());
9351002
}
9361003

9371004
else if (Token::Match(tok, "%var% ++|--")) {
9381005
if (!Token::Match(tok->previous(), "[;{}:]"))
9391006
variables.use(tok->varId(), tok);
9401007
else
9411008
variables.modified(tok->varId(), tok);
942-
usedVariables.push_back(tok->varId());
9431009
}
9441010

9451011
else if (tok->isAssignmentOp()) {
@@ -949,7 +1015,6 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
9491015
variables.write(tok2->varId(), tok);
9501016
else
9511017
variables.read(tok2->varId(), tok);
952-
usedVariables.push_back(tok2->varId());
9531018
}
9541019
}
9551020
}
@@ -972,8 +1037,7 @@ void CheckUnusedVar::checkFunctionVariableUsage()
9721037
// varId, usage {read, write, modified}
9731038
Variables variables;
9741039

975-
std::vector<unsigned int> usedVariables;
976-
checkFunctionVariableUsage_iterateScopes(&*scope, variables, false, usedVariables);
1040+
checkFunctionVariableUsage_iterateScopes(&*scope, variables, false);
9771041

9781042

9791043
// Check usage of all variables in the current scope..

‎lib/checkunusedvar.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class CPPCHECKLIB CheckUnusedVar : public Check {
6464
}
6565

6666
/** @brief %Check for unused function variables */
67-
void checkFunctionVariableUsage_iterateScopes(const Scope* const scope, Variables& variables, bool insideLoop, std::vector<unsigned int> &usedVariables);
67+
void checkFunctionVariableUsage_iterateScopes(const Scope* const scope, Variables& variables, bool insideLoop);
6868
void checkVariableUsage(const Scope* const scope, const Token* start, Variables& variables);
6969
void checkFunctionVariableUsage();
7070

0 commit comments

Comments
 (0)