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

Partial fix for #11131 FN variableScope with const member functions (#5027)

This commit is contained in:
chrchr-github 2023-05-03 10:02:16 +02:00 committed by GitHub
parent 7e0ddd3669
commit ec2a2ad41f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 74 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/checkother.cpp
View File

@ -855,6 +855,21 @@ void CheckOther::redundantContinueError(const Token *tok)
"'continue' is redundant since it is the last statement in a loop.", CWE561, Certainty::normal);
}
static bool isSimpleExpr(const Token* tok, const Variable* var, const Settings* settings) {
if (!tok)
return false;
if (tok->isNumber() || tok->tokType() == Token::eString || tok->tokType() == Token::eChar || tok->isBoolean())
return true;
bool needsCheck = tok->varId() > 0;
if (!needsCheck) {
const Token* ftok = tok->previous();
if (Token::Match(ftok, "%name% (") &&
((ftok->function() && ftok->function()->isConst()) || settings->library.isFunctionConst(ftok->str(), /*pure*/ true)))
needsCheck = true;
}
return (needsCheck && !isExpressionChanged(tok, tok->astParent(), var->scope()->bodyEnd, settings, true));
};
//---------------------------------------------------------------------------
// Check scope of variables..
//---------------------------------------------------------------------------
@ -906,14 +921,10 @@ void CheckOther::checkVariableScope()
if (forHead)
continue;
auto isSimpleExpr = [](const Token* tok) {
return tok && (tok->isNumber() || tok->tokType() == Token::eString || tok->tokType() == Token::eChar || tok->isBoolean());
};
const Token* tok = var->nameToken()->next();
if (Token::Match(tok, "; %varid% = %any% ;", var->declarationId())) { // bailout for assignment
tok = tok->tokAt(3);
if (!isSimpleExpr(tok))
if (Token::Match(tok, "; %varid% =", var->declarationId())) { // bailout for assignment
tok = tok->tokAt(2)->astOperand2();
if (!isSimpleExpr(tok, var, mSettings))
continue;
}
else if (Token::Match(tok, "{|(")) { // bailout for constructor
@ -921,11 +932,11 @@ void CheckOther::checkVariableScope()
bool bail = false;
while (argTok) {
if (Token::simpleMatch(argTok, ",")) {
if (!isSimpleExpr(argTok->astOperand2())) {
if (!isSimpleExpr(argTok->astOperand2(), var, mSettings)) {
bail = true;
break;
}
} else if (!isSimpleExpr(argTok)) {
} else if (argTok->str() != "." && !isSimpleExpr(argTok, var, mSettings)) {
bail = true;
break;
}

5
lib/checkstl.cpp
View File

@ -1551,13 +1551,12 @@ static std::pair<const Token *, const Token *> isMapFind(const Token *tok)
return {contTok, tok->astOperand2()};
}
static const Token *skipLocalVars(const Token *tok)
static const Token* skipLocalVars(const Token* const tok)
{
if (!tok)
return tok;
if (Token::simpleMatch(tok, "{"))
return skipLocalVars(tok->next());
const Scope *scope = tok->scope();
const Token *top = tok->astTop();
if (!top) {
@ -1579,7 +1578,7 @@ static const Token *skipLocalVars(const Token *tok)
const Variable *var = varTok->variable();
if (!var)
return tok;
if (var->scope() != scope)
if (var->scope() != tok->scope())
return tok;
const Token *endTok = nextAfterAstRightmostLeaf(top);
if (!endTok)

2
lib/programmemory.cpp
View File

@ -356,7 +356,6 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok
tok2->astOperand2()) {
bool setvar = false;
const Token* vartok = tok2->astOperand1();
const Token* valuetok = tok2->astOperand2();
for (const auto& p:vars) {
if (p.first != vartok->exprId())
continue;
@ -367,6 +366,7 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok
}
if (!setvar) {
if (!pm.hasValue(vartok->exprId())) {
const Token* valuetok = tok2->astOperand2();
pm.setValue(vartok, execute(valuetok, pm));
}
}

52
test/testother.cpp
View File

@ -104,6 +104,7 @@ private:
TEST_CASE(varScope30); // #8541
TEST_CASE(varScope31); // #11099
TEST_CASE(varScope32); // #11441
TEST_CASE(varScope33);
TEST_CASE(oldStylePointerCast);
TEST_CASE(invalidPointerCast);
@ -1563,6 +1564,53 @@ private:
ASSERT_EQUALS("[test.cpp:6]: (warning) Unused variable value 'x'\n", errout.str());
}
void varScope33() { // #11131
check("struct S {\n"
" const std::string& getStr() const;\n"
" void mutate();\n"
" bool getB() const;\n"
"};\n"
"void g(S& s) {\n"
" std::string str = s.getStr();\n"
" s.mutate();\n"
" if (s.getB()) {\n"
" if (str == \"abc\") {}\n"
" }\n"
"}\n"
"void g(char* s, bool b) {\n"
" int i = strlen(s);\n"
" s[0] = '\\0';\n"
" if (b) {\n"
" if (i == 5) {}\n"
" }\n"
"}\n"
"void f(const S& s) {\n"
" std::string str = s.getStr();\n"
" std::string str2{ s.getStr() };\n"
" if (s.getB()) {\n"
" if (str == \"abc\") {}\n"
" if (str2 == \"abc\") {}\n"
" }\n"
"}\n"
"void f(const char* s, bool b) {\n"
" int i = strlen(s);\n"
" if (b) {\n"
" if (i == 5) {}\n"
" }\n"
"}\n"
"void f(int j, bool b) {\n"
" int k = j;\n"
" if (b) {\n"
" if (k == 5) {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:21]: (style) The scope of the variable 'str' can be reduced.\n"
"[test.cpp:22]: (style) The scope of the variable 'str2' can be reduced.\n"
"[test.cpp:29]: (style) The scope of the variable 'i' can be reduced.\n"
"[test.cpp:35]: (style) The scope of the variable 'k' can be reduced.\n",
errout.str());
}
#define checkOldStylePointerCast(code) checkOldStylePointerCast_(code, __FILE__, __LINE__)
void checkOldStylePointerCast_(const char code[], const char* file, int line) {
// Clear the error buffer..
@ -5694,7 +5742,9 @@ private:
" x = j;\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5] -> [test.cpp:3]: (style, inconclusive) Found duplicate branches for 'if' and 'else'.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5] -> [test.cpp:3]: (style, inconclusive) Found duplicate branches for 'if' and 'else'.\n"
"[test.cpp:2]: (style) The scope of the variable 'j' can be reduced.\n",
errout.str());
check("void f(bool b, int i) {\n"
" int j = i;\n"