Skip to content

Commit eb4754b

Browse files
pfultz2pfultz2
authored
Fix issue 9587: False positive: parameter can be declared with const (#2667)
1 parent 0a6150a commit eb4754b

9 files changed

Lines changed: 99 additions & 47 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,22 @@ const Token* getParentMember(const Token * tok)
344344
return tok;
345345
}
346346

347+
const Token* getParentLifetime(const Token* tok)
348+
{
349+
if (!tok)
350+
return tok;
351+
const Variable* var = tok->variable();
352+
// TODO: Call getLifetimeVariable for deeper analysis
353+
if (!var)
354+
return tok;
355+
if (var->isLocal() || var->isArgument())
356+
return tok;
357+
const Token* parent = getParentMember(tok);
358+
if (parent != tok)
359+
return getParentLifetime(parent);
360+
return tok;
361+
}
362+
347363
bool astIsLHS(const Token* tok)
348364
{
349365
if (!tok)
@@ -448,8 +464,6 @@ bool isAliasOf(const Token *tok, nonneg int varid)
448464
{
449465
if (tok->varId() == varid)
450466
return false;
451-
if (tok->varId() == 0)
452-
return false;
453467
for (const ValueFlow::Value &val : tok->values()) {
454468
if (!val.isLocalLifetimeValue())
455469
continue;

‎lib/astutils.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ const Token* astParentSkipParens(const Token* tok);
9797

9898
const Token* getParentMember(const Token * tok);
9999

100+
const Token* getParentLifetime(const Token* tok);
101+
100102
bool astIsLHS(const Token* tok);
101103
bool astIsRHS(const Token* tok);
102104

‎lib/checkautovariables.cpp‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -432,22 +432,6 @@ static bool isDeadScope(const Token * tok, const Scope * scope)
432432
return false;
433433
}
434434

435-
static const Token * getParentLifetime(const Token *tok)
436-
{
437-
if (!tok)
438-
return tok;
439-
const Variable * var = tok->variable();
440-
// TODO: Call getLifetimeVariable for deeper analysis
441-
if (!var)
442-
return tok;
443-
if (var->isLocal())
444-
return tok;
445-
const Token * parent = getParentMember(tok);
446-
if (parent != tok)
447-
return getParentLifetime(parent);
448-
return tok;
449-
}
450-
451435
static int getPointerDepth(const Token *tok)
452436
{
453437
if (!tok)

‎lib/checkother.cpp‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "token.h"
3232
#include "tokenize.h"
3333
#include "utils.h"
34+
#include "valueflow.h"
3435

3536
#include <algorithm> // find_if()
3637
#include <list>
@@ -1363,9 +1364,20 @@ void CheckOther::checkConstVariable()
13631364
continue;
13641365
if (isVariableChanged(var, mSettings, mTokenizer->isCPP()))
13651366
continue;
1366-
if (Function::returnsReference(function) &&
1367-
Token::findmatch(var->nameToken(), "return %varid% ;|[|.", scope->bodyEnd, var->declarationId()))
1368-
continue;
1367+
if (Function::returnsReference(function)) {
1368+
std::vector<const Token*> returns = Function::findReturns(function);
1369+
if (std::any_of(returns.begin(), returns.end(), [&](const Token* retTok) {
1370+
if (retTok->varId() == var->declarationId())
1371+
return true;
1372+
while (Token::simpleMatch(retTok, "."))
1373+
retTok = retTok->astOperand2();
1374+
const Variable* retVar = getLifetimeVariable(getParentLifetime(retTok));
1375+
if (!retVar)
1376+
return false;
1377+
return retVar->declarationId() == var->declarationId();
1378+
}))
1379+
continue;
1380+
}
13691381
// Skip if address is taken
13701382
if (Token::findmatch(var->nameToken(), "& %varid%", scope->bodyEnd, var->declarationId()))
13711383
continue;

‎lib/symboldatabase.cpp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2439,6 +2439,32 @@ bool Function::returnsReference(const Function* function, bool unknown)
24392439
return false;
24402440
}
24412441

2442+
std::vector<const Token*> Function::findReturns(const Function* f)
2443+
{
2444+
std::vector<const Token*> result;
2445+
if (!f)
2446+
return result;
2447+
const Scope* scope = f->functionScope;
2448+
if (!scope)
2449+
return result;
2450+
for (const Token* tok = scope->bodyStart->next(); tok && tok != scope->bodyEnd; tok = tok->next()) {
2451+
if (tok->str() == "{" && tok->scope() &&
2452+
(tok->scope()->type == Scope::eLambda || tok->scope()->type == Scope::eClass)) {
2453+
tok = tok->link();
2454+
continue;
2455+
}
2456+
if (Token::simpleMatch(tok->astParent(), "return")) {
2457+
result.push_back(tok);
2458+
}
2459+
// Skip lambda functions since the scope may not be set correctly
2460+
const Token* lambdaEndToken = findLambdaEndToken(tok);
2461+
if (lambdaEndToken) {
2462+
tok = lambdaEndToken;
2463+
}
2464+
}
2465+
return result;
2466+
}
2467+
24422468
const Token * Function::constructorMemberInitialization() const
24432469
{
24442470
if (!isConstructor() || !functionScope || !functionScope->bodyStart)

‎lib/symboldatabase.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,8 @@ class CPPCHECKLIB Function {
896896

897897
static bool returnsReference(const Function* function, bool unknown = false);
898898

899+
static std::vector<const Token*> findReturns(const Function* f);
900+
899901
const Token* returnDefEnd() const {
900902
if (this->hasTrailingReturnType()) {
901903
return Token::findmatch(retDef, "{|;");

‎lib/valueflow.cpp‎

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2718,30 +2718,6 @@ static void valueFlowForward(Token* startToken,
27182718
}
27192719
}
27202720

2721-
static std::vector<const Token*> findReturns(const Function* f)
2722-
{
2723-
std::vector<const Token*> result;
2724-
const Scope* scope = f->functionScope;
2725-
if (!scope)
2726-
return result;
2727-
for (const Token* tok = scope->bodyStart->next(); tok && tok != scope->bodyEnd; tok = tok->next()) {
2728-
if (tok->str() == "{" && tok->scope() &&
2729-
(tok->scope()->type == Scope::eLambda || tok->scope()->type == Scope::eClass)) {
2730-
tok = tok->link();
2731-
continue;
2732-
}
2733-
if (Token::simpleMatch(tok->astParent(), "return")) {
2734-
result.push_back(tok);
2735-
}
2736-
// Skip lambda functions since the scope may not be set correctly
2737-
const Token* lambdaEndToken = findLambdaEndToken(tok);
2738-
if (lambdaEndToken) {
2739-
tok = lambdaEndToken;
2740-
}
2741-
}
2742-
return result;
2743-
}
2744-
27452721
static int getArgumentPos(const Variable *var, const Function *f)
27462722
{
27472723
auto arg_it = std::find_if(f->argumentList.begin(), f->argumentList.end(), [&](const Variable &v) {
@@ -2850,6 +2826,16 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
28502826
return {{tok, true, std::move(errorPath)}};
28512827
if (vartok)
28522828
return getLifetimeTokens(vartok, std::move(errorPath), depth - 1);
2829+
} else if (Token::simpleMatch(var->nameToken()->astParent(), ":") &&
2830+
var->nameToken()->astParent()->astParent() &&
2831+
Token::simpleMatch(var->nameToken()->astParent()->astParent()->previous(), "for (")) {
2832+
errorPath.emplace_back(var->nameToken(), "Assigned to reference.");
2833+
const Token* vartok = var->nameToken();
2834+
if (vartok == tok)
2835+
return {{tok, true, std::move(errorPath)}};
2836+
const Token* contok = var->nameToken()->astParent()->astOperand2();
2837+
if (contok)
2838+
return getLifetimeTokens(contok, std::move(errorPath), depth - 1);
28532839
} else {
28542840
return std::vector<LifetimeToken> {};
28552841
}
@@ -2860,7 +2846,7 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
28602846
if (!Function::returnsReference(f))
28612847
return {{tok, std::move(errorPath)}};
28622848
std::vector<LifetimeToken> result;
2863-
std::vector<const Token*> returns = findReturns(f);
2849+
std::vector<const Token*> returns = Function::findReturns(f);
28642850
for (const Token* returnTok : returns) {
28652851
if (returnTok == tok)
28662852
continue;
@@ -2947,6 +2933,12 @@ const Variable* getLifetimeVariable(const Token* tok, ValueFlow::Value::ErrorPat
29472933
return nullptr;
29482934
}
29492935

2936+
const Variable* getLifetimeVariable(const Token* tok)
2937+
{
2938+
ValueFlow::Value::ErrorPath errorPath;
2939+
return getLifetimeVariable(tok, errorPath, nullptr);
2940+
}
2941+
29502942
static bool isNotLifetimeValue(const ValueFlow::Value& val)
29512943
{
29522944
return !val.isLifetimeValue();
@@ -3394,7 +3386,7 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
33943386
const Function *f = tok->function();
33953387
if (Function::returnsReference(f))
33963388
return;
3397-
std::vector<const Token*> returns = findReturns(f);
3389+
std::vector<const Token*> returns = Function::findReturns(f);
33983390
const bool inconclusive = returns.size() > 1;
33993391
for (const Token* returnTok : returns) {
34003392
if (returnTok == tok)

‎lib/valueflow.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,8 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value:
359359

360360
const Variable* getLifetimeVariable(const Token* tok, ValueFlow::Value::ErrorPath& errorPath, bool* addressOf = nullptr);
361361

362+
const Variable* getLifetimeVariable(const Token* tok);
363+
362364
bool isLifetimeBorrowed(const Token *tok, const Settings *settings);
363365

364366
std::string lifetimeType(const Token *tok, const ValueFlow::Value *val);

‎test/testother.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1959,6 +1959,24 @@ class TestOther : public TestFixture {
19591959
"}\n");
19601960
ASSERT_EQUALS("", errout.str());
19611961

1962+
check("int* f(std::list<int>& x, unsigned int y) {\n"
1963+
" for (int& m : x) {\n"
1964+
" if (m == y)\n"
1965+
" return &m;\n"
1966+
" }\n"
1967+
" return nullptr;\n"
1968+
"}\n");
1969+
ASSERT_EQUALS("", errout.str());
1970+
1971+
check("int& f(std::list<int>& x, int& y) {\n"
1972+
" for (int& m : x) {\n"
1973+
" if (m == y)\n"
1974+
" return m;\n"
1975+
" }\n"
1976+
" return y;\n"
1977+
"}\n");
1978+
ASSERT_EQUALS("", errout.str());
1979+
19621980
check("void e();\n"
19631981
"void g(void);\n"
19641982
"void h(void);\n"

0 commit comments

Comments
 (0)