diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 80b9b4b9b..f215a8517 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -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 diff --git a/test/testother.cpp b/test/testother.cpp index 9603ff6de..21658139d 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -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() {