diff --git a/lib/checkother.cpp b/lib/checkother.cpp index ad4a8ef9e..d2adeb0d3 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2618,3 +2618,31 @@ void CheckOther::raceAfterInterlockedDecrementError(const Token* tok) reportError(tok, Severity::error, "raceAfterInterlockedDecrement", "Race condition: non-interlocked access after InterlockedDecrement(). Use InterlockedDecrement() return value instead."); } + +void CheckOther::checkUnusedLabel() +{ + if (!_settings->isEnabled("style")) + return; + + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + const std::size_t functions = symbolDatabase->functionScopes.size(); + for (std::size_t i = 0; i < functions; ++i) { + const Scope * scope = symbolDatabase->functionScopes[i]; + + for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { + if (!tok->scope()->isExecutable()) + tok = tok->scope()->classEnd; + + if (Token::Match(tok, "{|}|; %name% :") && tok->strAt(1) != "default") { + if (!Token::findsimplematch(scope->classStart->next(), ("goto " + tok->strAt(1)).c_str(), scope->classEnd->previous())) + unusedLabelError(tok->next()); + } + } + } +} + +void CheckOther::unusedLabelError(const Token* tok) +{ + reportError(tok, Severity::style, "unusedLabel", + "Label '" + (tok?tok->str():emptyString) + "' is not used."); +} diff --git a/lib/checkother.h b/lib/checkother.h index e4faf853c..34dd8e72c 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -69,10 +69,11 @@ public: checkOther.checkIgnoredReturnValue(); checkOther.checkRedundantPointerOp(); checkOther.checkZeroDivision(); + checkOther.checkInterlockedDecrement(); + checkOther.checkUnusedLabel(); // --check-library : functions with nonmatching configuration checkOther.checkLibraryMatchFunctions(); - checkOther.checkInterlockedDecrement(); } /** @brief Run checks against the simplified token list */ @@ -222,8 +223,12 @@ public: /** @brief --check-library: warn for unconfigured function calls */ void checkLibraryMatchFunctions(); + /** @brief %Check for race condition with non-interlocked access after InterlockedDecrement() */ void checkInterlockedDecrement(); + /** @brief %Check for unused labels */ + void checkUnusedLabel(); + private: // Error messages.. void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &strFunctionName, const std::string &varName, const bool result); @@ -276,6 +281,7 @@ private: void ignoredReturnValueError(const Token* tok, const std::string& function); void redundantPointerOpError(const Token* tok, const std::string& varname, bool inconclusive); void raceAfterInterlockedDecrementError(const Token* tok); + void unusedLabelError(const Token* tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { CheckOther c(0, settings, errorLogger); @@ -334,6 +340,7 @@ private: c.commaSeparatedReturnError(0); c.ignoredReturnValueError(0, "malloc"); c.redundantPointerOpError(0, "varname", false); + c.unusedLabelError(0); } static std::string myName() { @@ -391,7 +398,8 @@ private: "- comma in return statement (the comma can easily be misread as a semicolon).\n" "- prefer erfc, expm1 or log1p to avoid loss of precision.\n" "- identical code in both branches of if/else or ternary operator.\n" - "- redundant pointer operation on pointer like &*some_ptr.\n"; + "- redundant pointer operation on pointer like &*some_ptr.\n" + "- find unused 'goto' labels.\n"; } }; /// @} diff --git a/test/testother.cpp b/test/testother.cpp index a6544dc85..194249521 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -168,6 +168,8 @@ private: TEST_CASE(redundantPointerOp); TEST_CASE(test_isSameExpression); TEST_CASE(raceAfterInterlockedDecrement); + + TEST_CASE(testUnusedLabel); } void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, Settings* settings = 0) { @@ -3091,7 +3093,7 @@ private: " label:\n" " throw 0;\n" "}", nullptr, false, false, false); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Label 'label' is not used.\n", errout.str()); check("void foo() {\n" " wxCHECK2(state < 3 && state >= 0, return);\n" @@ -6447,6 +6449,51 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); } + + void testUnusedLabel() { + check("void f() {\n" + " label:\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Label 'label' is not used.\n", errout.str()); + + check("void f() {\n" + " label:\n" + " foo();\n" + " goto label;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " label:\n" + " foo();\n" + " goto label;\n" + "}\n" + "void g() {\n" + " label:\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (style) Label 'label' is not used.\n", errout.str()); + + check("void f() {\n" + " switch(a) {\n" + " default:\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " class X {\n" + " protected:\n" + " };\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " class X {\n" + " my_protected:\n" + " };\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestOther)