[clang-tools-extra] 490bf27 - Revert "[clang-tidy] Add bugprone-empty-catch check"
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 23 11:15:12 PDT 2023
Author: Piotr Zegar
Date: 2023-07-23T18:13:52Z
New Revision: 490bf27e53445fc4514c85142dec33ddf5bdcfe2
URL: https://github.com/llvm/llvm-project/commit/490bf27e53445fc4514c85142dec33ddf5bdcfe2
DIFF: https://github.com/llvm/llvm-project/commit/490bf27e53445fc4514c85142dec33ddf5bdcfe2.diff
LOG: Revert "[clang-tidy] Add bugprone-empty-catch check"
CI failed on "ubuntu-fast" due to disabled exceptions.
This reverts commit f256fee5343033bf8a31aee06a80f3e982b76f82.
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
Removed:
clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 0cb0924d445e5e..7509e94950e10e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -20,7 +20,6 @@
#include "DanglingHandleCheck.h"
#include "DynamicStaticInitializersCheck.h"
#include "EasilySwappableParametersCheck.h"
-#include "EmptyCatchCheck.h"
#include "ExceptionEscapeCheck.h"
#include "FoldInitTypeCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
@@ -107,7 +106,6 @@ class BugproneModule : public ClangTidyModule {
"bugprone-dynamic-static-initializers");
CheckFactories.registerCheck<EasilySwappableParametersCheck>(
"bugprone-easily-swappable-parameters");
- CheckFactories.registerCheck<EmptyCatchCheck>("bugprone-empty-catch");
CheckFactories.registerCheck<ExceptionEscapeCheck>(
"bugprone-exception-escape");
CheckFactories.registerCheck<FoldInitTypeCheck>("bugprone-fold-init-type");
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 4076e0d253584b..8bd892eeb41ecd 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -15,7 +15,6 @@ add_clang_library(clangTidyBugproneModule
DanglingHandleCheck.cpp
DynamicStaticInitializersCheck.cpp
EasilySwappableParametersCheck.cpp
- EmptyCatchCheck.cpp
ExceptionEscapeCheck.cpp
FoldInitTypeCheck.cpp
ForwardDeclarationNamespaceCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
deleted file mode 100644
index 865c88391b0b4b..00000000000000
--- a/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
+++ /dev/null
@@ -1,110 +0,0 @@
-//===--- EmptyCatchCheck.cpp - clang-tidy ---------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "EmptyCatchCheck.h"
-#include "../utils/Matchers.h"
-#include "../utils/OptionsUtils.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Lex/Lexer.h"
-#include <algorithm>
-
-using namespace clang::ast_matchers;
-using ::clang::ast_matchers::internal::Matcher;
-
-namespace clang::tidy::bugprone {
-
-namespace {
-AST_MATCHER(CXXCatchStmt, isInMacro) {
- return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID() ||
- Node.getCatchLoc().isMacroID();
-}
-
-AST_MATCHER_P(CXXCatchStmt, hasHandler, Matcher<Stmt>, InnerMatcher) {
- Stmt *Handler = Node.getHandlerBlock();
- if (!Handler)
- return false;
- return InnerMatcher.matches(*Handler, Finder, Builder);
-}
-
-AST_MATCHER_P(CXXCatchStmt, hasCaughtType, Matcher<QualType>, InnerMatcher) {
- return InnerMatcher.matches(Node.getCaughtType(), Finder, Builder);
-}
-
-AST_MATCHER_P(CompoundStmt, hasAnyTextFromList, std::vector<llvm::StringRef>,
- List) {
- if (List.empty())
- return false;
-
- ASTContext &Context = Finder->getASTContext();
- SourceManager &SM = Context.getSourceManager();
- StringRef Text = Lexer::getSourceText(
- CharSourceRange::getTokenRange(Node.getSourceRange()), SM,
- Context.getLangOpts());
- return std::any_of(List.begin(), List.end(), [&](const StringRef &Str) {
- return Text.contains_insensitive(Str);
- });
-}
-
-} // namespace
-
-EmptyCatchCheck::EmptyCatchCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context),
- IgnoreCatchWithKeywords(utils::options::parseStringList(
- Options.get("IgnoreCatchWithKeywords", "@TODO;@FIXME"))),
- AllowEmptyCatchForExceptions(utils::options::parseStringList(
- Options.get("AllowEmptyCatchForExceptions", ""))) {}
-
-void EmptyCatchCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
- Options.store(Opts, "IgnoreCatchWithKeywords",
- utils::options::serializeStringList(IgnoreCatchWithKeywords));
- Options.store(
- Opts, "AllowEmptyCatchForExceptions",
- utils::options::serializeStringList(AllowEmptyCatchForExceptions));
-}
-
-bool EmptyCatchCheck::isLanguageVersionSupported(
- const LangOptions &LangOpts) const {
- return LangOpts.CPlusPlus;
-}
-
-std::optional<TraversalKind> EmptyCatchCheck::getCheckTraversalKind() const {
- return TK_IgnoreUnlessSpelledInSource;
-}
-
-void EmptyCatchCheck::registerMatchers(MatchFinder *Finder) {
- auto AllowedNamedExceptionDecl =
- namedDecl(matchers::matchesAnyListedName(AllowEmptyCatchForExceptions));
- auto AllowedNamedExceptionTypes =
- qualType(anyOf(hasDeclaration(AllowedNamedExceptionDecl),
- references(AllowedNamedExceptionDecl),
- pointsTo(AllowedNamedExceptionDecl)));
- auto IgnoredExceptionType =
- qualType(anyOf(AllowedNamedExceptionTypes,
- hasCanonicalType(AllowedNamedExceptionTypes)));
-
- Finder->addMatcher(
- cxxCatchStmt(unless(isExpansionInSystemHeader()), unless(isInMacro()),
- unless(hasCaughtType(IgnoredExceptionType)),
- hasHandler(compoundStmt(
- statementCountIs(0),
- unless(hasAnyTextFromList(IgnoreCatchWithKeywords)))))
- .bind("catch"),
- this);
-}
-
-void EmptyCatchCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *MatchedCatchStmt = Result.Nodes.getNodeAs<CXXCatchStmt>("catch");
-
- diag(
- MatchedCatchStmt->getCatchLoc(),
- "empty catch statements hide issues; to handle exceptions appropriately, "
- "consider re-throwing, handling, or avoiding catch altogether");
-}
-
-} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h
deleted file mode 100644
index b0694384f5c2f7..00000000000000
--- a/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h
+++ /dev/null
@@ -1,37 +0,0 @@
-//===--- EmptyCatchCheck.h - clang-tidy -------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EMPTYCATCHCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EMPTYCATCHCHECK_H
-
-#include "../ClangTidyCheck.h"
-#include <vector>
-
-namespace clang::tidy::bugprone {
-
-/// Detects and suggests addressing issues with empty catch statements.
-///
-/// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/empty-catch.html
-class EmptyCatchCheck : public ClangTidyCheck {
-public:
- EmptyCatchCheck(StringRef Name, ClangTidyContext *Context);
- void registerMatchers(ast_matchers::MatchFinder *Finder) override;
- void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
- void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
- bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
- std::optional<TraversalKind> getCheckTraversalKind() const override;
-
-private:
- std::vector<llvm::StringRef> IgnoreCatchWithKeywords;
- std::vector<llvm::StringRef> AllowEmptyCatchForExceptions;
-};
-
-} // namespace clang::tidy::bugprone
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EMPTYCATCHCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 38a40539aca425..2cc0010884a7a0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -114,11 +114,6 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
-- New :doc:`bugprone-empty-catch
- <clang-tidy/checks/bugprone/empty-catch>` check.
-
- Detects and suggests addressing issues with empty catch statements.
-
- New :doc:`bugprone-multiple-new-in-one-expression
<clang-tidy/checks/bugprone/multiple-new-in-one-expression>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
deleted file mode 100644
index fd1db594ddb241..00000000000000
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
+++ /dev/null
@@ -1,149 +0,0 @@
-.. title:: clang-tidy - bugprone-empty-catch
-
-bugprone-empty-catch
-====================
-
-Detects and suggests addressing issues with empty catch statements.
-
-.. code-block:: c++
-
- try {
- // Some code that can throw an exception
- } catch(const std::exception&) {
- }
-
-Having empty catch statements in a codebase can be a serious problem that
-developers should be aware of. Catch statements are used to handle exceptions
-that are thrown during program execution. When an exception is thrown, the
-program jumps to the nearest catch statement that matches the type of the
-exception.
-
-Empty catch statements, also known as "swallowing" exceptions, catch the
-exception but do nothing with it. This means that the exception is not handled
-properly, and the program continues to run as if nothing happened. This can
-lead to several issues, such as:
-
-* *Hidden Bugs*: If an exception is caught and ignored, it can lead to hidden
- bugs that are
diff icult to diagnose and fix. The root cause of the problem
- may not be apparent, and the program may continue to behave in unexpected
- ways.
-
-* *Security Issues*: Ignoring exceptions can lead to security issues, such as
- buffer overflows or null pointer dereferences. Hackers can exploit these
- vulnerabilities to gain access to sensitive data or execute malicious code.
-
-* *Poor Code Quality*: Empty catch statements can indicate poor code quality
- and a lack of attention to detail. This can make the codebase
diff icult to
- maintain and update, leading to longer development cycles and increased
- costs.
-
-* *Unreliable Code*: Code that ignores exceptions is often unreliable and can
- lead to unpredictable behavior. This can cause frustration for users and
- erode trust in the software.
-
-To avoid these issues, developers should always handle exceptions properly.
-This means either fixing the underlying issue that caused the exception or
-propagating the exception up the call stack to a higher-level handler.
-If an exception is not important, it should still be logged or reported in
-some way so that it can be tracked and addressed later.
-
-If the exception is something that can be handled locally, then it should be
-handled within the catch block. This could involve logging the exception or
-taking other appropriate action to ensure that the exception is not ignored.
-
-Here is an example:
-
-.. code-block:: c++
-
- try {
- // Some code that can throw an exception
- } catch (const std::exception& ex) {
- // Properly handle the exception, e.g.:
- std::cerr << "Exception caught: " << ex.what() << std::endl;
- }
-
-If the exception cannot be handled locally and needs to be propagated up the
-call stack, it should be re-thrown or new exception should be thrown.
-
-Here is an example:
-
-.. code-block:: c++
-
- try {
- // Some code that can throw an exception
- } catch (const std::exception& ex) {
- // Re-throw the exception
- throw;
- }
-
-In some cases, catching the exception at this level may not be necessary, and
-it may be appropriate to let the exception propagate up the call stack.
-This can be done simply by not using ``try/catch`` block.
-
-Here is an example:
-
-.. code-block:: c++
-
- void function() {
- // Some code that can throw an exception
- }
-
- void callerFunction() {
- try {
- function();
- } catch (const std::exception& ex) {
- // Handling exception on higher level
- std::cerr << "Exception caught: " << ex.what() << std::endl;
- }
- }
-
-Other potential solution to avoid empty catch statements is to modify the code
-to avoid throwing the exception in the first place. This can be achieved by
-using a
diff erent API, checking for error conditions beforehand, or handling
-errors in a
diff erent way that does not involve exceptions. By eliminating the
-need for try-catch blocks, the code becomes simpler and less error-prone.
-
-Here is an example:
-
-.. code-block:: c++
-
- // Old code:
- try {
- mapContainer["Key"].callFunction();
- } catch(const std::out_of_range&) {
- }
-
- // New code
- if (auto it = mapContainer.find("Key"); it != mapContainer.end()) {
- it->second.callFunction();
- }
-
-In conclusion, empty catch statements are a bad practice that can lead to hidden
-bugs, security issues, poor code quality, and unreliable code. By handling
-exceptions properly, developers can ensure that their code is robust, secure,
-and maintainable.
-
-Options
--------
-
-.. option:: IgnoreCatchWithKeywords
-
- This option can be used to ignore specific catch statements containing
- certain keywords. If a ``catch`` statement body contains (case-insensitive)
- any of the keywords listed in this semicolon-separated option, then the
- catch will be ignored, and no warning will be raised.
- Default value: `@TODO;@FIXME`.
-
-.. option:: AllowEmptyCatchForExceptions
-
- This option can be used to ignore empty catch statements for specific
- exception types. By default, the check will raise a warning if an empty
- catch statement is detected, regardless of the type of exception being
- caught. However, in certain situations, such as when a developer wants to
- intentionally ignore certain exceptions or handle them in a
diff erent way,
- it may be desirable to allow empty catch statements for specific exception
- types.
- To configure this option, a semicolon-separated list of exception type names
- should be provided. If an exception type name in the list is caught in an
- empty catch statement, no warning will be raised.
- Default value: empty string.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index af147db71eb726..e13b6750196f40 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,7 +86,6 @@ Clang-Tidy Checks
`bugprone-dangling-handle <bugprone/dangling-handle.html>`_,
`bugprone-dynamic-static-initializers <bugprone/dynamic-static-initializers.html>`_,
`bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters.html>`_,
- `bugprone-empty-catch <bugprone/empty-catch.html>`_,
`bugprone-exception-escape <bugprone/exception-escape.html>`_,
`bugprone-fold-init-type <bugprone/fold-init-type.html>`_,
`bugprone-forward-declaration-namespace <bugprone/forward-declaration-namespace.html>`_,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
deleted file mode 100644
index cb264eeb4dc398..00000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-empty-catch %t -- \
-// RUN: -config="{CheckOptions: [{key: bugprone-empty-catch.AllowEmptyCatchForExceptions, value: '::SafeException;WarnException'}, \
-// RUN: {key: bugprone-empty-catch.IgnoreCatchWithKeywords, value: '@IGNORE;@TODO'}]}"
-
-struct Exception {};
-struct SafeException {};
-struct WarnException : Exception {};
-
-int functionWithThrow() {
- try {
- throw 5;
- } catch (const Exception &) {
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
- } catch (...) {
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
- }
- return 0;
-}
-
-int functionWithHandling() {
- try {
- throw 5;
- } catch (const Exception &) {
- return 2;
- } catch (...) {
- return 1;
- }
- return 0;
-}
-
-int functionWithReThrow() {
- try {
- throw 5;
- } catch (...) {
- throw;
- }
-}
-
-int functionWithNewThrow() {
- try {
- throw 5;
- } catch (...) {
- throw Exception();
- }
-}
-
-void functionWithAllowedException() {
- try {
-
- } catch (const SafeException &) {
- } catch (WarnException) {
- }
-}
-
-void functionWithComment() {
- try {
- } catch (const Exception &) {
- // @todo: implement later, check case insensitive
- }
-}
-
-void functionWithComment2() {
- try {
- } catch (const Exception &) {
- // @IGNORE: relax its safe
- }
-}
diff --git a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
index e9a9ed1f5bd820..6a34222c584b2e 100644
--- a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
@@ -27,7 +27,6 @@ static_library("bugprone") {
"DanglingHandleCheck.cpp",
"DynamicStaticInitializersCheck.cpp",
"EasilySwappableParametersCheck.cpp",
- "EmptyCatchCheck.cpp",
"ExceptionEscapeCheck.cpp",
"FoldInitTypeCheck.cpp",
"ForwardDeclarationNamespaceCheck.cpp",
More information about the cfe-commits
mailing list