Revert "[clang-tidy] Add bugprone-empty-catch check"
authorPiotr Zegar <me@piotrzegar.pl>
Sun, 23 Jul 2023 18:12:47 +0000 (18:12 +0000)
committerPiotr Zegar <me@piotrzegar.pl>
Sun, 23 Jul 2023 18:13:52 +0000 (18:13 +0000)
CI failed on "ubuntu-fast" due to disabled exceptions.

This reverts commit f256fee5343033bf8a31aee06a80f3e982b76f82.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp [deleted file]
clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h [deleted file]
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst [deleted file]
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp [deleted file]
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn

index 0cb0924..7509e94 100644 (file)
@@ -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 @@ public:
         "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");
index 4076e0d..8bd892e 100644 (file)
@@ -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 (file)
index 865c883..0000000
+++ /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 (file)
index b069438..0000000
+++ /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
index 38a4053..2cc0010 100644 (file)
@@ -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 (file)
index fd1db59..0000000
+++ /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 difficult 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 difficult 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 different API, checking for error conditions beforehand, or handling
-errors in a different 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 different 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.
index af147db..e13b675 100644 (file)
@@ -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 (file)
index cb264ee..0000000
+++ /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
-  }
-}
index e9a9ed1..6a34222 100644 (file)
@@ -27,7 +27,6 @@ static_library("bugprone") {
     "DanglingHandleCheck.cpp",
     "DynamicStaticInitializersCheck.cpp",
     "EasilySwappableParametersCheck.cpp",
-    "EmptyCatchCheck.cpp",
     "ExceptionEscapeCheck.cpp",
     "FoldInitTypeCheck.cpp",
     "ForwardDeclarationNamespaceCheck.cpp",