diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 465ece706..05beaba98 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1752,14 +1752,18 @@ void CheckOther::checkUnreachableCode() labelName = tok->tokAt(1); } - if (secondBreak) { + // Statements follow directly, no line between them. (#3383) + // TODO: Try to find a better way to avoid false positives due to preprocessor configurations. + bool inconclusive = secondBreak && (secondBreak->linenr()-1 > secondBreak->previous()->linenr()); + + if (secondBreak && (_settings->inconclusive || !inconclusive)) { if (Token::Match(secondBreak, "continue|goto|throw") || (secondBreak->str() == "return" && (tok->str() == "return" || secondBreak->strAt(1) == ";"))) { // return with value after statements like throw can be necessary to make a function compile - duplicateBreakError(secondBreak); + duplicateBreakError(secondBreak, inconclusive); tok = Token::findmatch(secondBreak, "[}:]"); } else if (secondBreak->str() == "break") { // break inside switch as second break statement should not issue a warning if (tok->str() == "break") // If the previous was a break, too: Issue warning - duplicateBreakError(secondBreak); + duplicateBreakError(secondBreak, inconclusive); else { unsigned int indent = 0; for (const Token* tok2 = tok; tok2; tok2 = tok2->previous()) { // Check, if the enclosing scope is a switch (TODO: Can we use SymbolDatabase here?) @@ -1767,7 +1771,7 @@ void CheckOther::checkUnreachableCode() indent++; else if (indent == 0 && tok2->str() == "{" && tok2->strAt(-1) == ")") { if (tok2->previous()->link()->strAt(-1) != "switch") { - duplicateBreakError(secondBreak); + duplicateBreakError(secondBreak, inconclusive); break; } else break; @@ -1792,7 +1796,7 @@ void CheckOther::checkUnreachableCode() } } if (!labelInFollowingLoop) - unreachableCodeError(secondBreak); + unreachableCodeError(secondBreak, inconclusive); tok = Token::findmatch(secondBreak, "[}:]"); } else tok = secondBreak; @@ -1803,17 +1807,26 @@ void CheckOther::checkUnreachableCode() } } -void CheckOther::duplicateBreakError(const Token *tok) +void CheckOther::duplicateBreakError(const Token *tok, bool inconclusive) { - reportError(tok, Severity::style, "duplicateBreak", - "Consecutive return, break, continue, goto or throw statements are unnecessary.\n" - "The second of the two statements can never be executed, and so should be removed."); + if (inconclusive) + reportInconclusiveError(tok, Severity::style, "duplicateBreak", + "Consecutive return, break, continue, goto or throw statements are unnecessary.\n" + "The second of the two statements can never be executed, and so should be removed."); + else + reportError(tok, Severity::style, "duplicateBreak", + "Consecutive return, break, continue, goto or throw statements are unnecessary.\n" + "The second of the two statements can never be executed, and so should be removed."); } -void CheckOther::unreachableCodeError(const Token *tok) +void CheckOther::unreachableCodeError(const Token *tok, bool inconclusive) { - reportError(tok, Severity::style, "unreachableCode", - "Statements following return, break, continue, goto or throw will never be executed."); + if (inconclusive) + reportInconclusiveError(tok, Severity::style, "unreachableCode", + "Statements following return, break, continue, goto or throw will never be executed."); + else + reportError(tok, Severity::style, "unreachableCode", + "Statements following return, break, continue, goto or throw will never be executed."); } //--------------------------------------------------------------------------- diff --git a/lib/checkother.h b/lib/checkother.h index e5c8f99bb..13d9ff093 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -300,8 +300,8 @@ private: void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op); void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2); void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2); - void duplicateBreakError(const Token *tok); - void unreachableCodeError(const Token* tok); + void duplicateBreakError(const Token *tok, bool inconclusive); + void unreachableCodeError(const Token* tok, bool inconclusive); void assignBoolToPointerError(const Token *tok); void unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive); void unsignedPositiveError(const Token *tok, const std::string &varname, bool inconclusive); @@ -359,8 +359,8 @@ private: c.duplicateExpressionError(0, 0, "&&"); c.alwaysTrueFalseStringCompareError(0, "str1", "str2"); c.alwaysTrueStringVariableCompareError(0, "varname1", "varname2"); - c.duplicateBreakError(0); - c.unreachableCodeError(0); + c.duplicateBreakError(0, false); + c.unreachableCodeError(0, false); c.unsignedLessThanZeroError(0, "varname", false); c.unsignedPositiveError(0, "varname", false); c.bitwiseOnBooleanError(0, "varname", "&&"); diff --git a/test/testother.cpp b/test/testother.cpp index 2fa714958..f911840df 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -159,13 +159,13 @@ private: TEST_CASE(checkDoubleFree); } - void check(const char code[], const char *filename = NULL, bool experimental = false) { + void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true) { // Clear the error buffer.. errout.str(""); Settings settings; settings.addEnabled("style"); - settings.inconclusive = true; + settings.inconclusive = inconclusive; settings.experimental = experimental; // Tokenize.. @@ -2043,6 +2043,22 @@ private: ASSERT_EQUALS("", errout.str()); // #3457 check("%: return ; ()"); // Don't crash. #3441. + + // #3383. TODO: Use preprocessor + check("int foo() {\n" + "\n" // #ifdef A + " return 0;\n" + "\n" // #endif + " return 1;\n" + "}", 0, false, false); + ASSERT_EQUALS("", errout.str()); + check("int foo() {\n" + "\n" // #ifdef A + " return 0;\n" + "\n" // #endif + " return 1;\n" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (style) Consecutive return, break, continue, goto or throw statements are unnecessary.\n", errout.str()); }