[clang-tools-extra] f256fee - [clang-tidy] Add bugprone-empty-catch check
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 23 11:04:17 PDT 2023
Author: Piotr Zegar
Date: 2023-07-23T18:02:10Z
New Revision: f256fee5343033bf8a31aee06a80f3e982b76f82
URL: https://github.com/llvm/llvm-project/commit/f256fee5343033bf8a31aee06a80f3e982b76f82
DIFF: https://github.com/llvm/llvm-project/commit/f256fee5343033bf8a31aee06a80f3e982b76f82.diff
LOG: [clang-tidy] Add bugprone-empty-catch check
Detects and suggests addressing issues with empty catch statements.
Reviewed By: xgupta
Differential Revision: https://reviews.llvm.org/D144748
Added:
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
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 7509e94950e10e..0cb0924d445e5e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -20,6 +20,7 @@
#include "DanglingHandleCheck.h"
#include "DynamicStaticInitializersCheck.h"
#include "EasilySwappableParametersCheck.h"
+#include "EmptyCatchCheck.h"
#include "ExceptionEscapeCheck.h"
#include "FoldInitTypeCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
@@ -106,6 +107,7 @@ 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 8bd892eeb41ecd..4076e0d253584b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -15,6 +15,7 @@ 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
new file mode 100644
index 00000000000000..865c88391b0b4b
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
@@ -0,0 +1,110 @@
+//===--- 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
new file mode 100644
index 00000000000000..b0694384f5c2f7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h
@@ -0,0 +1,37 @@
+//===--- 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 2cc0010884a7a0..38a40539aca425 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -114,6 +114,11 @@ 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
new file mode 100644
index 00000000000000..fd1db594ddb241
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
@@ -0,0 +1,149 @@
+.. 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 e13b6750196f40..af147db71eb726 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@ 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
new file mode 100644
index 00000000000000..cb264eeb4dc398
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
@@ -0,0 +1,67 @@
+// 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
+ }
+}
More information about the cfe-commits
mailing list