Skip to content

Fix #2062 FN variable scope can be reduced (class instance) #7885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrchr-github merged 3 commits into from
Oct 14, 2025
Merged

Fix #2062 FN variable scope can be reduced (class instance) #7885

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9e6cee
Fix #2062 FN variable scope can be reduced (class instance)
web-flow Oct 13, 2025
ac01ae7
Format
web-flow Oct 13, 2025
8b45f7a
const
web-flow Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/checkother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,7 @@ void CheckOther::checkVariableScope()
continue;

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

Expand Down
180 changes: 4 additions & 176 deletions lib/checkunusedvar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
else if (mTokenizer->isC() ||
i->typeEndToken()->isStandardType() ||
i->isStlType() ||
isRecordTypeWithoutSideEffects(i->type()) ||
mTokenizer->getSymbolDatabase()->isRecordTypeWithoutSideEffects(i->type()) ||
mSettings->library.detectContainer(i->typeStartToken()) ||
mSettings->library.getTypeCheck("unusedvar", i->typeStartToken()->str()) == Library::TypeCheck::check)
type = Variables::standard;
Expand Down Expand Up @@ -963,7 +963,7 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
// is it a user defined type?
if (!type->isStandardType()) {
const Variable *variable = start->variable();
if (!variable || !isRecordTypeWithoutSideEffects(variable->type()))
if (!variable || !mTokenizer->getSymbolDatabase()->isRecordTypeWithoutSideEffects(variable->type()))
allocateMemory = false;
}
}
Expand Down Expand Up @@ -1254,7 +1254,7 @@ void CheckUnusedVar::checkFunctionVariableUsage()
if (tok->isName()) {
if (isRaiiClass(tok->valueType(), tok->isCpp(), false) ||
(tok->valueType() && tok->valueType()->type == ValueType::RECORD &&
(!tok->valueType()->typeScope || !isRecordTypeWithoutSideEffects(tok->valueType()->typeScope->definedType))))
(!tok->valueType()->typeScope || !symbolDatabase->isRecordTypeWithoutSideEffects(tok->valueType()->typeScope->definedType))))
continue;
tok = tok->next();
}
Expand Down Expand Up @@ -1618,7 +1618,7 @@ void CheckUnusedVar::checkStructMemberUsage()

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

bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type)
{
// a type that has no side effects (no constructors and no members with constructors)
/** @todo false negative: check constructors for side effects */
const std::pair<std::map<const Type *,bool>::iterator,bool> found=mIsRecordTypeWithoutSideEffectsMap.insert(
std::pair<const Type *,bool>(type,false)); //Initialize with side effects for possible recursions
bool & withoutSideEffects = found.first->second;
if (!found.second)
return withoutSideEffects;

// unknown types are assumed to have side effects
if (!type || !type->classScope)
return (withoutSideEffects = false);

// Non-empty constructors => possible side effects
for (const Function& f : type->classScope->functionList) {
if (!f.isConstructor() && !f.isDestructor())
continue;
if (f.argDef && Token::simpleMatch(f.argDef->link(), ") ="))
continue; // ignore default/deleted constructors
const bool emptyBody = (f.functionScope && Token::simpleMatch(f.functionScope->bodyStart, "{ }"));

const Token* nextToken = f.argDef ? f.argDef->link() : nullptr;
if (Token::simpleMatch(nextToken, ") :")) {
// validating initialization list
nextToken = nextToken->next(); // goto ":"

for (const Token *initListToken = nextToken; Token::Match(initListToken, "[:,] %var% [({]"); initListToken = initListToken->linkAt(2)->next()) {
const Token* varToken = initListToken->next();
const Variable* variable = varToken->variable();
if (variable && !isVariableWithoutSideEffects(*variable)) {
return withoutSideEffects = false;
}

const Token* valueEnd = initListToken->linkAt(2);
for (const Token* valueToken = initListToken->tokAt(3); valueToken != valueEnd; valueToken = valueToken->next()) {
const Variable* initValueVar = valueToken->variable();
if (initValueVar && !isVariableWithoutSideEffects(*initValueVar)) {
return withoutSideEffects = false;
}
if ((valueToken->tokType() == Token::Type::eName) ||
(valueToken->tokType() == Token::Type::eLambda) ||
(valueToken->tokType() == Token::Type::eOther)) {
return withoutSideEffects = false;
}
const Function* initValueFunc = valueToken->function();
if (initValueFunc && !isFunctionWithoutSideEffects(*initValueFunc, valueToken,
std::list<const Function*> {})) {
return withoutSideEffects = false;
}
}
}
}

if (!emptyBody)
return (withoutSideEffects = false);
}

// Derived from type that has side effects?
if (std::any_of(type->derivedFrom.cbegin(), type->derivedFrom.cend(), [this](const Type::BaseInfo& derivedFrom) {
return !isRecordTypeWithoutSideEffects(derivedFrom.type);
}))
return (withoutSideEffects = false);

// Is there a member variable with possible side effects
for (const Variable& var : type->classScope->varlist) {
withoutSideEffects = isVariableWithoutSideEffects(var, type);
if (!withoutSideEffects) {
return withoutSideEffects;
}
}


return (withoutSideEffects = true);
}

bool CheckUnusedVar::isVariableWithoutSideEffects(const Variable& var, const Type* type)
{
const Type* variableType = var.type();
if (variableType && variableType != type) {
if (!isRecordTypeWithoutSideEffects(variableType))
return false;
} else {
if (WRONG_DATA(!var.valueType(), var.typeStartToken()))
return false;
const ValueType::Type valueType = var.valueType()->type;
if ((valueType == ValueType::Type::UNKNOWN_TYPE) || (valueType == ValueType::Type::NONSTD))
return false;
}

return true;
}

bool CheckUnusedVar::isEmptyType(const Type* type)
{
// a type that has no variables and no constructor
Expand All @@ -1794,85 +1701,6 @@ bool CheckUnusedVar::isEmptyType(const Type* type)
return (emptyType = false);
}

bool CheckUnusedVar::isFunctionWithoutSideEffects(const Function& func, const Token* functionUsageToken,
std::list<const Function*> checkedFuncs)
{
// no body to analyze
if (!func.hasBody()) {
return false;
}

for (const Token* argsToken = functionUsageToken->next(); !Token::simpleMatch(argsToken, ")"); argsToken = argsToken->next()) {
const Variable* argVar = argsToken->variable();
if (argVar && argVar->isGlobal()) {
return false; // TODO: analyze global variable usage
}
}

bool sideEffectReturnFound = false;
std::set<const Variable*> pointersToGlobals;
for (const Token* bodyToken = func.functionScope->bodyStart->next(); bodyToken != func.functionScope->bodyEnd;
bodyToken = bodyToken->next()) {
// check variable inside function body
const Variable* bodyVariable = bodyToken->variable();
if (bodyVariable) {
if (!isVariableWithoutSideEffects(*bodyVariable)) {
return false;
}
// check if global variable is changed
if (bodyVariable->isGlobal() || (pointersToGlobals.find(bodyVariable) != pointersToGlobals.end())) {
const int indirect = bodyVariable->isArray() ? bodyVariable->dimensions().size() : bodyVariable->isPointer();
if (isVariableChanged(bodyToken, indirect, *mSettings)) {
return false;
}
// check if pointer to global variable assigned to another variable (another_var = &global_var)
if (Token::simpleMatch(bodyToken->tokAt(-1), "&") && Token::simpleMatch(bodyToken->tokAt(-2), "=")) {
const Token* assigned_var_token = bodyToken->tokAt(-3);
if (assigned_var_token && assigned_var_token->variable()) {
pointersToGlobals.insert(assigned_var_token->variable());
}
}
}
}

// check nested function
const Function* bodyFunction = bodyToken->function();
if (bodyFunction) {
if (std::find(checkedFuncs.cbegin(), checkedFuncs.cend(), bodyFunction) != checkedFuncs.cend()) { // recursion found
continue;
}
checkedFuncs.push_back(bodyFunction);
if (!isFunctionWithoutSideEffects(*bodyFunction, bodyToken, checkedFuncs)) {
return false;
}
}

// check returned value
if (Token::simpleMatch(bodyToken, "return")) {
const Token* returnValueToken = bodyToken->next();
// TODO: handle complex return expressions
if (!Token::simpleMatch(returnValueToken->next(), ";")) {
sideEffectReturnFound = true;
continue;
}
// simple one-token return
const Variable* returnVariable = returnValueToken->variable();
if (returnValueToken->isLiteral() ||
(returnVariable && isVariableWithoutSideEffects(*returnVariable))) {
continue;
}
sideEffectReturnFound = true;
}

// unknown name
if (bodyToken->isNameOnly()) {
return false;
}
}

return !sideEffectReturnFound;
}

void CheckUnusedVar::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger)
{
CheckUnusedVar checkUnusedVar(&tokenizer, &tokenizer.getSettings(), errorLogger);
Expand Down
6 changes: 0 additions & 6 deletions lib/checkunusedvar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ class CPPCHECKLIB CheckUnusedVar : public Check {
/** @brief %Check that all struct members are used */
void checkStructMemberUsage();

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

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

std::map<const Type *,bool> mIsRecordTypeWithoutSideEffectsMap;

std::map<const Type *,bool> mIsEmptyTypeMap;

};
Expand Down
Loading
Loading