[clang-tools-extra] [clang-tidy] Added bugprone-exception-rethrow check (PR #86448)
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Tue May 21 13:20:22 PDT 2024
https://github.com/PiotrZSL updated https://github.com/llvm/llvm-project/pull/86448
>From 1b5c351acca7375c577111a4c55506146ef108ef Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sun, 24 Mar 2024 18:39:54 +0000
Subject: [PATCH 01/10] [clang-tidy] Added bugprone-exception-rethrow check
Identifies problematic exception rethrowing, especially with caught
exception variables or empty throw statements outside catch blocks.
Closes #71292
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
.../bugprone/ExceptionRethrowCheck.cpp | 50 ++++++++++++++
.../bugprone/ExceptionRethrowCheck.h | 37 ++++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++
.../checks/bugprone/exception-rethrow.rst | 68 +++++++++++++++++++
.../docs/clang-tidy/checks/list.rst | 1 +
.../checkers/bugprone/exception-rethrow.cpp | 60 ++++++++++++++++
8 files changed, 226 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 1b92d2e60cc17..7466d3f2e4fc2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -26,6 +26,7 @@
#include "EasilySwappableParametersCheck.h"
#include "EmptyCatchCheck.h"
#include "ExceptionEscapeCheck.h"
+#include "ExceptionRethrowCheck.h"
#include "FoldInitTypeCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
#include "ForwardingReferenceOverloadCheck.h"
@@ -127,6 +128,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck<EmptyCatchCheck>("bugprone-empty-catch");
CheckFactories.registerCheck<ExceptionEscapeCheck>(
"bugprone-exception-escape");
+ CheckFactories.registerCheck<ExceptionRethrowCheck>(
+ "bugprone-exception-rethrow");
CheckFactories.registerCheck<FoldInitTypeCheck>("bugprone-fold-init-type");
CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
"bugprone-forward-declaration-namespace");
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 2d303191f8865..345ae420398e6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -20,6 +20,7 @@ add_clang_library(clangTidyBugproneModule
EasilySwappableParametersCheck.cpp
EmptyCatchCheck.cpp
ExceptionEscapeCheck.cpp
+ ExceptionRethrowCheck.cpp
FoldInitTypeCheck.cpp
ForwardDeclarationNamespaceCheck.cpp
ForwardingReferenceOverloadCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
new file mode 100644
index 0000000000000..4855ccc2724a9
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
@@ -0,0 +1,50 @@
+//===--- ExceptionRethrowCheck.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 "ExceptionRethrowCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(VarDecl, isExceptionVariable) { return Node.isExceptionVariable(); }
+} // namespace
+
+void ExceptionRethrowCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ cxxThrowExpr(unless(isExpansionInSystemHeader()),
+ anyOf(unless(has(expr())),
+ has(declRefExpr(to(varDecl(isExceptionVariable()))))),
+ optionally(hasAncestor(cxxCatchStmt().bind("catch"))))
+ .bind("throw"),
+ this);
+}
+
+void ExceptionRethrowCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedThrow = Result.Nodes.getNodeAs<CXXThrowExpr>("throw");
+
+ if (const Expr *ThrownObject = MatchedThrow->getSubExpr()) {
+ diag(MatchedThrow->getThrowLoc(),
+ "throwing a copy of the caught %0 exception, remove the argument to "
+ "throw the original exception object")
+ << ThrownObject->getType().getNonReferenceType();
+ return;
+ }
+
+ const bool HasCatchAnsestor =
+ Result.Nodes.getNodeAs<Stmt>("catch") != nullptr;
+ if (!HasCatchAnsestor) {
+ diag(MatchedThrow->getThrowLoc(),
+ "empty 'throw' outside a catch block without an exception can trigger "
+ "'std::terminate'");
+ }
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h
new file mode 100644
index 0000000000000..bbb30f779cf25
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h
@@ -0,0 +1,37 @@
+//===--- ExceptionRethrowCheck.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_EXCEPTIONRETHROWCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTIONRETHROWCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Identifies problematic exception rethrowing, especially with caught
+/// exception variables or empty throw statements outside catch blocks.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/exception-rethrow.html
+class ExceptionRethrowCheck : public ClangTidyCheck {
+public:
+ ExceptionRethrowCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus && LangOpts.CXXExceptions;
+ }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTIONRETHROWCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3038d2b125f20..776edb1da2ad0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,12 @@ New checks
Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP
can be constructed outside itself and the derived class.
+- New :doc:`bugprone-exception-rethrow
+ <clang-tidy/checks/bugprone/exception-rethrow>` check.
+
+ Identifies problematic exception rethrowing, especially with caught exception
+ variables or empty throw statements outside catch blocks.
+
- New :doc:`bugprone-return-const-ref-from-parameter
<clang-tidy/checks/bugprone/return-const-ref-from-parameter>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
new file mode 100644
index 0000000000000..981dfff852d58
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -0,0 +1,68 @@
+.. title:: clang-tidy - bugprone-exception-rethrow
+
+bugprone-exception-rethrow
+==========================
+
+Identifies problematic exception rethrowing, especially with caught exception
+variables or empty throw statements outside catch blocks.
+
+In C++ exception handling, a common pitfall occurs when developers rethrow
+caught exceptions within catch blocks by directly passing the caught exception
+variable to the ``throw`` statement. While this approach can propagate
+exceptions to higher levels of the program, it often leads to code that is less
+clear and more error-prone. Rethrowing caught exceptions with the same exception
+object within catch blocks can obscure the original context of the exception and
+make it challenging to trace program flow. Additionally, this method can
+introduce issues such as exception object slicing and performance overhead due
+to the invocation of the copy constructor.
+
+.. code-block:: c++
+
+ try {
+ // Code that may throw an exception
+ } catch (const std::exception& e) {
+ throw e; // Bad
+ }
+
+To prevent these issues, it is advisable to utilize ``throw;`` statements to
+rethrow the original exception object for currently handled exceptions.
+
+.. code-block:: c++
+
+ try {
+ // Code that may throw an exception
+ } catch (const std::exception& e) {
+ throw; // Good
+ }
+
+However, when empty throw statement is used outside of a catch block, it
+will result in a call to ``std::terminate()``, which abruptly terminates the
+application. This behavior can lead to abnormal termination of the program and
+is often unintended. Such occurrences may indicate errors or oversights in the
+exception handling logic, and it is essential to avoid empty throw statements
+outside catch blocks to prevent unintended program termination.
+
+.. code-block:: c++
+
+ void foo() {
+ // std::terminate will be called because there is no exception to rethrow
+ throw;
+ }
+
+ int main() {
+ try {
+ foo();
+ } catch(...) {
+ return 1;
+ }
+ return 0;
+ }
+
+Above program will be terminated with:
+
+.. code:: text
+
+ terminate called without an active exception
+ Aborted (core dumped)
+
+
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 49747ff896ba5..79e998a7d99c2 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -92,6 +92,7 @@ Clang-Tidy Checks
:doc:`bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters>`,
:doc:`bugprone-empty-catch <bugprone/empty-catch>`,
:doc:`bugprone-exception-escape <bugprone/exception-escape>`,
+ :doc:`bugprone-exception-rethrow <bugprone/exception-rethrow>`,
:doc:`bugprone-fold-init-type <bugprone/fold-init-type>`,
:doc:`bugprone-forward-declaration-namespace <bugprone/forward-declaration-namespace>`,
:doc:`bugprone-forwarding-reference-overload <bugprone/forwarding-reference-overload>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
new file mode 100644
index 0000000000000..5e7ba03652393
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
@@ -0,0 +1,60 @@
+// RUN: %check_clang_tidy %s bugprone-exception-rethrow %t -- -- -fexceptions
+
+struct exception {};
+
+void correct() {
+ try {
+ throw exception();
+ } catch(const exception &) {
+ throw;
+ }
+}
+
+void correct2() {
+ try {
+ throw exception();
+ } catch(const exception &e) {
+ try {
+ throw exception();
+ } catch(...) {}
+ }
+}
+
+void not_correct() {
+ try {
+ throw exception();
+ } catch(const exception &e) {
+ throw e;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'exception' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+ }
+}
+
+void not_correct2() {
+ try {
+ throw 5;
+ } catch(const int &e) {
+ throw e;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'int' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+ }
+}
+
+void rethrow_not_correct() {
+ throw;
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: empty 'throw' outside a catch block without an exception can trigger 'std::terminate' [bugprone-exception-rethrow]
+}
+
+void rethrow_not_correct2() {
+ try {
+ throw;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty 'throw' outside a catch block without an exception can trigger 'std::terminate' [bugprone-exception-rethrow]
+ } catch(...) {
+ }
+}
+
+void rethrow_correct() {
+ try {
+ throw 5;
+ } catch(...) {
+ throw;
+ }
+}
>From 3d1ae3f17ec41116cb22569a82a7e4c369a355a6 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <pitor.zegar at nokia.com>
Date: Sun, 24 Mar 2024 18:43:47 +0000
Subject: [PATCH 02/10] Fix code -> code-block in doc
---
.../docs/clang-tidy/checks/bugprone/exception-rethrow.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
index 981dfff852d58..68be515297bc5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -60,7 +60,7 @@ outside catch blocks to prevent unintended program termination.
Above program will be terminated with:
-.. code:: text
+.. code-block:: text
terminate called without an active exception
Aborted (core dumped)
>From 3a6573e01e5532a71bd226f15ab2ed5844a51e17 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sun, 31 Mar 2024 12:12:43 +0000
Subject: [PATCH 03/10] Fixes for review
---
.../bugprone/ExceptionRethrowCheck.cpp | 23 ++++++-----
.../bugprone/ExceptionRethrowCheck.h | 4 +-
.../checks/bugprone/exception-rethrow.rst | 1 -
.../checkers/bugprone/exception-rethrow.cpp | 38 ++++++++++++++++++-
4 files changed, 52 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
index 4855ccc2724a9..3e327606ee562 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
@@ -8,20 +8,26 @@
#include "ExceptionRethrowCheck.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
-namespace {
-AST_MATCHER(VarDecl, isExceptionVariable) { return Node.isExceptionVariable(); }
-} // namespace
-
void ExceptionRethrowCheck::registerMatchers(MatchFinder *Finder) {
+
+ auto RefToExceptionVariable = declRefExpr(to(varDecl(isExceptionVariable())));
+ auto StdMoveCall = callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))), hasArgument(0, RefToExceptionVariable));
+ auto CopyOrMoveConstruction = cxxConstructExpr(argumentCountIs(1), hasDeclaration(cxxConstructorDecl(anyOf(isCopyConstructor(), isMoveConstructor()))), hasArgument(0, anyOf(RefToExceptionVariable, StdMoveCall)));
+ auto FunctionCast = cxxFunctionalCastExpr( hasSourceExpression(anyOf(RefToExceptionVariable, StdMoveCall)));
+
Finder->addMatcher(
cxxThrowExpr(unless(isExpansionInSystemHeader()),
anyOf(unless(has(expr())),
- has(declRefExpr(to(varDecl(isExceptionVariable()))))),
+ has(RefToExceptionVariable),
+ has(StdMoveCall),
+ has(CopyOrMoveConstruction),
+ has(FunctionCast)),
optionally(hasAncestor(cxxCatchStmt().bind("catch"))))
.bind("throw"),
this);
@@ -38,12 +44,11 @@ void ExceptionRethrowCheck::check(const MatchFinder::MatchResult &Result) {
return;
}
- const bool HasCatchAnsestor =
+ const bool HasCatchAncestor =
Result.Nodes.getNodeAs<Stmt>("catch") != nullptr;
- if (!HasCatchAnsestor) {
+ if (!HasCatchAncestor) {
diag(MatchedThrow->getThrowLoc(),
- "empty 'throw' outside a catch block without an exception can trigger "
- "'std::terminate'");
+ "empty 'throw' outside a catch block with no operand triggers 'std::terminate()'");
}
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h
index bbb30f779cf25..efe9aad80d13d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.h
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTIONRETHROWCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTIONRETHROWCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTION_RETHROW_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTION_RETHROW_CHECK_H
#include "../ClangTidyCheck.h"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
index 68be515297bc5..677537094e5f6 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -65,4 +65,3 @@ Above program will be terminated with:
terminate called without an active exception
Aborted (core dumped)
-
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
index 5e7ba03652393..a5732d9b2f44b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
@@ -2,6 +2,13 @@
struct exception {};
+namespace std {
+ template <class T>
+ T&& move(T &x) {
+ return static_cast<T&&>(x);
+ }
+}
+
void correct() {
try {
throw exception();
@@ -30,6 +37,33 @@ void not_correct() {
}
void not_correct2() {
+ try {
+ throw exception();
+ } catch(const exception &e) {
+ throw (e);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'exception' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+ }
+}
+
+void not_correct3() {
+ try {
+ throw exception();
+ } catch(const exception &e) {
+ throw exception(e);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'exception' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+ }
+}
+
+void not_correct4() {
+ try {
+ throw exception();
+ } catch(exception &e) {
+ throw std::move(e);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 'exception' exception, remove the argument to throw the original exception object [bugprone-exception-rethrow]
+ }
+}
+
+void not_correct5() {
try {
throw 5;
} catch(const int &e) {
@@ -40,13 +74,13 @@ void not_correct2() {
void rethrow_not_correct() {
throw;
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: empty 'throw' outside a catch block without an exception can trigger 'std::terminate' [bugprone-exception-rethrow]
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: empty 'throw' outside a catch block with no operand triggers 'std::terminate()' [bugprone-exception-rethrow]
}
void rethrow_not_correct2() {
try {
throw;
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty 'throw' outside a catch block without an exception can trigger 'std::terminate' [bugprone-exception-rethrow]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty 'throw' outside a catch block with no operand triggers 'std::terminate()' [bugprone-exception-rethrow]
} catch(...) {
}
}
>From 17a7099228aa9d59a991ebbc59f3718419c4540b Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sun, 28 Apr 2024 16:46:08 +0000
Subject: [PATCH 04/10] Add support for copy constructors, std::move, and so on
---
.../bugprone/ExceptionRethrowCheck.cpp | 47 ++++++++++++-------
.../checkers/bugprone/exception-rethrow.cpp | 9 ++++
2 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
index 3e327606ee562..55f94306f395e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionRethrowCheck.cpp
@@ -17,26 +17,39 @@ namespace clang::tidy::bugprone {
void ExceptionRethrowCheck::registerMatchers(MatchFinder *Finder) {
auto RefToExceptionVariable = declRefExpr(to(varDecl(isExceptionVariable())));
- auto StdMoveCall = callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))), hasArgument(0, RefToExceptionVariable));
- auto CopyOrMoveConstruction = cxxConstructExpr(argumentCountIs(1), hasDeclaration(cxxConstructorDecl(anyOf(isCopyConstructor(), isMoveConstructor()))), hasArgument(0, anyOf(RefToExceptionVariable, StdMoveCall)));
- auto FunctionCast = cxxFunctionalCastExpr( hasSourceExpression(anyOf(RefToExceptionVariable, StdMoveCall)));
+ auto StdMoveCall =
+ callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))),
+ hasArgument(0, RefToExceptionVariable));
+ auto CopyOrMoveConstruction = cxxConstructExpr(
+ argumentCountIs(1),
+ traverse(TK_AsIs, hasDeclaration(cxxConstructorDecl(
+ anyOf(isCopyConstructor(), isMoveConstructor())))),
+ hasArgument(0, anyOf(RefToExceptionVariable, StdMoveCall)));
+ auto HasEmptyThrowExprDescendant =
+ hasDescendant(cxxThrowExpr(equalsBoundNode("empty-throw")));
+
+ Finder->addMatcher(
+ cxxThrowExpr(
+ unless(isExpansionInSystemHeader()), unless(has(expr())),
+ expr().bind("empty-throw"),
+ anyOf(unless(hasAncestor(cxxCatchStmt())),
+ hasAncestor(cxxCatchStmt(anyOf(
+ hasDescendant(functionDecl(HasEmptyThrowExprDescendant)),
+ hasDescendant(lambdaExpr(HasEmptyThrowExprDescendant))))))),
+ this);
Finder->addMatcher(
cxxThrowExpr(unless(isExpansionInSystemHeader()),
- anyOf(unless(has(expr())),
- has(RefToExceptionVariable),
- has(StdMoveCall),
- has(CopyOrMoveConstruction),
- has(FunctionCast)),
- optionally(hasAncestor(cxxCatchStmt().bind("catch"))))
+ has(expr(anyOf(RefToExceptionVariable, StdMoveCall,
+ CopyOrMoveConstruction))))
.bind("throw"),
this);
}
void ExceptionRethrowCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *MatchedThrow = Result.Nodes.getNodeAs<CXXThrowExpr>("throw");
-
- if (const Expr *ThrownObject = MatchedThrow->getSubExpr()) {
+ if (const auto *MatchedThrow =
+ Result.Nodes.getNodeAs<CXXThrowExpr>("throw")) {
+ const Expr *ThrownObject = MatchedThrow->getSubExpr();
diag(MatchedThrow->getThrowLoc(),
"throwing a copy of the caught %0 exception, remove the argument to "
"throw the original exception object")
@@ -44,11 +57,11 @@ void ExceptionRethrowCheck::check(const MatchFinder::MatchResult &Result) {
return;
}
- const bool HasCatchAncestor =
- Result.Nodes.getNodeAs<Stmt>("catch") != nullptr;
- if (!HasCatchAncestor) {
- diag(MatchedThrow->getThrowLoc(),
- "empty 'throw' outside a catch block with no operand triggers 'std::terminate()'");
+ if (const auto *MatchedEmptyThrow =
+ Result.Nodes.getNodeAs<CXXThrowExpr>("empty-throw")) {
+ diag(MatchedEmptyThrow->getThrowLoc(),
+ "empty 'throw' outside a catch block with no operand triggers "
+ "'std::terminate()'");
}
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
index a5732d9b2f44b..2914d2e7452b8 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
@@ -92,3 +92,12 @@ void rethrow_correct() {
throw;
}
}
+
+void rethrow_in_lambda() {
+ try {
+ throw 5;
+ } catch(...) {
+ auto lambda = [] { throw; };
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: empty 'throw' outside a catch block with no operand triggers 'std::terminate()' [bugprone-exception-rethrow]
+ }
+}
>From 5dd82533b3b013e2731575ee9803bbff4c6d5c08 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Tue, 21 May 2024 22:18:43 +0200
Subject: [PATCH 05/10] Update
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
Co-authored-by: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
---
.../checks/bugprone/exception-rethrow.rst | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
index 677537094e5f6..71719c3e6dde5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -21,9 +21,21 @@ to the invocation of the copy constructor.
try {
// Code that may throw an exception
} catch (const std::exception& e) {
- throw e; // Bad
+ throw e; // Bad, 'e' is copied
}
+.. code-block:: c++
+ class derived_exception : public std::exception { ... };
+
+ void throwDerived() { throw derived_exception{}; }
+
+ try {
+ throwDerived();
+ } catch (const std::exception& e) {
+ throw e; // Bad, exception slicing occurs as 'derived_exception' is rethrown as 'exception' (maybe with better wording ?)
+ }
+
+
To prevent these issues, it is advisable to utilize ``throw;`` statements to
rethrow the original exception object for currently handled exceptions.
>From ffd2e386d89373f62e9d096ba0eb240af7f98dd9 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Tue, 21 May 2024 22:18:52 +0200
Subject: [PATCH 06/10] Update
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
Co-authored-by: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
---
.../docs/clang-tidy/checks/bugprone/exception-rethrow.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
index 71719c3e6dde5..ec38e947dcfba 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -43,7 +43,7 @@ rethrow the original exception object for currently handled exceptions.
try {
// Code that may throw an exception
- } catch (const std::exception& e) {
+ } catch (const std::exception&) {
throw; // Good
}
>From 2b57dd0895b840eaacb7adf40fa902d17d2e3aa6 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Tue, 21 May 2024 22:19:14 +0200
Subject: [PATCH 07/10] Update
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
Co-authored-by: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
---
.../docs/clang-tidy/checks/bugprone/exception-rethrow.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
index ec38e947dcfba..4bee09840f2cf 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -47,8 +47,8 @@ rethrow the original exception object for currently handled exceptions.
throw; // Good
}
-However, when empty throw statement is used outside of a catch block, it
-will result in a call to ``std::terminate()``, which abruptly terminates the
+However, when an empty throw statement is used outside a catch block, it
+results in a call to ``std::terminate()``, which abruptly terminates the
application. This behavior can lead to abnormal termination of the program and
is often unintended. Such occurrences may indicate errors or oversights in the
exception handling logic, and it is essential to avoid empty throw statements
>From 80f40a8df60e3ef5fd69493ec188d9132341ad93 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Tue, 21 May 2024 22:19:26 +0200
Subject: [PATCH 08/10] Update
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
Co-authored-by: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
---
.../docs/clang-tidy/checks/bugprone/exception-rethrow.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
index 4bee09840f2cf..e31eaaf501f20 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -49,7 +49,7 @@ rethrow the original exception object for currently handled exceptions.
However, when an empty throw statement is used outside a catch block, it
results in a call to ``std::terminate()``, which abruptly terminates the
-application. This behavior can lead to abnormal termination of the program and
+application. This behavior can lead to the abnormal termination of the program and
is often unintended. Such occurrences may indicate errors or oversights in the
exception handling logic, and it is essential to avoid empty throw statements
outside catch blocks to prevent unintended program termination.
>From f36be60cdad0d03adeb806e89c30a20c5a9d5d35 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Tue, 21 May 2024 22:19:40 +0200
Subject: [PATCH 09/10] Update
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
Co-authored-by: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
---
.../docs/clang-tidy/checks/bugprone/exception-rethrow.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
index e31eaaf501f20..a09527c955856 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
@@ -70,7 +70,7 @@ outside catch blocks to prevent unintended program termination.
return 0;
}
-Above program will be terminated with:
+The above program will be terminated with:
.. code-block:: text
>From 9dc632a667e786fb852b3aaa07444a4aee34a9ea Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Tue, 21 May 2024 22:20:11 +0200
Subject: [PATCH 10/10] Update
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
Co-authored-by: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
---
.../test/clang-tidy/checkers/bugprone/exception-rethrow.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
index 2914d2e7452b8..de2f41d04ae50 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-rethrow.cpp
@@ -4,7 +4,7 @@ struct exception {};
namespace std {
template <class T>
- T&& move(T &x) {
+ T&& move(T &&x) {
return static_cast<T&&>(x);
}
}
More information about the cfe-commits
mailing list