Skip to content

Fix #9861 FN: "static" not suggested for non-const method #4101

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
orbitcowboy merged 4 commits into from
May 11, 2022
Merged

Fix #9861 FN: "static" not suggested for non-const method #4101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1ff8d38
Fix #9861 FN: "static" not suggested for non-const method
chrchr-github May 11, 2022
71bdd48
Format
chrchr-github May 11, 2022
31bac45
Make functions static
chrchr-github May 11, 2022
625fc39
static
chrchr-github May 11, 2022
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
59 changes: 28 additions & 31 deletions lib/checkclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1988,37 +1988,33 @@ void CheckClass::checkConst()
if (func.isFriend() || func.isStatic() || func.hasVirtualSpecifier())
continue;

// don't warn when returning non-const pointer/reference
{
auto isPointerOrReference = [this](const Token* start, const Token* end) -> bool {
bool inTemplArgList = false, isConstTemplArg = false;
for (const Token* tok = start; tok != end; tok = tok->next()) {
if (tok->str() == "{") // end of trailing return type
// don't suggest const when returning non-const pointer/reference, but still suggest static
auto isPointerOrReference = [this](const Token* start, const Token* end) -> bool {
bool inTemplArgList = false, isConstTemplArg = false;
for (const Token* tok = start; tok != end; tok = tok->next()) {
if (tok->str() == "{") // end of trailing return type
return false;
if (tok->str() == "<") {
if (!tok->link())
mSymbolDatabase->debugMessage(tok, "debug", "CheckClass::checkConst found unlinked template argument list '" + tok->expressionString() + "'.");
inTemplArgList = true;
}
else if (tok->str() == ">") {
inTemplArgList = false;
isConstTemplArg = false;
}
else if (tok->str() == "const") {
if (!inTemplArgList)
return false;
if (tok->str() == "<") {
if (!tok->link())
mSymbolDatabase->debugMessage(tok, "debug", "CheckClass::checkConst found unlinked template argument list '" + tok->expressionString() + "'.");
inTemplArgList = true;
}
else if (tok->str() == ">") {
inTemplArgList = false;
isConstTemplArg = false;
}
else if (tok->str() == "const") {
if (!inTemplArgList)
return false;
isConstTemplArg = true;
}
else if (!isConstTemplArg && Token::Match(tok, "*|&"))
return true;
isConstTemplArg = true;
}
return false;
};

if (isPointerOrReference(func.retDef, func.tokenDef))
continue;
}
else if (!isConstTemplArg && Token::Match(tok, "*|&"))
return true;
}
return false;
};

const bool returnsPtrOrRef = isPointerOrReference(func.retDef, func.tokenDef);

if (func.isOperator()) { // Operator without return type: conversion operator
const std::string& opName = func.tokenDef->str();
Expand All @@ -2043,7 +2039,8 @@ void CheckClass::checkConst()
if (!checkConstFunc(scope, &func, memberAccessed))
continue;

if (func.isConst() && (memberAccessed || func.isOperator()))
const bool suggestStatic = !memberAccessed && !func.isOperator();
if ((returnsPtrOrRef || func.isConst()) && !suggestStatic)
continue;

std::string classname = scope->className;
Expand All @@ -2062,9 +2059,9 @@ void CheckClass::checkConst()
functionName += "]";

if (func.isInline())
checkConstError(func.token, classname, functionName, !memberAccessed && !func.isOperator());
checkConstError(func.token, classname, functionName, suggestStatic);
else // not inline
checkConstError2(func.token, func.tokenDef, classname, functionName, !memberAccessed && !func.isOperator());
checkConstError2(func.token, func.tokenDef, classname, functionName, suggestStatic);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/reverseanalyzer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct ReverseTraversal {
return true;
}

Token* getParentFunction(Token* tok)
static Token* getParentFunction(Token* tok)
{
if (!tok)
return nullptr;
Expand All @@ -90,7 +90,7 @@ struct ReverseTraversal {
return nullptr;
}

Token* getTopFunction(Token* tok)
static Token* getTopFunction(Token* tok)
{
if (!tok)
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion lib/tokenize.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ class CPPCHECKLIB Tokenizer {
* '; int *p(0);' => '; int *p = 0;'
*/
void simplifyInitVar();
Token * initVar(Token * tok);
static Token* initVar(Token* tok);

/**
* Simplify easy constant '?:' operation
Expand Down
12 changes: 12 additions & 0 deletions test/testclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ class TestClass : public TestFixture {
TEST_CASE(const76); // ticket #10825
TEST_CASE(const77); // ticket #10307, #10311
TEST_CASE(const78); // ticket #10315
TEST_CASE(const79); // ticket #9861
TEST_CASE(const_handleDefaultParameters);
TEST_CASE(const_passThisToMemberOfOtherClass);
TEST_CASE(assigningPointerToPointerIsNotAConstOperation);
Expand Down Expand Up @@ -6068,6 +6069,17 @@ class TestClass : public TestFixture {
ASSERT_EQUALS("", errout.str());
}

void const79() { // #9861
checkConst("class A {\n"
"public:\n"
" char* f() {\n"
" return nullptr;\n"
" }\n"
"};\n");
ASSERT_EQUALS("[test.cpp:3]: (performance, inconclusive) Technically the member function 'A::f' can be static (but you may consider moving to unnamed namespace).\n",
errout.str());
}

void const_handleDefaultParameters() {
checkConst("struct Foo {\n"
" void foo1(int i, int j = 0) {\n"
Expand Down