walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

10671 fix const iterator function const take two (#3802) 

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

This reverts commit 9f6a36c1a8.

* Fix crash by adding missing null check
This commit is contained in:
Rikard Falkeborn 2022-02-11 21:23:23 +01:00 committed by GitHub
parent c690bfb03a
commit d4cd249c21
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 80 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
lib/checkclass.cpp
View File

@ -2214,8 +2214,14 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
const Variable *var = lastVarTok->variable();
if (!var)
return false;
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")))
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"))) ||
(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() && tok1->tokAt(-2)->variable() && Token::Match(tok1->tokAt(-2)->variable()->typeEndToken(), "const_iterator|const_reverse_iterator")))))
;
else if (!var->typeScope() || !isConstMemberFunc(var->typeScope(), end))
return false;

43
test/cfg/std.cpp
View File

@ -3681,3 +3681,46 @@ void stdbind()
// cppcheck-suppress unreadVariable
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) {;}
}
};

29
test/testclass.cpp
View File

@ -53,6 +53,12 @@ private:
" <dealloc>free</dealloc>\n"
" </memory>\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>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
@ -183,6 +189,7 @@ private:
TEST_CASE(const71); // ticket #10146
TEST_CASE(const72); // ticket #10520
TEST_CASE(const73); // ticket #10735
TEST_CASE(const74); // ticket #10671
TEST_CASE(const_handleDefaultParameters);
TEST_CASE(const_passThisToMemberOfOtherClass);
TEST_CASE(assigningPointerToPointerIsNotAConstOperation);
@ -5918,6 +5925,28 @@ private:
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());
// Don't crash
checkConst("struct S {\n"
" std::vector<T*> v;\n"
" void f() const;\n"
"};\n"
"void S::f() const {\n"
" for (std::vector<T*>::const_iterator it = v.begin(), end = v.end(); it != end; ++it) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void const_handleDefaultParameters() {
checkConst("struct Foo {\n"
" void foo1(int i, int j = 0) {\n"