full implementation of switch case fall through

lib/checkother.cpp

@@ -24,6 +24,7 @@
 
 #include <cctype> // std::isupper
 #include <cmath> // fabs()
+#include <stack>
 //---------------------------------------------------------------------------
 
 // Register this check class (by creating a static instance of it)
@@ -308,10 +309,8 @@ void CheckOther::checkRedundantAssignmentInSwitch()
 
 void CheckOther::checkSwitchCaseFallThrough()
 {
-    const char switchPattern[] = "switch ( %any% ) { case";
+    const char switchPattern[] = "switch (";
-    //const char breakPattern[] = "break|continue|return|exit|goto";
+    const char breakPattern[] = "break|continue|return|exit|goto";
-    //const char functionPattern[] = "%var% (";
-    // nested switch
 
     // Find the beginning of a switch. E.g.:
     //   switch (var) { ...
@@ -320,23 +319,99 @@ void CheckOther::checkSwitchCaseFallThrough()
     {
 
         // Check the contents of the switch statement
-        for (const Token *tok2 = tok->tokAt(6); tok2; tok2 = tok2->next())
+        std::stack<std::pair<Token *, bool> > ifnest;
+        std::stack<Token *> loopnest;
+        std::stack<Token *> scopenest;
+        bool justbreak = true;
+        for (const Token *tok2 = tok->tokAt(1)->link()->tokAt(2); tok2; tok2 = tok2->next())
         {
-            if (Token::Match(tok2, switchPattern))
+            if (Token::Match(tok2, "if ("))
             {
-                tok2 = tok2->tokAt(4)->link()->previous();
+                tok2 = tok2->tokAt(1)->link()->next();
+                ifnest.push(std::make_pair(tok2->link(), false));
+                justbreak = false;
             }
-            else if (tok2->str() == "case")
+            else if (Token::Match(tok2, "while ("))
             {
-                if (!Token::Match(tok2->previous()->previous(), "break"))
+                tok2 = tok2->tokAt(1)->link()->next();
+                loopnest.push(tok2->link());
+                justbreak = false;
+            }
+            else if (Token::Match(tok2, "do {"))
+            {
+                tok2 = tok2->tokAt(1);
+                loopnest.push(tok2->link());
+                justbreak = false;
+            }
+            else if (Token::Match(tok2, "for ("))
+            {
+                tok2 = tok2->tokAt(1)->link()->next();
+                loopnest.push(tok2->link());
+                justbreak = false;
+            }
+            else if (Token::Match(tok2, switchPattern))
+            {
+                // skip over nested switch, we'll come to that soon
+                tok2 = tok2->tokAt(1)->link()->next()->link();
+            }
+            else if (Token::Match(tok2, breakPattern))
+            {
+                if (loopnest.empty())
+                {
+                    justbreak = true;
+                }
+                tok2 = Token::findmatch(tok2, ";");
+            }
+            else if (Token::Match(tok2, "case|default"))
+            {
+                if (!justbreak)
                 {
                     switchCaseFallThrough(tok2);
                 }
+                tok2 = Token::findmatch(tok2, ":");
+                justbreak = true;
+            }
+            else if (tok2->str() == "{")
+            {
+                scopenest.push(tok2->link());
             }
             else if (tok2->str() == "}")
             {
-                // End of the switch block
+                if (!ifnest.empty() && tok2 == ifnest.top().first)
-                break;
+                {
+                    if (tok2->next()->str() == "else")
+                    {
+                        tok2 = tok2->tokAt(2);
+                        ifnest.pop();
+                        ifnest.push(std::make_pair(tok2->link(), justbreak));
+                        justbreak = false;
+                    }
+                    else
+                    {
+                        justbreak &= ifnest.top().second;
+                        ifnest.pop();
+                    }
+                }
+                else if (!loopnest.empty() && tok2 == loopnest.top())
+                {
+                    loopnest.pop();
+                }
+                else if (!scopenest.empty() && tok2 == scopenest.top())
+                {
+                    scopenest.pop();
+                }
+                else
+                {
+                    assert(ifnest.empty());
+                    assert(loopnest.empty());
+                    assert(scopenest.empty());
+                    // end of switch block
+                    break;
+                }
+            }
+            else if (tok2->str() != ";")
+            {
+                justbreak = false;
             }
 
         }

lib/checkother.h

@@ -61,7 +61,6 @@ public:
         checkOther.sizeofsizeof();
         checkOther.sizeofCalculation();
         checkOther.checkRedundantAssignmentInSwitch();
-        checkOther.checkSwitchCaseFallThrough();
         checkOther.checkAssignmentInAssert();
         checkOther.checkSizeofForArrayParameter();
         checkOther.checkSelfAssignment();
@@ -91,6 +90,7 @@ public:
         checkOther.checkIncorrectStringCompare();
         checkOther.checkIncrementBoolean();
         checkOther.checkComparisonOfBoolWithInt();
+        checkOther.checkSwitchCaseFallThrough();
     }
 
     /** @brief Clarify calculation for ".. a * b ? .." */

test/testother.cpp

@@ -202,6 +202,8 @@ private:
         // Check..
         CheckOther checkOther(&tokenizer, &settings, &logger);
         checkOther.checkSwitchCaseFallThrough();
+
+        logger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(filename));
     }
 
 
@@ -1215,6 +1217,18 @@ private:
             "}\n");
         ASSERT_EQUALS("", errout.str());
 
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 0:\n"
+            "        case 1:\n"
+            "            break;\n"
+            "        case 2:\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
+        ASSERT_EQUALS("", errout.str());
+
         check_preprocess_suppress(
             "void foo() {\n"
             "    switch (a) {\n"
@@ -1226,6 +1240,17 @@ private:
             "}\n");
         ASSERT_EQUALS("[test.cpp:5]: (warning) Switch falls through case without comment\n", errout.str());
 
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 1:\n"
+            "            g();\n"
+            "        default:\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Switch falls through case without comment\n", errout.str());
+
         check_preprocess_suppress(
             "void foo() {\n"
             "    switch (a) {\n"
@@ -1249,7 +1274,103 @@ private:
             "            break;\n"
             "    }\n"
             "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (information) Unmatched suppression: switchCaseFallThrough\n", errout.str());
+
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 1:\n"
+            "            {\n"
+            "                break;\n"
+            "            }\n"
+            "        case 2:\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 1:\n"
+            "            for (;;) {\n"
+            "                break;\n"
+            "            }\n"
+            "        case 2:\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (warning) Switch falls through case without comment\n", errout.str());
+
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 1:\n"
+            "            if (b) {\n"
+            "                break;\n"
+            "            } else {\n"
+            "                break;\n"
+            "            }\n"
+            "        case 2:\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 1:\n"
+            "            if (b) {\n"
+            "                break;\n"
+            "            } else {\n"
+            "            }\n"
+            "        case 2:\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
+        ASSERT_EQUALS("[test.cpp:8]: (warning) Switch falls through case without comment\n", errout.str());
+
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 1:\n"
+            "            if (b) {\n"
+            "                break;\n"
+            "            }\n"
+            "        case 2:\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
+        ASSERT_EQUALS("[test.cpp:7]: (warning) Switch falls through case without comment\n", errout.str());
+
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 1:\n"
+            "            if (b) {\n"
+            "            } else {\n"
+            "                break;\n"
+            "            }\n"
+            "        case 2:\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
+        ASSERT_EQUALS("[test.cpp:8]: (warning) Switch falls through case without comment\n", errout.str());
+
+        check_preprocess_suppress(
+            "void foo() {\n"
+            "    switch (a) {\n"
+            "        case 1:\n"
+            "            if (b) {\n"
+            "        case 2:\n"
+            "            } else {\n"
+            "                break;\n"
+            "            }\n"
+            "            break;\n"
+            "    }\n"
+            "}\n");
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Switch falls through case without comment\n", errout.str());
     }
 
     void selfAssignment()