Skip to content

Commit aad6e09

Browse files
Partial fix for #13301 false negative: functionConst with const/non-const overloads (danmar#7003)
1 parent d6e7f40 commit aad6e09

File tree

2 files changed

+65
-8
lines changed

2 files changed

+65
-8
lines changed

lib/checkclass.cpp

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2537,18 +2537,35 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, Member
25372537
return func && func->isConst();
25382538
};
25392539

2540+
auto isConstContainerUsage = [&](const ValueType* vt) -> bool {
2541+
if (!vt || !vt->container)
2542+
return false;
2543+
const auto yield = vt->container->getYield(end->str());
2544+
if (yield == Library::Container::Yield::START_ITERATOR || yield == Library::Container::Yield::END_ITERATOR) {
2545+
const Token* parent = tok1->astParent();
2546+
while (Token::Match(parent, "(|.|::"))
2547+
parent = parent->astParent();
2548+
if (parent && parent->isComparisonOp())
2549+
return true;
2550+
// TODO: use AST
2551+
if (parent && parent->isAssignmentOp() && tok1->tokAt(-2)->variable() && Token::Match(tok1->tokAt(-2)->variable()->typeEndToken(), "const_iterator|const_reverse_iterator"))
2552+
return true;
2553+
}
2554+
if ((yield == Library::Container::Yield::ITEM || yield == Library::Container::Yield::AT_INDEX) &&
2555+
(lhs->isComparisonOp() || lhs->isAssignmentOp() || (lhs->str() == "(" && Token::Match(lhs->astParent(), "%cop%"))))
2556+
return true; // assume that these functions have const overloads
2557+
return false;
2558+
};
2559+
25402560
if (end->strAt(1) == "(") {
25412561
const Variable *var = lastVarTok->variable();
25422562
if (!var)
25432563
return false;
2544-
if ((var->isStlType() // assume all std::*::size() and std::*::empty() are const
2545-
&& (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy"))) ||
2546-
2547-
(lastVarTok->valueType() && lastVarTok->valueType()->container &&
2548-
((lastVarTok->valueType()->container->getYield(end->str()) == Library::Container::Yield::START_ITERATOR) ||
2549-
(lastVarTok->valueType()->container->getYield(end->str()) == Library::Container::Yield::END_ITERATOR))
2550-
&& (tok1->previous()->isComparisonOp() ||
2551-
(tok1->previous()->isAssignmentOp() && tok1->tokAt(-2)->variable() && Token::Match(tok1->tokAt(-2)->variable()->typeEndToken(), "const_iterator|const_reverse_iterator"))))) {
2564+
if (var->isStlType() // assume all std::*::size() and std::*::empty() are const
2565+
&& (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy"))) {
2566+
// empty body
2567+
}
2568+
else if (isConstContainerUsage(lastVarTok->valueType())) {
25522569
// empty body
25532570
}
25542571
else if (var->smartPointerType() && var->smartPointerType()->classScope && isConstMemberFunc(var->smartPointerType()->classScope, end)) {

test/testclass.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ class TestClass : public TestFixture {
188188
TEST_CASE(const94);
189189
TEST_CASE(const95); // #13320 - do not warn about r-value ref method
190190
TEST_CASE(const96);
191+
TEST_CASE(const97);
191192

192193
TEST_CASE(const_handleDefaultParameters);
193194
TEST_CASE(const_passThisToMemberOfOtherClass);
@@ -6723,6 +6724,45 @@ class TestClass : public TestFixture {
67236724
ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Either there is a missing 'override', or the member function 'S::f' can be const.\n", errout_str());
67246725
}
67256726

6727+
void const97() { // #13301
6728+
checkConst("struct S {\n"
6729+
" std::vector<int> v;\n"
6730+
" int f() {\n"
6731+
" const int& r = v.front();\n"
6732+
" return r;\n"
6733+
" }\n"
6734+
" int g() {\n"
6735+
" const int& r = v.at(0);\n"
6736+
" return r;\n"
6737+
" }\n"
6738+
" void h() {\n"
6739+
" if (v.front() == 0) {}\n"
6740+
" if (1 == v.front()) {}\n"
6741+
" }\n"
6742+
" void i() {\n"
6743+
" v.at(0) = 0;\n"
6744+
" }\n"
6745+
" void j() {\n"
6746+
" dostuff(1, v.at(0));\n"
6747+
" }\n"
6748+
" void k() {\n"
6749+
" int& r = v.front();\n"
6750+
" r = 0;\n"
6751+
" }\n"
6752+
"};\n");
6753+
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Technically the member function 'S::f' can be const.\n"
6754+
"[test.cpp:7]: (style, inconclusive) Technically the member function 'S::g' can be const.\n"
6755+
"[test.cpp:11]: (style, inconclusive) Technically the member function 'S::h' can be const.\n",
6756+
errout_str());
6757+
6758+
checkConst("struct B { std::string s; };\n"
6759+
"struct D : B {\n"
6760+
" bool f(std::string::iterator it) { return it == B::s.begin(); }\n"
6761+
"};\n");
6762+
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Technically the member function 'D::f' can be const.\n",
6763+
errout_str());
6764+
}
6765+
67266766
void const_handleDefaultParameters() {
67276767
checkConst("struct Foo {\n"
67286768
" void foo1(int i, int j = 0) {\n"

0 commit comments

Comments
 (0)