walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix issue 10086: false positive: (style) constVariable: Variable 'x' can be declared with const (#3219)

This commit is contained in:
Paul Fultz II 2021-04-30 10:47:08 -05:00 committed by GitHub
parent f4e69b99d2
commit 31e3e4d87b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 98 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
lib/checkother.cpp
View File

@ -1392,21 +1392,18 @@ void CheckOther::checkConstVariable()
continue;
if (isVariableChanged(var, mSettings, mTokenizer->isCPP()))
continue;
if (Function::returnsReference(function)) {
if (Function::returnsReference(function) && !Function::returnsConst(function)) {
std::vector<const Token*> returns = Function::findReturns(function);
if (std::any_of(returns.begin(), returns.end(), [&](const Token* retTok) {
if (retTok->varId() == var->declarationId())
return true;
while (retTok && retTok->isCast())
retTok = retTok->astOperand2();
while (Token::simpleMatch(retTok, "."))
retTok = retTok->astOperand2();
const Variable* retVar = getLifetimeVariable(getParentLifetime(retTok));
if (!retVar)
return false;
return retVar->declarationId() == var->declarationId();
}))
continue;
if (retTok->varId() == var->declarationId())
return true;
while (retTok && retTok->isCast())
retTok = retTok->astOperand2();
while (Token::simpleMatch(retTok, "."))
retTok = retTok->astOperand2();
return hasLifetimeToken(getParentLifetime(retTok), var->nameToken());
}))
continue;
}
// Skip if address is taken
if (Token::findmatch(var->nameToken(), "& %varid%", scope->bodyEnd, var->declarationId()))

43
lib/symboldatabase.cpp
View File

@ -2629,6 +2629,37 @@ bool Function::argsMatch(const Scope *scope, const Token *first, const Token *se
return false;
}
static bool isUnknownType(const Token* start, const Token* end)
{
while (Token::Match(start, "const|volatile"))
start = start->next();
if (Token::Match(start, ":: %name%"))
start = start->next();
while (Token::Match(start, "%name% :: %name%"))
start = start->tokAt(2);
if (start->tokAt(1) == end && !start->type() && !start->isStandardType())
return true;
// TODO: Try to deduce the type of the expression
if (Token::Match(start, "decltype|typeof"))
return true;
return false;
}
bool Function::returnsConst(const Function* function, bool unknown)
{
if (!function)
return false;
if (function->type != Function::eFunction)
return false;
const Token* defEnd = function->returnDefEnd();
if (Token::findsimplematch(function->retDef, "const", defEnd))
return true;
// Check for unknown types, which could be a const
if (isUnknownType(function->retDef, defEnd))
return unknown;
return false;
}
bool Function::returnsReference(const Function* function, bool unknown)
{
if (!function)
@ -2639,17 +2670,7 @@ bool Function::returnsReference(const Function* function, bool unknown)
if (defEnd->strAt(-1) == "&")
return true;
// Check for unknown types, which could be a reference
const Token* start = function->retDef;
while (Token::Match(start, "const|volatile"))
start = start->next();
if (Token::Match(start, ":: %name%"))
start = start->next();
while (Token::Match(start, "%name% :: %name%"))
start = start->tokAt(2);
if (start->tokAt(1) == defEnd && !start->type() && !start->isStandardType())
return unknown;
// TODO: Try to deduce the type of the expression
if (Token::Match(start, "decltype|typeof"))
if (isUnknownType(function->retDef, defEnd))
return unknown;
return false;
}

2
lib/symboldatabase.h
View File

@ -904,6 +904,8 @@ public:
bool argsMatch(const Scope *scope, const Token *first, const Token *second, const std::string &path, nonneg int path_length) const;
static bool returnsConst(const Function* function, bool unknown = false);
static bool returnsReference(const Function* function, bool unknown = false);
static std::vector<const Token*> findReturns(const Function* f);

4
lib/token.h
View File

@ -812,7 +812,9 @@ public:
}
nonneg int exprId() const {
return mImpl->mExprId;
if (mImpl->mExprId)
return mImpl->mExprId;
return mImpl->mVarId;
}
void exprId(nonneg int id) {
mImpl->mExprId = id;

40
lib/valueflow.cpp
View File

@ -2676,11 +2676,18 @@ ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive)
return values.front();
}
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, ValueFlow::Value::ErrorPath errorPath, int depth)
template <class Predicate>
static std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
bool escape,
ValueFlow::Value::ErrorPath errorPath,
Predicate pred,
int depth = 20)
{
if (!tok)
return std::vector<LifetimeToken> {};
const Variable *var = tok->variable();
if (pred(tok))
return {{tok, std::move(errorPath)}};
if (depth < 0)
return {{tok, std::move(errorPath)}};
if (var && var->declarationId() == tok->varId()) {
@ -2699,7 +2706,7 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, Valu
(!escape && (var->isConst() || var->isRValueReference()) && temporary))
return {{tok, true, std::move(errorPath)}};
if (vartok)
return getLifetimeTokens(vartok, escape, std::move(errorPath), depth - 1);
return getLifetimeTokens(vartok, escape, std::move(errorPath), pred, depth - 1);
} else if (Token::simpleMatch(var->nameToken()->astParent(), ":") &&
var->nameToken()->astParent()->astParent() &&
Token::simpleMatch(var->nameToken()->astParent()->astParent()->previous(), "for (")) {
@ -2709,7 +2716,7 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, Valu
return {{tok, true, std::move(errorPath)}};
const Token* contok = var->nameToken()->astParent()->astOperand2();
if (contok)
return getLifetimeTokens(contok, escape, std::move(errorPath), depth - 1);
return getLifetimeTokens(contok, escape, std::move(errorPath), pred, depth - 1);
} else {
return std::vector<LifetimeToken> {};
}
@ -2724,7 +2731,7 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, Valu
for (const Token* returnTok : returns) {
if (returnTok == tok)
continue;
for (LifetimeToken& lt : getLifetimeTokens(returnTok, escape, errorPath, depth - returns.size())) {
for (LifetimeToken& lt : getLifetimeTokens(returnTok, escape, errorPath, pred, depth - returns.size())) {
const Token* argvarTok = lt.token;
const Variable* argvar = argvarTok->variable();
if (!argvar)
@ -2741,7 +2748,8 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, Valu
lt.errorPath.emplace_back(returnTok, "Return reference.");
lt.errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->expressionString() + "'.");
std::vector<LifetimeToken> arglts = LifetimeToken::setInconclusive(
getLifetimeTokens(argTok, escape, std::move(lt.errorPath), depth - returns.size()), returns.size() > 1);
getLifetimeTokens(argTok, escape, std::move(lt.errorPath), pred, depth - returns.size()),
returns.size() > 1);
result.insert(result.end(), arglts.begin(), arglts.end());
}
}
@ -2753,7 +2761,8 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, Valu
if (y == Library::Container::Yield::AT_INDEX || y == Library::Container::Yield::ITEM) {
errorPath.emplace_back(tok->previous(), "Accessing container.");
return LifetimeToken::setAddressOf(
getLifetimeTokens(tok->tokAt(-2)->astOperand1(), escape, std::move(errorPath), depth - 1), false);
getLifetimeTokens(tok->tokAt(-2)->astOperand1(), escape, std::move(errorPath), pred, depth - 1),
false);
}
}
} else if (Token::Match(tok, ".|::|[")) {
@ -2778,16 +2787,31 @@ std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, Valu
if (v.tokvalue == tok)
continue;
errorPath.insert(errorPath.end(), v.errorPath.begin(), v.errorPath.end());
return getLifetimeTokens(v.tokvalue, escape, std::move(errorPath), depth - 1);
return getLifetimeTokens(v.tokvalue, escape, std::move(errorPath), pred, depth - 1);
}
} else {
return LifetimeToken::setAddressOf(getLifetimeTokens(vartok, escape, std::move(errorPath), depth - 1),
return LifetimeToken::setAddressOf(getLifetimeTokens(vartok, escape, std::move(errorPath), pred, depth - 1),
!(astIsContainer(vartok) && Token::simpleMatch(vartok->astParent(), "[")));
}
}
return {{tok, std::move(errorPath)}};
}
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, ValueFlow::Value::ErrorPath errorPath)
{
return getLifetimeTokens(tok, escape, std::move(errorPath), [](const Token*) { return false; });
}
bool hasLifetimeToken(const Token* tok, const Token* lifetime)
{
bool result = false;
getLifetimeTokens(tok, false, ValueFlow::Value::ErrorPath{}, [&](const Token* tok2) {
result = tok2->exprId() == lifetime->exprId();
return result;
});
return result;
}
static const Token* getLifetimeToken(const Token* tok, ValueFlow::Value::ErrorPath& errorPath, bool* addressOf = nullptr)
{
std::vector<LifetimeToken> lts = getLifetimeTokens(tok);

7
lib/valueflow.h
View File

@ -427,9 +427,10 @@ ValueFlow::Value inferCondition(std::string op, MathLib::bigint val, const Token
ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, MathLib::bigint val);
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
bool escape = false,
ValueFlow::Value::ErrorPath errorPath = ValueFlow::Value::ErrorPath{},
int depth = 20);
bool escape = false,
ValueFlow::Value::ErrorPath errorPath = ValueFlow::Value::ErrorPath{});
bool hasLifetimeToken(const Token* tok, const Token* lifetime);
const Variable* getLifetimeVariable(const Token* tok, ValueFlow::Value::ErrorPath& errorPath, bool* addressOf = nullptr);

15
test/testother.cpp
View File

@ -1865,6 +1865,11 @@ private:
"}");
ASSERT_EQUALS("", errout.str());
check("const int& f(std::vector<int>& x) {\n"
" return x[0];\n"
"}");
ASSERT_EQUALS("[test.cpp:1]: (style) Parameter 'x' can be declared with const\n", errout.str());
check("int f(std::vector<int>& x) {\n"
" x[0]++;\n"
" return x[0];\n"
@ -2325,6 +2330,16 @@ private:
"}\n");
ASSERT_EQUALS("", errout.str());
// #10086
check("struct V {\n"
" V& get(typename std::vector<V>::size_type i) {\n"
" std::vector<V>& arr = v;\n"
" return arr[i];\n"
" }\n"
" std::vector<V> v;\n"
"};\n");
ASSERT_EQUALS("", errout.str());
check("void e();\n"
"void g(void);\n"
"void h(void);\n"