Skip to content

Commit 7ca4556

Browse files
chrchr-githubweb-flow
chrchr-github
and
web-flow
authored
Fix #2062 FN variable scope can be reduced (class instance) (#7885)
Co-authored-by: chrchr-github <noreply@github.com>
1 parent d69955c commit 7ca4556

File tree

6 files changed

+208
-185
lines changed

6 files changed

+208
-185
lines changed

‎lib/checkother.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1213,7 +1213,7 @@ void CheckOther::checkVariableScope()
12131213
continue;
12141214

12151215
const bool isPtrOrRef = var->isPointer() || var->isReference();
1216-
const bool isSimpleType = var->typeStartToken()->isStandardType() || var->typeStartToken()->isEnumType() || (var->typeStartToken()->isC() && var->type() && var->type()->isStructType());
1216+
const bool isSimpleType = var->typeStartToken()->isStandardType() || var->typeStartToken()->isEnumType() || var->typeStartToken()->isC() || symbolDatabase->isRecordTypeWithoutSideEffects(var->type());
12171217
if (!isPtrOrRef && !isSimpleType && !astIsContainer(var->nameToken()))
12181218
continue;
12191219

‎lib/checkunusedvar.cpp‎

Lines changed: 5 additions & 177 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ static void useFunctionArgs(const Token *tok, Variables& variables)
689689
//---------------------------------------------------------------------------
690690
// Usage of function variables
691691
//---------------------------------------------------------------------------
692-
void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const scope, Variables& variables)
692+
void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const scope, Variables& variables) const
693693
{
694694
// Find declarations if the scope is executable..
695695
if (scope->isExecutable()) {
@@ -715,7 +715,7 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
715715
else if (mTokenizer->isC() ||
716716
i->typeEndToken()->isStandardType() ||
717717
i->isStlType() ||
718-
isRecordTypeWithoutSideEffects(i->type()) ||
718+
mTokenizer->getSymbolDatabase()->isRecordTypeWithoutSideEffects(i->type()) ||
719719
mSettings->library.detectContainer(i->typeStartToken()) ||
720720
mSettings->library.getTypeCheck("unusedvar", i->typeStartToken()->str()) == Library::TypeCheck::check)
721721
type = Variables::standard;
@@ -963,7 +963,7 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
963963
// is it a user defined type?
964964
if (!type->isStandardType()) {
965965
const Variable *variable = start->variable();
966-
if (!variable || !isRecordTypeWithoutSideEffects(variable->type()))
966+
if (!variable || !mTokenizer->getSymbolDatabase()->isRecordTypeWithoutSideEffects(variable->type()))
967967
allocateMemory = false;
968968
}
969969
}
@@ -1254,7 +1254,7 @@ void CheckUnusedVar::checkFunctionVariableUsage()
12541254
if (tok->isName()) {
12551255
if (isRaiiClass(tok->valueType(), tok->isCpp(), false) ||
12561256
(tok->valueType() && tok->valueType()->type == ValueType::RECORD &&
1257-
(!tok->valueType()->typeScope || !isRecordTypeWithoutSideEffects(tok->valueType()->typeScope->definedType))))
1257+
(!tok->valueType()->typeScope || !symbolDatabase->isRecordTypeWithoutSideEffects(tok->valueType()->typeScope->definedType))))
12581258
continue;
12591259
tok = tok->next();
12601260
}
@@ -1618,7 +1618,7 @@ void CheckUnusedVar::checkStructMemberUsage()
16181618

16191619
for (const Variable &var : scope.varlist) {
16201620
// only warn for variables without side effects
1621-
if (!var.typeStartToken()->isStandardType() && !var.isPointer() && !astIsContainer(var.nameToken()) && !isRecordTypeWithoutSideEffects(var.type()))
1621+
if (!var.typeStartToken()->isStandardType() && !var.isPointer() && !astIsContainer(var.nameToken()) && !mTokenizer->getSymbolDatabase()->isRecordTypeWithoutSideEffects(var.type()))
16221622
continue;
16231623
if (isInherited && !var.isPrivate())
16241624
continue;
@@ -1680,99 +1680,6 @@ void CheckUnusedVar::unusedStructMemberError(const Token* tok, const std::string
16801680
reportError(tok, Severity::style, "unusedStructMember", "$symbol:" + structname + "::" + varname + '\n' + prefix + " member '$symbol' is never used.", CWE563, Certainty::normal);
16811681
}
16821682

1683-
bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type)
1684-
{
1685-
// a type that has no side effects (no constructors and no members with constructors)
1686-
/** @todo false negative: check constructors for side effects */
1687-
const std::pair<std::map<const Type *,bool>::iterator,bool> found=mIsRecordTypeWithoutSideEffectsMap.insert(
1688-
std::pair<const Type *,bool>(type,false)); //Initialize with side effects for possible recursions
1689-
bool & withoutSideEffects = found.first->second;
1690-
if (!found.second)
1691-
return withoutSideEffects;
1692-
1693-
// unknown types are assumed to have side effects
1694-
if (!type || !type->classScope)
1695-
return (withoutSideEffects = false);
1696-
1697-
// Non-empty constructors => possible side effects
1698-
for (const Function& f : type->classScope->functionList) {
1699-
if (!f.isConstructor() && !f.isDestructor())
1700-
continue;
1701-
if (f.argDef && Token::simpleMatch(f.argDef->link(), ") ="))
1702-
continue; // ignore default/deleted constructors
1703-
const bool emptyBody = (f.functionScope && Token::simpleMatch(f.functionScope->bodyStart, "{ }"));
1704-
1705-
const Token* nextToken = f.argDef ? f.argDef->link() : nullptr;
1706-
if (Token::simpleMatch(nextToken, ") :")) {
1707-
// validating initialization list
1708-
nextToken = nextToken->next(); // goto ":"
1709-
1710-
for (const Token *initListToken = nextToken; Token::Match(initListToken, "[:,] %var% [({]"); initListToken = initListToken->linkAt(2)->next()) {
1711-
const Token* varToken = initListToken->next();
1712-
const Variable* variable = varToken->variable();
1713-
if (variable && !isVariableWithoutSideEffects(*variable)) {
1714-
return withoutSideEffects = false;
1715-
}
1716-
1717-
const Token* valueEnd = initListToken->linkAt(2);
1718-
for (const Token* valueToken = initListToken->tokAt(3); valueToken != valueEnd; valueToken = valueToken->next()) {
1719-
const Variable* initValueVar = valueToken->variable();
1720-
if (initValueVar && !isVariableWithoutSideEffects(*initValueVar)) {
1721-
return withoutSideEffects = false;
1722-
}
1723-
if ((valueToken->tokType() == Token::Type::eName) ||
1724-
(valueToken->tokType() == Token::Type::eLambda) ||
1725-
(valueToken->tokType() == Token::Type::eOther)) {
1726-
return withoutSideEffects = false;
1727-
}
1728-
const Function* initValueFunc = valueToken->function();
1729-
if (initValueFunc && !isFunctionWithoutSideEffects(*initValueFunc, valueToken,
1730-
std::list<const Function*> {})) {
1731-
return withoutSideEffects = false;
1732-
}
1733-
}
1734-
}
1735-
}
1736-
1737-
if (!emptyBody)
1738-
return (withoutSideEffects = false);
1739-
}
1740-
1741-
// Derived from type that has side effects?
1742-
if (std::any_of(type->derivedFrom.cbegin(), type->derivedFrom.cend(), [this](const Type::BaseInfo& derivedFrom) {
1743-
return !isRecordTypeWithoutSideEffects(derivedFrom.type);
1744-
}))
1745-
return (withoutSideEffects = false);
1746-
1747-
// Is there a member variable with possible side effects
1748-
for (const Variable& var : type->classScope->varlist) {
1749-
withoutSideEffects = isVariableWithoutSideEffects(var, type);
1750-
if (!withoutSideEffects) {
1751-
return withoutSideEffects;
1752-
}
1753-
}
1754-
1755-
1756-
return (withoutSideEffects = true);
1757-
}
1758-
1759-
bool CheckUnusedVar::isVariableWithoutSideEffects(const Variable& var, const Type* type)
1760-
{
1761-
const Type* variableType = var.type();
1762-
if (variableType && variableType != type) {
1763-
if (!isRecordTypeWithoutSideEffects(variableType))
1764-
return false;
1765-
} else {
1766-
if (WRONG_DATA(!var.valueType(), var.typeStartToken()))
1767-
return false;
1768-
const ValueType::Type valueType = var.valueType()->type;
1769-
if ((valueType == ValueType::Type::UNKNOWN_TYPE) || (valueType == ValueType::Type::NONSTD))
1770-
return false;
1771-
}
1772-
1773-
return true;
1774-
}
1775-
17761683
bool CheckUnusedVar::isEmptyType(const Type* type)
17771684
{
17781685
// a type that has no variables and no constructor
@@ -1794,85 +1701,6 @@ bool CheckUnusedVar::isEmptyType(const Type* type)
17941701
return (emptyType = false);
17951702
}
17961703

1797-
bool CheckUnusedVar::isFunctionWithoutSideEffects(const Function& func, const Token* functionUsageToken,
1798-
std::list<const Function*> checkedFuncs)
1799-
{
1800-
// no body to analyze
1801-
if (!func.hasBody()) {
1802-
return false;
1803-
}
1804-
1805-
for (const Token* argsToken = functionUsageToken->next(); !Token::simpleMatch(argsToken, ")"); argsToken = argsToken->next()) {
1806-
const Variable* argVar = argsToken->variable();
1807-
if (argVar && argVar->isGlobal()) {
1808-
return false; // TODO: analyze global variable usage
1809-
}
1810-
}
1811-
1812-
bool sideEffectReturnFound = false;
1813-
std::set<const Variable*> pointersToGlobals;
1814-
for (const Token* bodyToken = func.functionScope->bodyStart->next(); bodyToken != func.functionScope->bodyEnd;
1815-
bodyToken = bodyToken->next()) {
1816-
// check variable inside function body
1817-
const Variable* bodyVariable = bodyToken->variable();
1818-
if (bodyVariable) {
1819-
if (!isVariableWithoutSideEffects(*bodyVariable)) {
1820-
return false;
1821-
}
1822-
// check if global variable is changed
1823-
if (bodyVariable->isGlobal() || (pointersToGlobals.find(bodyVariable) != pointersToGlobals.end())) {
1824-
const int indirect = bodyVariable->isArray() ? bodyVariable->dimensions().size() : bodyVariable->isPointer();
1825-
if (isVariableChanged(bodyToken, indirect, *mSettings)) {
1826-
return false;
1827-
}
1828-
// check if pointer to global variable assigned to another variable (another_var = &global_var)
1829-
if (Token::simpleMatch(bodyToken->tokAt(-1), "&") && Token::simpleMatch(bodyToken->tokAt(-2), "=")) {
1830-
const Token* assigned_var_token = bodyToken->tokAt(-3);
1831-
if (assigned_var_token && assigned_var_token->variable()) {
1832-
pointersToGlobals.insert(assigned_var_token->variable());
1833-
}
1834-
}
1835-
}
1836-
}
1837-
1838-
// check nested function
1839-
const Function* bodyFunction = bodyToken->function();
1840-
if (bodyFunction) {
1841-
if (std::find(checkedFuncs.cbegin(), checkedFuncs.cend(), bodyFunction) != checkedFuncs.cend()) { // recursion found
1842-
continue;
1843-
}
1844-
checkedFuncs.push_back(bodyFunction);
1845-
if (!isFunctionWithoutSideEffects(*bodyFunction, bodyToken, checkedFuncs)) {
1846-
return false;
1847-
}
1848-
}
1849-
1850-
// check returned value
1851-
if (Token::simpleMatch(bodyToken, "return")) {
1852-
const Token* returnValueToken = bodyToken->next();
1853-
// TODO: handle complex return expressions
1854-
if (!Token::simpleMatch(returnValueToken->next(), ";")) {
1855-
sideEffectReturnFound = true;
1856-
continue;
1857-
}
1858-
// simple one-token return
1859-
const Variable* returnVariable = returnValueToken->variable();
1860-
if (returnValueToken->isLiteral() ||
1861-
(returnVariable && isVariableWithoutSideEffects(*returnVariable))) {
1862-
continue;
1863-
}
1864-
sideEffectReturnFound = true;
1865-
}
1866-
1867-
// unknown name
1868-
if (bodyToken->isNameOnly()) {
1869-
return false;
1870-
}
1871-
}
1872-
1873-
return !sideEffectReturnFound;
1874-
}
1875-
18761704
void CheckUnusedVar::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger)
18771705
{
18781706
CheckUnusedVar checkUnusedVar(&tokenizer, &tokenizer.getSettings(), errorLogger);

‎lib/checkunusedvar.h‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,13 @@ class CPPCHECKLIB CheckUnusedVar : public Check {
6060
void runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) override;
6161

6262
/** @brief %Check for unused function variables */
63-
void checkFunctionVariableUsage_iterateScopes(const Scope* scope, Variables& variables);
63+
void checkFunctionVariableUsage_iterateScopes(const Scope* scope, Variables& variables) const;
6464
void checkFunctionVariableUsage();
6565

6666
/** @brief %Check that all struct members are used */
6767
void checkStructMemberUsage();
6868

69-
bool isRecordTypeWithoutSideEffects(const Type* type);
70-
bool isVariableWithoutSideEffects(const Variable& var, const Type* type = nullptr);
7169
bool isEmptyType(const Type* type);
72-
bool isFunctionWithoutSideEffects(const Function& func, const Token* functionUsageToken,
73-
std::list<const Function*> checkedFuncs);
7470

7571
// Error messages..
7672
void unusedStructMemberError(const Token *tok, const std::string &structname, const std::string &varname, const std::string& prefix = "struct");
@@ -96,8 +92,6 @@ class CPPCHECKLIB CheckUnusedVar : public Check {
9692
"- unused struct member\n";
9793
}
9894

99-
std::map<const Type *,bool> mIsRecordTypeWithoutSideEffectsMap;
100-
10195
std::map<const Type *,bool> mIsEmptyTypeMap;
10296

10397
};

0 commit comments

Comments
 (0)