Skip to content

Commit 2151244

Browse files
chrchr-githubchrchr-github
authored
Fix #11499 FN functionConst with operator usage (#4722)
1 parent 3813616 commit 2151244

6 files changed

Lines changed: 142 additions & 29 deletions

File tree

‎lib/checkclass.cpp‎

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,11 +2291,37 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
22912291
auto getFuncTok = [](const Token* tok) -> const Token* {
22922292
if (Token::simpleMatch(tok, "this"))
22932293
tok = getFuncTokFromThis(tok);
2294-
if ((Token::Match(tok, "%name% (|{") || Token::simpleMatch(tok->astParent(), "return {")) && !tok->isStandardType() && !tok->isKeyword())
2294+
bool isReturn = false;
2295+
if ((Token::Match(tok, "%name% (|{") || (isReturn = Token::simpleMatch(tok->astParent(), "return {"))) && !tok->isStandardType() && !tok->isKeyword()) {
2296+
if (isReturn)
2297+
tok = tok->astParent();
22952298
return tok;
2299+
}
22962300
return nullptr;
22972301
};
22982302

2303+
auto checkFuncCall = [this, &memberAccessed](const Token* funcTok, const Scope* scope) {
2304+
if (isMemberFunc(scope, funcTok) && (funcTok->strAt(-1) != "." || Token::simpleMatch(funcTok->tokAt(-2), "this ."))) {
2305+
if (!isConstMemberFunc(scope, funcTok))
2306+
return false;
2307+
memberAccessed = true;
2308+
}
2309+
// Member variable given as parameter
2310+
const Token *lpar = funcTok->next();
2311+
if (Token::simpleMatch(lpar, "( ) ("))
2312+
lpar = lpar->tokAt(2);
2313+
for (const Token* tok = lpar->next(); tok && tok != funcTok->next()->link(); tok = tok->next()) {
2314+
if (tok->str() == "(")
2315+
tok = tok->link();
2316+
else if ((tok->isName() && isMemberVar(scope, tok)) || (tok->isUnaryOp("&") && (tok = tok->astOperand1()))) {
2317+
const Variable* var = tok->variable();
2318+
if (!var || !var->isMutable())
2319+
return false; // TODO: Only bailout if function takes argument as non-const reference
2320+
}
2321+
}
2322+
return true;
2323+
};
2324+
22992325
// if the function doesn't have any assignment nor function call,
23002326
// it can be a const function..
23012327
for (const Token *tok1 = func->functionScope->bodyStart; tok1 && tok1 != func->functionScope->bodyEnd; tok1 = tok1->next()) {
@@ -2374,6 +2400,18 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
23742400
break;
23752401
}
23762402

2403+
auto hasOverloadedMemberAccess = [](const Token* end, const Scope* scope) -> bool {
2404+
if (!end || !scope || !Token::simpleMatch(end->astParent(), "."))
2405+
return false;
2406+
auto it = std::find_if(scope->functionList.begin(), scope->functionList.end(), [](const Function& f) {
2407+
return f.isConst() && f.name() == "operator.";
2408+
});
2409+
if (it == scope->functionList.end() || !it->retType || !it->retType->classScope)
2410+
return false;
2411+
const Function* func = it->retType->classScope->findFunction(end, /*requireConst*/ true);
2412+
return func && func->isConst();
2413+
};
2414+
23772415
if (end->strAt(1) == "(") {
23782416
const Variable *var = lastVarTok->variable();
23792417
if (!var)
@@ -2389,6 +2427,9 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
23892427
;
23902428
else if (var->smartPointerType() && var->smartPointerType()->classScope && isConstMemberFunc(var->smartPointerType()->classScope, end)) {
23912429
;
2430+
}
2431+
else if (hasOverloadedMemberAccess(end, var->typeScope())) {
2432+
;
23922433
} else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end))
23932434
return false;
23942435
}
@@ -2416,8 +2457,8 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
24162457
return false;
24172458

24182459
tok1 = jumpBackToken?jumpBackToken:end; // Jump back to first [ to check inside, or jump to end of expression
2419-
if (tok1 == end && Token::Match(end->previous(), ". %name% ( !!)"))
2420-
tok1 = tok1->previous(); // check function call
2460+
if (tok1 == end && Token::Match(end->previous(), ". %name% ( !!)") && !checkFuncCall(tok1, scope)) // function call on member
2461+
return false;
24212462
}
24222463

24232464
// streaming: <<
@@ -2435,24 +2476,8 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
24352476

24362477
// function/constructor call, return init list
24372478
else if (const Token* funcTok = getFuncTok(tok1)) {
2438-
if (isMemberFunc(scope, funcTok) && (funcTok->strAt(-1) != "." || Token::simpleMatch(funcTok->tokAt(-2), "this ."))) {
2439-
if (!isConstMemberFunc(scope, funcTok))
2440-
return false;
2441-
memberAccessed = true;
2442-
}
2443-
// Member variable given as parameter
2444-
const Token *lpar = funcTok->next();
2445-
if (Token::simpleMatch(lpar, "( ) ("))
2446-
lpar = lpar->tokAt(2);
2447-
for (const Token* tok2 = lpar->next(); tok2 && tok2 != funcTok->next()->link(); tok2 = tok2->next()) {
2448-
if (tok2->str() == "(")
2449-
tok2 = tok2->link();
2450-
else if ((tok2->isName() && isMemberVar(scope, tok2)) || (tok2->isUnaryOp("&") && (tok2 = tok2->astOperand1()))) {
2451-
const Variable* var = tok2->variable();
2452-
if (!var || !var->isMutable())
2453-
return false; // TODO: Only bailout if function takes argument as non-const reference
2454-
}
2455-
}
2479+
if (!checkFuncCall(funcTok, scope))
2480+
return false;
24562481
} else if (Token::simpleMatch(tok1, "> (") && (!tok1->link() || !Token::Match(tok1->link()->previous(), "static_cast|const_cast|dynamic_cast|reinterpret_cast"))) {
24572482
return false;
24582483
}

‎lib/checkmemoryleak.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ bool CheckMemoryLeakInFunction::test_white_list(const std::string &funcname, con
452452
// a = malloc(10); a = realloc(a, 100);
453453
//---------------------------------------------------------------------------
454454

455-
void CheckMemoryLeakInFunction::checkReallocUsage()
455+
void CheckMemoryLeakInFunction::checkReallocUsage() const
456456
{
457457
// only check functions
458458
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();

‎lib/checkmemoryleak.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ class CPPCHECKLIB CheckMemoryLeakInFunction : private Check, public CheckMemoryL
183183
/**
184184
* Checking for a memory leak caused by improper realloc usage.
185185
*/
186-
void checkReallocUsage();
186+
void checkReallocUsage() const;
187187

188188
private:
189189
/** Report all possible errors (for the --errorlist) */

‎lib/forwardanalyzer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,15 +288,15 @@ struct ForwardTraversal {
288288
return ft;
289289
}
290290

291-
std::vector<ForwardTraversal> tryForkScope(Token* endBlock, bool isModified = false) {
291+
std::vector<ForwardTraversal> tryForkScope(Token* endBlock, bool isModified = false) const {
292292
if (analyzer->updateScope(endBlock, isModified)) {
293293
ForwardTraversal ft = fork();
294294
return {std::move(ft)};
295295
}
296296
return std::vector<ForwardTraversal> {};
297297
}
298298

299-
std::vector<ForwardTraversal> tryForkUpdateScope(Token* endBlock, bool isModified = false) {
299+
std::vector<ForwardTraversal> tryForkUpdateScope(Token* endBlock, bool isModified = false) const {
300300
std::vector<ForwardTraversal> result = tryForkScope(endBlock, isModified);
301301
for (ForwardTraversal& ft : result)
302302
ft.updateScope(endBlock);

‎lib/reverseanalyzer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ struct ReverseTraversal {
4242
ValuePtr<Analyzer> analyzer;
4343
const Settings* settings;
4444

45-
std::pair<bool, bool> evalCond(const Token* tok) {
45+
std::pair<bool, bool> evalCond(const Token* tok) const {
4646
std::vector<MathLib::bigint> result = analyzer->evaluate(tok);
4747
// TODO: We should convert to bool
4848
const bool checkThen = std::any_of(result.cbegin(), result.cend(), [](int x) {
@@ -142,7 +142,7 @@ struct ReverseTraversal {
142142
return result;
143143
}
144144

145-
Token* isDeadCode(Token* tok, const Token* end = nullptr) {
145+
Token* isDeadCode(Token* tok, const Token* end = nullptr) const {
146146
int opSide = 0;
147147
for (; tok && tok->astParent(); tok = tok->astParent()) {
148148
if (tok == end)

‎test/testclass.cpp‎

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6221,8 +6221,8 @@ class TestClass : public TestFixture {
62216221
errout.str());
62226222
}
62236223

6224-
void const81() { // #11330
6225-
checkConst("struct A {\n"
6224+
void const81() {
6225+
checkConst("struct A {\n" // #11330
62266226
" bool f() const;\n"
62276227
"};\n"
62286228
"struct S {\n"
@@ -6233,6 +6233,94 @@ class TestClass : public TestFixture {
62336233
"};\n");
62346234
ASSERT_EQUALS("[test.cpp:6]: (style, inconclusive) Technically the member function 'S::g' can be const.\n",
62356235
errout.str());
6236+
6237+
checkConst("struct A {\n" // #11499
6238+
" void f() const;\n"
6239+
"};\n"
6240+
"template<class T>\n"
6241+
"struct P {\n"
6242+
" T* operator->();\n"
6243+
" const T* operator->() const;\n"
6244+
"};\n"
6245+
"struct S {\n"
6246+
" P<A> p;\n"
6247+
" void g() { p->f(); }\n"
6248+
"};\n");
6249+
ASSERT_EQUALS("[test.cpp:11]: (style, inconclusive) Technically the member function 'S::g' can be const.\n",
6250+
errout.str());
6251+
6252+
checkConst("struct A {\n"
6253+
" void f(int) const;\n"
6254+
"};\n"
6255+
"template<class T>\n"
6256+
"struct P {\n"
6257+
" T* operator->();\n"
6258+
" const T* operator->() const;\n"
6259+
"};\n"
6260+
"struct S {\n"
6261+
" P<A> p;\n"
6262+
" void g() { p->f(1); }\n"
6263+
"};\n");
6264+
ASSERT_EQUALS("[test.cpp:11]: (style, inconclusive) Technically the member function 'S::g' can be const.\n", errout.str());
6265+
6266+
checkConst("struct A {\n"
6267+
" void f(void*) const;\n"
6268+
"};\n"
6269+
"template<class T>\n"
6270+
"struct P {\n"
6271+
" T* operator->();\n"
6272+
" const T* operator->() const;\n"
6273+
" P<T>& operator=(P) {\n"
6274+
" return *this;\n"
6275+
" }\n"
6276+
"};\n"
6277+
"struct S {\n"
6278+
" P<A> p;\n"
6279+
" std::vector<S> g() { p->f(nullptr); return {}; }\n"
6280+
"};\n");
6281+
ASSERT_EQUALS("[test.cpp:14]: (style, inconclusive) Technically the member function 'S::g' can be const.\n", errout.str());
6282+
6283+
checkConst("struct A {\n"
6284+
" void f();\n"
6285+
"};\n"
6286+
"template<class T>\n"
6287+
"struct P {\n"
6288+
" T* operator->();\n"
6289+
" const T* operator->() const;\n"
6290+
"};\n"
6291+
"struct S {\n"
6292+
" P<A> p;\n"
6293+
" void g() { p->f(); }\n"
6294+
"};\n");
6295+
ASSERT_EQUALS("", errout.str());
6296+
6297+
checkConst("struct A {\n"
6298+
" void f() const;\n"
6299+
"};\n"
6300+
"template<class T>\n"
6301+
"struct P {\n"
6302+
" T* operator->();\n"
6303+
"};\n"
6304+
"struct S {\n"
6305+
" P<A> p;\n"
6306+
" void g() { p->f(); }\n"
6307+
"};\n");
6308+
ASSERT_EQUALS("", errout.str());
6309+
6310+
checkConst("struct A {\n"
6311+
" void f(int&) const;\n"
6312+
"};\n"
6313+
"template<class T>\n"
6314+
"struct P {\n"
6315+
" T* operator->();\n"
6316+
" const T* operator->() const;\n"
6317+
"};\n"
6318+
"struct S {\n"
6319+
" P<A> p;\n"
6320+
" int i;\n"
6321+
" void g() { p->f(i); }\n"
6322+
"};\n");
6323+
ASSERT_EQUALS("", errout.str());
62366324
}
62376325

62386326
void const_handleDefaultParameters() {

0 commit comments

Comments
 (0)