Skip to content
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.

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

Fix #11713 FN constParameterPointer/Reference with unused parameter #6085

Merged
merged 11 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
57 changes: 34 additions & 23 deletions lib/checkother.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1333,20 +1333,6 @@ void CheckOther::passedByValueError(const Variable* var, bool inconclusive, bool
reportError(errorPath, Severity::performance, id.c_str(), msg, CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal);
}

static bool isUnusedVariable(const Variable *var)
{
if (!var)
return false;
if (!var->scope())
return false;
const Token *start = var->declEndToken();
if (!start)
return false;
if (Token::Match(start, "; %varid% =", var->declarationId()))
start = start->tokAt(2);
return !Token::findmatch(start->next(), "%varid%", var->scope()->bodyEnd, var->declarationId());
}

static bool isVariableMutableInInitializer(const Token* start, const Token * end, nonneg int varid)
{
if (!start)
Expand Down Expand Up @@ -1402,8 +1388,6 @@ void CheckOther::checkConstVariable()
if (function && var->isArgument()) {
if (function->isImplicitlyVirtual() || function->templateDef)
continue;
if (isUnusedVariable(var))
continue;
if (function->isConstructor() && isVariableMutableInInitializer(function->constructorMemberInitialization(), scope->bodyStart, var->declarationId()))
continue;
}
Expand All @@ -1417,6 +1401,8 @@ void CheckOther::checkConstVariable()
continue;
if (var->isVolatile())
continue;
if (var->isMaybeUnused())
continue;
if (var->nameToken()->isExpandedMacro())
continue;
if (isStructuredBindingVariable(var)) // TODO: check all bound variables
Expand Down Expand Up @@ -1505,6 +1491,26 @@ static const Token* getVariableChangedStart(const Variable* p)
return start;
}

namespace {
struct CompareVariables {
bool operator()(const Variable* a, const Variable* b) const {
const int fileA = a->nameToken()->fileIndex();
const int fileB = b->nameToken()->fileIndex();
if (fileA == fileB) {
danmar marked this conversation as resolved.
Show resolved Hide resolved
const int lineA = a->nameToken()->linenr();
const int lineB = b->nameToken()->linenr();
if (lineA == lineB) {
const int columnA = a->nameToken()->column();
const int columnB = b->nameToken()->column();
return columnA < columnB;
}
return lineA < lineB;
}
return fileA < fileB;
}
};
}

void CheckOther::checkConstPointer()
{
if (!mSettings->severity.isEnabled(Severity::style) &&
Expand All @@ -1514,18 +1520,20 @@ void CheckOther::checkConstPointer()

logChecker("CheckOther::checkConstPointer"); // style

std::vector<const Variable*> pointers, nonConstPointers;
std::set<const Variable*, CompareVariables> pointers, nonConstPointers;
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
const Variable* const var = tok->variable();
if (!var)
continue;
if (!var->isLocal() && !var->isArgument())
continue;
const Token* const nameTok = var->nameToken();
// declarations of (static) pointers are (not) split up, array declarations are never split up
if (tok == nameTok && (!var->isStatic() || Token::simpleMatch(nameTok->next(), "[")) &&
!astIsRangeBasedForDecl(nameTok))
continue;
if (tok == nameTok) {
// declarations of (static) pointers are (not) split up, array declarations are never split up
if (var->isLocal() && (!var->isStatic() || Token::simpleMatch(nameTok->next(), "[")) &&
!astIsRangeBasedForDecl(nameTok))
continue;
}
const ValueType* const vt = tok->valueType();
if (!vt)
continue;
Expand All @@ -1535,7 +1543,7 @@ void CheckOther::checkConstPointer()
continue;
if (std::find(nonConstPointers.cbegin(), nonConstPointers.cend(), var) != nonConstPointers.cend())
continue;
pointers.emplace_back(var);
pointers.emplace(var);
const Token* parent = tok->astParent();
enum Deref { NONE, DEREF, MEMBER } deref = NONE;
bool hasIncDec = false;
Expand Down Expand Up @@ -1620,12 +1628,15 @@ void CheckOther::checkConstPointer()
continue;
}
}
nonConstPointers.emplace_back(var);
if (tok != nameTok)
nonConstPointers.emplace(var);
}
for (const Variable *p: pointers) {
if (p->isArgument()) {
if (!p->scope() || !p->scope()->function || p->scope()->function->isImplicitlyVirtual(true) || p->scope()->function->hasVirtualSpecifier())
continue;
if (p->isMaybeUnused())
continue;
}
if (std::find(nonConstPointers.cbegin(), nonConstPointers.cend(), p) == nonConstPointers.cend()) {
const Token *start = getVariableChangedStart(p);
Expand Down
1 change: 1 addition & 0 deletions test/cfg/libcurl.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ void validCode()
}
}

// cppcheck-suppress constParameterPointer
void ignoredReturnValue(CURL * handle)
{
// cppcheck-suppress ignoredReturnValue
Expand Down
1 change: 1 addition & 0 deletions test/cfg/posix.c
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,7 @@ void nullPointer(char *p, int fd, pthread_mutex_t mutex)
pthread_mutex_unlock(NULL);
}

// cppcheck-suppress constParameterCallback
void* f_returns_NULL(void* arg)
{
return NULL;
Expand Down
6 changes: 3 additions & 3 deletions test/cfg/wxwidgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,10 @@ void deprecatedFunctions_wxDataViewCustomRenderer(wxDataViewCustomRenderer &data
dataViewCustomRenderer.LeftClick(cursor, cell, model, item, col);
}

void deprecatedFunctions(wxApp &a,
void deprecatedFunctions([[maybe_unused]] wxApp &a,
const wxString &s,
wxArtProvider *artProvider,
wxCalendarCtrl &calenderCtrl,
[[maybe_unused]] wxArtProvider *artProvider,
[[maybe_unused]] wxCalendarCtrl &calenderCtrl,
wxComboCtrl &comboCtrl,
wxChar * path)
{
Expand Down
Loading
Loading