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

Merge pull request #2709 from pfultz2/fp-duplicate-cond-this 

Fix FP of duplicateCondition when modifying the this variable
This commit is contained in:
Daniel Marjamäki 2020-07-13 21:14:58 +02:00 committed by GitHub
parent b2f463dd65 519f2a537a
commit e0be224f4e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 83 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
lib/astutils.cpp
View File

@ -552,7 +552,10 @@ bool exprDependsOnThis(const Token* expr, nonneg int depth)
// calling nonstatic method?
if (Token::Match(expr->previous(), "!!:: %name% (") && expr->function() && expr->function()->nestedIn && expr->function()->nestedIn->isClassOrStruct()) {
// is it a method of this?
const Scope *nestedIn = expr->scope()->functionOf;
const Scope* fScope = expr->scope();
while (!fScope->functionOf && fScope->nestedIn)
fScope = fScope->nestedIn;
const Scope* nestedIn = fScope->functionOf;
if (nestedIn && nestedIn->function)
nestedIn = nestedIn->function->token->scope();
while (nestedIn && nestedIn != expr->function()->nestedIn) {
@ -1575,6 +1578,27 @@ bool isVariablesChanged(const Token* start,
return false;
}
bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp)
{
for (const Token* tok = start; tok != end; tok = tok->next()) {
if (!exprDependsOnThis(tok))
continue;
if (Token::Match(tok->previous(), "%name% (")) {
if (tok->previous()->function()) {
if (!tok->previous()->function()->isConst())
return true;
else
continue;
} else if (!tok->previous()->isKeyword()) {
return true;
}
}
if (isVariableChanged(tok, indirect, settings, cpp))
return true;
}
return false;
}
int numberOfArguments(const Token *start)
{
int arguments=0;

2
lib/astutils.h
View File

@ -194,6 +194,8 @@ bool isVariablesChanged(const Token* start,
const Settings* settings,
bool cpp);
bool isThisChanged(const Token* start, const Token* end, int indirect, const Settings* settings, bool cpp);
const Token* findVariableChanged(const Token *start, const Token *end, int indirect, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);

6
lib/checkcondition.cpp
View File

@ -453,6 +453,12 @@ void CheckCondition::duplicateCondition()
bool modified = false;
visitAstNodes(cond1, [&](const Token* tok3) {
if (exprDependsOnThis(tok3)) {
if (isThisChanged(scope.classDef->next(), cond2, false, mSettings, mTokenizer->isCPP())) {
modified = true;
return ChildrenToVisit::done;
}
}
if (tok3->varId() > 0 &&
isVariableChanged(scope.classDef->next(), cond2, tok3->varId(), false, mSettings, mTokenizer->isCPP())) {
modified = true;

49
test/testcondition.cpp
View File

@ -3688,6 +3688,55 @@ private:
" if (a.b) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("struct A {\n"
" int a;\n"
" void b() const {\n"
" return a == 1;\n"
" }\n"
" void c();\n"
" void d() {\n"
" if(b()) {\n"
" c();\n"
" }\n"
" if (b()) {\n"
" a = 3;\n"
" }\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("struct A {\n"
" int a;\n"
" void b() const {\n"
" return a == 1;\n"
" }\n"
" void d() {\n"
" if(b()) {\n"
" a = 2;\n"
" }\n"
" if (b()) {\n"
" a = 3;\n"
" }\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("struct A {\n"
" int a;\n"
" void b() const {\n"
" return a == 1;\n"
" }\n"
" void d() {\n"
" if(b()) {\n"
" }\n"
" if (b()) {\n"
" a = 3;\n"
" }\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:9]: (style) The if condition is the same as the previous if condition\n",
errout.str());
}
void checkInvalidTestForOverflow() {