redundant variable assignments: avoid FPs for initializations

This commit is contained in:
Daniel Marjamäki 2018-11-25 08:49:09 +01:00
parent 2f66b31d43
commit 22f736eae2
2 changed files with 44 additions and 22 deletions

View File

@ -553,17 +553,28 @@ void CheckOther::checkRedundantAssignment()
if ((tok->isAssignmentOp() || Token::Match(tok, "++|--")) && tok->astOperand1()) {
if (tok->astParent())
continue;
// Do not warn about redundant initialization
// TODO : do not simplify the variable declarations
if (Token::Match(tok->tokAt(-3), "%var% ; %var% =") && tok->previous()->variable() && tok->previous()->variable()->nameToken() == tok->tokAt(-3))
continue;
// Do not warn about assignment with 0 / NULL
if (Token::simpleMatch(tok->astOperand2(), "0"))
continue;
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference())
// todo: check references
continue;
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isStatic())
// todo: check static variables
continue;
if (hasOperand(tok->astOperand2(), tok->astOperand1(), mTokenizer->isCPP(), mSettings->library))
continue;
// If there is a custom assignment operator => this is inconclusive
bool inconclusive = false;
if (mTokenizer->isCPP() && tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->typeScope) {
const std::string op = "operator" + tok->str();
@ -577,6 +588,7 @@ void CheckOther::checkRedundantAssignment()
if (inconclusive && !mSettings->inconclusive)
continue;
// Is there a redundant assignment?
bool read = false;
const Token *start;
if (tok->isAssignmentOp())
@ -587,6 +599,7 @@ void CheckOther::checkRedundantAssignment()
if (read || !nextAssign)
continue;
// there is redundant assignment. Is there a case between the assignments?
bool hasCase = false;
for (const Token *tok2 = tok; tok2 != nextAssign; tok2 = tok2->next()) {
if (tok2->str() == "case") {
@ -595,6 +608,7 @@ void CheckOther::checkRedundantAssignment()
}
}
// warn
if (hasCase)
redundantAssignmentInSwitchError(tok, nextAssign, tok->astOperand1()->expressionString());
else

View File

@ -5601,13 +5601,15 @@ private:
check("void f() {\n"
" int i;\n"
" dostuff();\n"
" i = 1;\n"
" i = 1;\n"
"}", nullptr, false, false, false);
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
check("void f() {\n"
" static int i;\n"
" dostuff();\n"
" i = 1;\n"
" i = 1;\n"
"}");
@ -5617,7 +5619,7 @@ private:
" int i[10];\n"
" i[2] = 1;\n"
" i[2] = 1;\n"
"}", nullptr, false, false, false);
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'i[2]' is reassigned a value before the old one has been used.\n", errout.str());
check("void f(int x) {\n"
@ -5625,14 +5627,14 @@ private:
" i[x] = 1;\n"
" x=1;\n"
" i[x] = 1;\n"
"}", nullptr, false, false, false);
"}");
ASSERT_EQUALS("", errout.str());
check("void f(const int x) {\n"
" int i[10];\n"
" i[x] = 1;\n"
" i[x] = 1;\n"
"}", nullptr, false, false, false);
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'i[x]' is reassigned a value before the old one has been used.\n", errout.str());
// Testing different types
@ -5683,18 +5685,19 @@ private:
check("void f() {\n"
" int i;\n"
" dostuff();\n"
" i = 1;\n"
" bar();\n"
" i = 1;\n"
"}", nullptr, false, false, false);
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
check("void bar(int i) {}\n"
"void f(int i) {\n"
" i = 1;\n"
" bar(i);\n" // Passed as argument
" i = 1;\n"
"}", nullptr, false, false, false);
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
@ -5749,7 +5752,7 @@ private:
check("class C {\n"
" int x;\n"
" void g() { return x*x; }\n"
" void g() { return x * x; }\n"
" void f();\n"
"};\n"
"\n"
@ -5774,15 +5777,17 @@ private:
// from #3103 (avoid a false negative)
check("int foo(){\n"
" int x;\n"
" dostuff();\n"
" x = 1;\n"
" x = 1;\n"
" return x + 1;\n"
"}", nullptr, false, false, false);
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'x' is reassigned a value before the old one has been used.\n", errout.str());
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (style) Variable 'x' is reassigned a value before the old one has been used.\n", errout.str());
// from #3103 (avoid a false positive)
check("int foo(){\n"
" int x;\n"
" dostuff();\n"
" x = 1;\n"
" if (y)\n" // <-- cppcheck does not know anything about 'y'
" x = 2;\n"
@ -5790,23 +5795,25 @@ private:
"}");
ASSERT_EQUALS("", errout.str());
// initialization, assignment with 0
check("void f() {\n" // Ticket #4356
" int x = 0;\n" // <- ignore assignment with 0
" int x = 0;\n" // <- ignore initialization with 0
" x = 3;\n"
"}", 0, false, false, false);
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int i = 54;\n"
" i = 0;\n"
"}", 0, false, false, false);
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
" state_t *x = NULL;\n"
" x = dostuff();\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int i = 54;\n"
" i = 1;\n"
"}", 0, false, false, false);
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
" state_t *x;\n"
" x = NULL;\n"
" x = dostuff();\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int foo() {\n" // #4420
" int x;\n"
@ -6008,10 +6015,11 @@ private:
check("class C { void operator=(int x); };\n" // #8368 - assignment operator might have side effects => inconclusive
"void f() {\n"
" C c;\n"
" dostuff();\n"
" c = x;\n"
" c = x;\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (style, inconclusive) Variable 'c' is reassigned a value before the old one has been used if variable is no semaphore variable.\n", errout.str());
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6]: (style, inconclusive) Variable 'c' is reassigned a value before the old one has been used if variable is no semaphore variable.\n", errout.str());
}
void redundantVarAssignment_stackoverflow() {