Skip to content

Commit 9ed21fb

Browse files
chrchr-githubchrchr-github
authored
Fix #11513 FN functionConst with comparison as argument (#4738)
1 parent 8094306 commit 9ed21fb

7 files changed

Lines changed: 48 additions & 14 deletions

File tree

‎lib/checkclass.cpp‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2306,16 +2306,29 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, bool&
23062306
return false;
23072307
memberAccessed = true;
23082308
}
2309+
bool mayModifyArgs = true;
2310+
if (const Function* f = funcTok->function()) { // TODO: improve (we bail out if there is any possible modification of any argument)
2311+
const std::vector<const Token*> args = getArguments(funcTok);
2312+
const auto argMax = std::min<nonneg int>(args.size(), f->argCount());
2313+
mayModifyArgs = false;
2314+
for (nonneg int argIndex = 0; argIndex < argMax; ++argIndex) {
2315+
const Variable* const argVar = f->getArgumentVar(argIndex);
2316+
if (!argVar || !argVar->valueType() || ((argVar->valueType()->pointer || argVar->valueType()->reference != Reference::None) && !argVar->isConst())) {
2317+
mayModifyArgs = true;
2318+
break;
2319+
}
2320+
}
2321+
}
23092322
// Member variable given as parameter
23102323
const Token *lpar = funcTok->next();
23112324
if (Token::simpleMatch(lpar, "( ) ("))
23122325
lpar = lpar->tokAt(2);
23132326
for (const Token* tok = lpar->next(); tok && tok != funcTok->next()->link(); tok = tok->next()) {
23142327
if (tok->str() == "(")
23152328
tok = tok->link();
2316-
else if ((tok->isName() && isMemberVar(scope, tok)) || (tok->isUnaryOp("&") && (tok = tok->astOperand1()))) {
2329+
else if ((tok->isName() && isMemberVar(scope, tok)) || (tok->isUnaryOp("&") && (tok = tok->astOperand1()) && isMemberVar(scope, tok))) {
23172330
const Variable* var = tok->variable();
2318-
if (!var || !var->isMutable())
2331+
if ((!var || !var->isMutable()) && mayModifyArgs)
23192332
return false; // TODO: Only bailout if function takes argument as non-const reference
23202333
}
23212334
}

‎lib/cppcheck.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
145145
bool isUnusedFunctionCheckEnabled() const;
146146

147147
/** Remove *.ctu-info files */
148-
void removeCtuInfoFiles(const std::map<std::string, std::size_t>& files);
148+
void removeCtuInfoFiles(const std::map<std::string, std::size_t>& files); // cppcheck-suppress functionConst // has side effects
149149

150150
private:
151151
/** Are there "simple" rules */

‎lib/symboldatabase.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,12 +1474,12 @@ class CPPCHECKLIB SymbolDatabase {
14741474
void createSymbolDatabaseNeedInitialization();
14751475
void createSymbolDatabaseVariableSymbolTable();
14761476
void createSymbolDatabaseSetScopePointers();
1477-
void createSymbolDatabaseSetFunctionPointers(bool firstPass);
1477+
void createSymbolDatabaseSetFunctionPointers(bool firstPass); // cppcheck-suppress functionConst // has side effects
14781478
void createSymbolDatabaseSetVariablePointers();
14791479
// cppcheck-suppress functionConst
14801480
void createSymbolDatabaseSetTypePointers();
14811481
void createSymbolDatabaseSetSmartPointerType();
1482-
void createSymbolDatabaseEnums();
1482+
void createSymbolDatabaseEnums(); // cppcheck-suppress functionConst // has side effects
14831483
void createSymbolDatabaseEscapeFunctions();
14841484
// cppcheck-suppress functionConst
14851485
void createSymbolDatabaseIncompleteVars();

‎lib/tokenize.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ class CPPCHECKLIB Tokenizer {
586586

587587
void unsupportedTypedef(const Token *tok) const;
588588

589-
void setVarIdClassDeclaration(const Token * const startToken,
589+
void setVarIdClassDeclaration(const Token * const startToken, // cppcheck-suppress functionConst // has side effects
590590
VariableMap &variableMap,
591591
const nonneg int scopeStartVarId,
592592
std::map<nonneg int, std::map<std::string, nonneg int>>& structMembers);
@@ -595,7 +595,7 @@ class CPPCHECKLIB Tokenizer {
595595
std::map<nonneg int, std::map<std::string, nonneg int>>& structMembers,
596596
nonneg int &varId) const;
597597

598-
void setVarIdClassFunction(const std::string &classname,
598+
void setVarIdClassFunction(const std::string &classname, // cppcheck-suppress functionConst // has side effects
599599
Token * const startToken,
600600
const Token * const endToken,
601601
const std::map<std::string, nonneg int> &varlist,

‎test/testclass.cpp‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,9 @@ class TestClass : public TestFixture {
195195
TEST_CASE(const79); // ticket #9861
196196
TEST_CASE(const80); // ticket #11328
197197
TEST_CASE(const81); // ticket #11330
198-
TEST_CASE(const82);
198+
TEST_CASE(const82); // ticket #11513
199+
TEST_CASE(const83);
200+
199201
TEST_CASE(const_handleDefaultParameters);
200202
TEST_CASE(const_passThisToMemberOfOtherClass);
201203
TEST_CASE(assigningPointerToPointerIsNotAConstOperation);
@@ -6322,7 +6324,26 @@ class TestClass : public TestFixture {
63226324
ASSERT_EQUALS("", errout.str());
63236325
}
63246326

6325-
void const82() {
6327+
void const82() { // #11513
6328+
checkConst("struct S {\n"
6329+
" int i;\n"
6330+
" void h(bool) const;\n"
6331+
" void g() { h(i == 1); }\n"
6332+
"};\n");
6333+
ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Technically the member function 'S::g' can be const.\n",
6334+
errout.str());
6335+
6336+
checkConst("struct S {\n"
6337+
" int i;\n"
6338+
" void h(int, int*) const;\n"
6339+
" void g() { int a; h(i, &a); }\n"
6340+
"};\n");
6341+
TODO_ASSERT_EQUALS("[test.cpp:4]: (style, inconclusive) Technically the member function 'S::g' can be const.\n",
6342+
"",
6343+
errout.str());
6344+
}
6345+
6346+
void const83() {
63266347
checkConst("struct S {\n"
63276348
" int i1, i2;\n"
63286349
" void f(bool b);\n"

‎test/testsuppressions.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ class TestSuppressions : public TestFixture {
561561
ASSERT_EQUALS(true, suppressions6.isSuppressed(errorMessage("abc", "test.cpp", 123)));
562562
}
563563

564-
void inlinesuppress() {
564+
void inlinesuppress() const {
565565
Suppressions::Suppression s;
566566
std::string msg;
567567
ASSERT_EQUALS(false, s.parseComment("/* some text */", &msg));
@@ -593,7 +593,7 @@ class TestSuppressions : public TestFixture {
593593
ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: a\n", errout.str());
594594
}
595595

596-
void inlinesuppress_comment() {
596+
void inlinesuppress_comment() const {
597597
Suppressions::Suppression s;
598598
std::string errMsg;
599599
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc ; some comment", &errMsg));
@@ -604,7 +604,7 @@ class TestSuppressions : public TestFixture {
604604
ASSERT_EQUALS("", errMsg);
605605
}
606606

607-
void multi_inlinesuppress() {
607+
void multi_inlinesuppress() const {
608608
std::vector<Suppressions::Suppression> suppressions;
609609
std::string errMsg;
610610

@@ -669,7 +669,7 @@ class TestSuppressions : public TestFixture {
669669
ASSERT_EQUALS(false, errMsg.empty());
670670
}
671671

672-
void multi_inlinesuppress_comment() {
672+
void multi_inlinesuppress_comment() const {
673673
std::vector<Suppressions::Suppression> suppressions;
674674
std::string errMsg;
675675

‎test/testsymboldatabase.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7171,7 +7171,7 @@ class TestSymbolDatabase : public TestFixture {
71717171
ASSERT(tok->tokAt(2)->function());
71727172
}
71737173

7174-
void valueTypeMatchParameter() {
7174+
void valueTypeMatchParameter() const {
71757175
ValueType vt_int(ValueType::Sign::SIGNED, ValueType::Type::INT, 0);
71767176
ValueType vt_const_int(ValueType::Sign::SIGNED, ValueType::Type::INT, 0, 1);
71777177
ASSERT_EQUALS((int)ValueType::MatchResult::SAME, (int)ValueType::matchParameter(&vt_int, &vt_int));

0 commit comments

Comments
 (0)