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.ze...@nokia.com>
Date: Sun, 24 Mar 2024 18:39:54 +0000
Subject: [PATCH 1/7] [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.ze...@nokia.com>
Date: Sun, 24 Mar 2024 18:43:47 +0000
Subject: [PATCH 2/7] 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.ze...@nokia.com>
Date: Sun, 31 Mar 2024 12:12:43 +0000
Subject: [PATCH 3/7] 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.ze...@nokia.com>
Date: Sun, 28 Apr 2024 16:46:08 +0000
Subject: [PATCH 4/7] 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 <m...@piotrzegar.pl>
Date: Tue, 21 May 2024 22:18:43 +0200
Subject: [PATCH 5/7] Update
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
Co-authored-by: isuckatcs <65320245+isucka...@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 <m...@piotrzegar.pl>
Date: Tue, 21 May 2024 22:18:52 +0200
Subject: [PATCH 6/7] Update
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
Co-authored-by: isuckatcs <65320245+isucka...@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 <m...@piotrzegar.pl>
Date: Tue, 21 May 2024 22:19:14 +0200
Subject: [PATCH 7/7] Update
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-rethrow.rst
Co-authored-by: isuckatcs <65320245+isucka...@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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
