diff --git a/lib/checkother.cpp b/lib/checkother.cpp index b7e3e6e19..02688ce81 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -24,6 +24,7 @@ #include // std::isupper #include // fabs() +#include //--------------------------------------------------------------------------- // 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 breakPattern[] = "break|continue|return|exit|goto"; - //const char functionPattern[] = "%var% ("; - // nested switch + const char switchPattern[] = "switch ("; + const char breakPattern[] = "break|continue|return|exit|goto"; // 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 > ifnest; + std::stack loopnest; + std::stack 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 - break; + if (!ifnest.empty() && tok2 == ifnest.top().first) + { + 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; } } diff --git a/lib/checkother.h b/lib/checkother.h index 63c7ebed7..80605333b 100644 --- a/lib/checkother.h +++ b/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 ? .." */ diff --git a/test/testother.cpp b/test/testother.cpp index 43d07ee8b..c699b78fd 100644 --- a/test/testother.cpp +++ 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()