Fixed #3383: If there is an empty line between subsequent break statements, only issue a message for inconclusive checking

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",
+    if (inconclusive)
-                "Consecutive return, break, continue, goto or throw statements are unnecessary.\n"
+        reportInconclusiveError(tok, Severity::style, "duplicateBreak",
-                "The second of the two statements can never be executed, and so should be removed.");
+                                "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",
+    if (inconclusive)
-                "Statements following return, break, continue, goto or throw will never be executed.");
+        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.");
 }
 
 //---------------------------------------------------------------------------

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 duplicateBreakError(const Token *tok, bool inconclusive);
-    void unreachableCodeError(const Token* tok);
+    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.duplicateBreakError(0, false);
-        c.unreachableCodeError(0);
+        c.unreachableCodeError(0, false);
         c.unsignedLessThanZeroError(0, "varname", false);
         c.unsignedPositiveError(0, "varname", false);
         c.bitwiseOnBooleanError(0, "varname", "&&");

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());
     }