Fix #10671: functionConst FN with begin/end and const_iterator (#3749)

Check if the iterator is assigned to a const_iterator or
const_revese_iterator, in which case it is possible the function can be
const. Unfortunately, it is not possible to remove the hard coding of
cbegin, cend, crbegin and crend due to the need to handle auto, as in
the following code snippet:

    void cbegin_auto(void)
    {
        for (auto it = m_str.cbegin(); it != m_str.cend(); ++it) {;}
    }
This commit is contained in:
Rikard Falkeborn 2022-02-02 19:37:06 +01:00 committed by GitHub
parent dad64bfcc8
commit 2cacb13f85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 69 additions and 2 deletions

View File

@ -2205,8 +2205,13 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
const Variable *var = lastVarTok->variable(); const Variable *var = lastVarTok->variable();
if (!var) if (!var)
return false; return false;
if (var->isStlType() // assume all std::*::size() and std::*::empty() are const if ((var->isStlType() // assume all std::*::size() and std::*::empty() are const
&& (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy"))) && (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy"))) ||
(lastVarTok->valueType() && lastVarTok->valueType()->container &&
((lastVarTok->valueType()->container->getYield(end->str()) == Library::Container::Yield::START_ITERATOR) ||
(lastVarTok->valueType()->container->getYield(end->str()) == Library::Container::Yield::END_ITERATOR))
&& (tok1->previous()->isComparisonOp() || (tok1->previous()->isAssignmentOp() && Token::Match(tok1->tokAt(-2)->variable()->typeEndToken(), "const_iterator|const_reverse_iterator")))))
; ;
else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end)) else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end))
return false; return false;

View File

@ -3636,3 +3636,46 @@ void stdbind()
// cppcheck-suppress unreadVariable // cppcheck-suppress unreadVariable
auto f2 = std::bind(stdbind_helper, 10); auto f2 = std::bind(stdbind_helper, 10);
} }
class A
{
std::vector<std::string> m_str;
public:
A() {}
// cppcheck-suppress functionConst
void begin_const_iterator(void)
{
for (std::vector<std::string>::const_iterator it = m_str.begin(); it != m_str.end(); ++it) {;}
}
// cppcheck-suppress functionConst
void cbegin_const_iterator(void)
{
for (std::vector<std::string>::const_iterator it = m_str.cbegin(); it != m_str.cend(); ++it) {;}
}
// cppcheck-suppress functionConst
void crbegin_const_iterator(void)
{
for (std::vector<std::string>::const_reverse_iterator it = m_str.crbegin(); it != m_str.crend(); ++it) {;}
}
// cppcheck-suppress functionConst
void rbegin_const_iterator(void)
{
for (std::vector<std::string>::const_reverse_iterator it = m_str.rbegin(); it != m_str.rend(); ++it) {;}
}
// cppcheck-suppress functionConst
void cbegin_auto(void)
{
for (auto it = m_str.cbegin(); it != m_str.cend(); ++it) {;}
}
void baz_begin_no_const_iterator(void)
{
for (std::vector<std::string>::iterator it = m_str.begin(); it != m_str.end(); ++it) {;}
}
void rbegin_no_const_iterator(void)
{
for (std::vector<std::string>::reverse_iterator it = m_str.rbegin(); it != m_str.rend(); ++it) {;}
}
};

View File

@ -54,6 +54,12 @@ private:
" <dealloc>free</dealloc>\n" " <dealloc>free</dealloc>\n"
" </memory>\n" " </memory>\n"
" <smart-pointer class-name=\"std::shared_ptr\"/>\n" " <smart-pointer class-name=\"std::shared_ptr\"/>\n"
" <container id=\"stdVector\" startPattern=\"std :: vector &lt;\" itEndPattern=\"&gt; :: const_iterator\">\n"
" <access>\n"
" <function name=\"begin\" yields=\"start-iterator\"/>\n"
" <function name=\"end\" yields=\"end-iterator\"/>\n"
" </access>\n"
" </container>\n"
"</def>"; "</def>";
tinyxml2::XMLDocument doc; tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata)); doc.Parse(xmldata, sizeof(xmldata));
@ -184,6 +190,7 @@ private:
TEST_CASE(const71); // ticket #10146 TEST_CASE(const71); // ticket #10146
TEST_CASE(const72); // ticket #10520 TEST_CASE(const72); // ticket #10520
TEST_CASE(const73); // ticket #10735 TEST_CASE(const73); // ticket #10735
TEST_CASE(const74); // ticket #10671
TEST_CASE(const_handleDefaultParameters); TEST_CASE(const_handleDefaultParameters);
TEST_CASE(const_passThisToMemberOfOtherClass); TEST_CASE(const_passThisToMemberOfOtherClass);
TEST_CASE(assigningPointerToPointerIsNotAConstOperation); TEST_CASE(assigningPointerToPointerIsNotAConstOperation);
@ -5919,6 +5926,18 @@ private:
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (style, inconclusive) Technically the member function 'S::f' can be const.\n", errout.str()); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (style, inconclusive) Technically the member function 'S::f' can be const.\n", errout.str());
} }
void const74() { // #10671
checkConst("class A {\n"
" std::vector<std::string> m_str;\n"
"public:\n"
" A() {}\n"
" void bar(void) {\n"
" for(std::vector<std::string>::const_iterator it = m_str.begin(); it != m_str.end(); ++it) {;}\n"
" }\n"
"};");
ASSERT_EQUALS("[test.cpp:5]: (style, inconclusive) Technically the member function 'A::bar' can be const.\n", errout.str());
}
void const_handleDefaultParameters() { void const_handleDefaultParameters() {
checkConst("struct Foo {\n" checkConst("struct Foo {\n"
" void foo1(int i, int j = 0) {\n" " void foo1(int i, int j = 0) {\n"