diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 07e5c1468..9928e779f 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3382,3 +3382,112 @@ void CheckOther::incompleteArrayFillError(const Token* tok, const std::string& b "Array '" + buffer + "' is filled incompletely. Did you forget to multiply the size given to '" + function + "()' with 'sizeof(*" + buffer + ")'?\n" "The array '" + buffer + "' is filled incompletely. The function '" + function + "()' needs the size given in bytes, but an element of the given array is larger than one byte. Did you forget to multiply the size with 'sizeof(*" + buffer + ")'?", true); } + + +void CheckOther::avoidDeadEndInNestedIfs() +{ + if (!_settings->isEnabled("style")) + return; + + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + + for (std::list::const_iterator scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) { + const Token* const toke = scope->classDef; + + + if (scope->type == Scope::eIf && toke) { + + int flag = 0; + const Token *op1Tok, *op2Tok; + op1Tok = scope->classDef->tokAt(2); + op2Tok = scope->classDef->tokAt(4); + + if (scope->classDef->strAt(6) == "{") { + + if (scope->classDef->strAt(3) == "==") { + for (const Token* tok = scope->classStart; tok != scope->classEnd && flag == 0; tok = tok->next()) { + if ((tok->str() == op1Tok->str() || tok->str() == op2Tok->str()) && tok->strAt(1) == "=") + break; + else if (Token::Match(tok, "%any% ( %any% )")) { + if ((tok->strAt(2) == op1Tok->str() || tok->strAt(2) == op2Tok->str())) + break; + } else if (Token::Match(tok, "%any% ( %any% , %any%")) { + for (const Token* tok2 = tok->next(); tok2 != tok->linkAt(1); tok2 = tok2->next()) { + if (tok2->str() == op1Tok->str()) { + flag = 1; + break; + } + } + } else if (Token::Match(tok, "if ( %any% !=|<|>|<=|>= %any% )")) { + if ((tok->strAt(2) == op1Tok->str() && tok->strAt(4) == op2Tok->str()) || (tok->strAt(2) == op2Tok->str() && tok->strAt(4) == op1Tok->str())) + warningDeadCode(toke); + } + } + } else if (scope->classDef->strAt(3) == "!=") { + for (const Token* tok = scope->classStart; tok != scope->classEnd && flag == 0; tok = tok->next()) { + if ((tok->str() == op1Tok->str() || tok->str() == op2Tok->str()) && tok->strAt(1) == "=") + break; + else if (Token::Match(tok, "%any% ( %any% )")) { + if ((tok->strAt(2) == op1Tok->str() || tok->strAt(2) == op2Tok->str())) + break; + } else if (Token::Match(tok, "%any% ( %any% , %any%")) { + for (const Token* tok2 = tok->next(); tok2 != tok->linkAt(1); tok2 = tok2->next()) { + if (tok2->str() == op1Tok->str()) { + flag = 1; + break; + } + } + } else if (Token::Match(tok, "if ( %any% ==|>=|<= %any% )")) { + if ((tok->strAt(2) == op1Tok->str() && tok->strAt(4) == op2Tok->str()) || (tok->strAt(2) == op2Tok->str() && tok->strAt(4) == op1Tok->str())) + warningDeadCode(toke); + } + } + } else if (scope->classDef->strAt(3) == "<") { + for (const Token* tok = scope->classStart; tok != scope->classEnd && flag == 0; tok = tok->next()) { + if ((tok->str() == op1Tok->str() || tok->str() == op2Tok->str()) && tok->strAt(1) == "=") + break; + else if (Token::Match(tok, "%any% ( %any% )")) { + if ((tok->strAt(2) == op1Tok->str() || tok->strAt(2) == op2Tok->str())) + break; + } else if (Token::Match(tok, "%any% ( %any% , %any%")) { + for (const Token* tok2 = tok->next(); tok2 != tok->linkAt(1); tok2 = tok2->next()) { + if (tok2->str() == op1Tok->str()) { + flag = 1; + break; + } + } + } else if (Token::Match(tok, "if ( %any% <|<=|>|>=|== %any% )")) { + if ((tok->strAt(2) == op1Tok->str() && tok->strAt(4) == op2Tok->str()) || (tok->strAt(2) == op2Tok->str() && tok->strAt(4) == op1Tok->str())) + warningDeadCode(toke); + } + } + } else if (scope->classDef->strAt(3) == "<=") { + for (const Token* tok = scope->classStart; tok != scope->classEnd && flag == 0; tok = tok->next()) { + if ((tok->str() == op1Tok->str() || tok->str() == op2Tok->str()) && tok->strAt(1) == "=") + break; + else if (Token::Match(tok, "%any% ( %any% )")) { + if ((tok->strAt(2) == op1Tok->str() || tok->strAt(2) == op2Tok->str())) + break; + } else if (Token::Match(tok, "%any% ( %any% , %any%")) { + for (const Token* tok2 = tok->next(); tok2 != tok->linkAt(1); tok2 = tok2->next()) { + if (tok2->str() == op1Tok->str()) { + flag = 1; + break; + } + } + } else if (Token::Match(tok, "if ( %any% <|<=|>|>= %any% )")) { + if ((tok->strAt(2) == op1Tok->str() && tok->strAt(4) == op2Tok->str()) || (tok->strAt(2) == op2Tok->str() && tok->strAt(4) == op1Tok->str())) + warningDeadCode(toke); + } + } + } + } + } + + } +} + +void CheckOther::warningDeadCode(const Token *tok) +{ + reportError(tok, Severity::warning, "redundantOperationIn", "There are opposite condition checks in your nested-if block, which leads to a dead code block", true); +} diff --git a/lib/checkother.h b/lib/checkother.h index b926ae2af..baf068676 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -84,6 +84,7 @@ public: CheckOther checkOther(tokenizer, settings, errorLogger); // Checks + checkOther.avoidDeadEndInNestedIfs(); checkOther.clarifyCalculation(); checkOther.clarifyStatement(); checkOther.checkConstantFunctionParameter(); @@ -112,6 +113,9 @@ public: checkOther.checkNegativeBitwiseShift(); } + /** To check the dead code in a program, which is unaccessible due to the counter-conditions check in nested-if statements **/ + void avoidDeadEndInNestedIfs(); + /** @brief Clarify calculation for ".. a * b ? .." */ void clarifyCalculation(); @@ -267,6 +271,7 @@ public: private: // Error messages.. + void warningDeadCode(const Token *tok); void clarifyCalculationError(const Token *tok, const std::string &op); void clarifyConditionError(const Token *tok, bool assign, bool boolop); void clarifyStatementError(const Token* tok, const std::string &expr, const std::string &suggested); @@ -351,6 +356,7 @@ private: c.redundantAssignmentError(0, 0, "var"); // style/warning + c.warningDeadCode(0); c.cstyleCastError(0); c.dangerousUsageStrtolError(0); c.passedByValueError(0, "parametername"); @@ -421,6 +427,7 @@ private: "* subsequent assignment or copying to a variable or buffer\n" // style + "* Find dead code which is unaccessible due to the counter-conditions check in nested if statements\n" "* C-style pointer cast in cpp file\n" "* casting between incompatible pointer types\n" "* redundant if\n" diff --git a/test/testother.cpp b/test/testother.cpp index 071b4230b..61982eb1e 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -33,6 +33,7 @@ private: void run() { + TEST_CASE(avoidDeadEndInNestedIfs); TEST_CASE(assignBoolToPointer); TEST_CASE(zeroDiv1); @@ -252,6 +253,43 @@ private: } + void avoidDeadEndInNestedIfs() { + check("void foo(int a, int b)\n" + "{\n" + " if(a==b)\n" + " if(a!=b)\n" + " cout << a;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) There are opposite condition checks in your nested-if block, which leads to a dead code block\n", errout.str()); + + check("void foo(int i) \n" + "{\n" + " if(i > 5) {\n" + " i = bar();\n" + " if(i < 5) {\n" + " cout << a; \n" + " }\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + + check("void foo(int& i)\n" + "{\n" + " i=6;\n" + "}\n" + "void bar(int i)\n" + "{\n" + " if(i>5){\n" + " foo(i);\n" + " if(i<5){\n" + " }\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + + void assignBoolToPointer() { check("void foo(bool *p) {\n" " p = false;\n"