https://github.com/tharunvk created https://github.com/llvm/llvm-project/pull/190580
This PR introduces a new clang-tidy check, readability-use-rethrow, which identifies instances where a caught exception is re-thrown using its variable name (e.g., throw e;) instead of a bare throw; Fixes : https://github.com/llvm/llvm-project/issues/189672 >From 8c22122ed7d1d54b99e290593b1e5696590cc9b6 Mon Sep 17 00:00:00 2001 From: Tharun V K <[email protected]> Date: Mon, 6 Apr 2026 10:58:35 +0530 Subject: [PATCH] [clang-tidy] Add readability-use-rethrow check --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 2 + .../readability/UseRethrowCheck.cpp | 43 ++++++++++++++ .../clang-tidy/readability/UseRethrowCheck.h | 34 +++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/use-rethrow.rst | 32 +++++++++++ .../checkers/readability/use-rethrow.cpp | 56 +++++++++++++++++++ 8 files changed, 175 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/UseRethrowCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/UseRethrowCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/use-rethrow.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/use-rethrow.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 686e7c19d650b..ac7dc58999a8f 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -64,6 +64,7 @@ add_clang_library(clangTidyReadabilityModule STATIC UppercaseLiteralSuffixCheck.cpp UseAnyOfAllOfCheck.cpp UseConcisePreprocessorDirectivesCheck.cpp + UseRethrowCheck.cpp UseStdMinMaxCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 8e9e00b23c84a..bbe65cdc5ee2e 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -66,6 +66,7 @@ #include "UppercaseLiteralSuffixCheck.h" #include "UseAnyOfAllOfCheck.h" #include "UseConcisePreprocessorDirectivesCheck.h" +#include "UseRethrowCheck.h" #include "UseStdMinMaxCheck.h" namespace clang::tidy { @@ -191,6 +192,7 @@ class ReadabilityModule : public ClangTidyModule { "readability-use-anyofallof"); CheckFactories.registerCheck<UseConcisePreprocessorDirectivesCheck>( "readability-use-concise-preprocessor-directives"); + CheckFactories.registerCheck<UseRethrowCheck>("readability-use-rethrow"); CheckFactories.registerCheck<UseStdMinMaxCheck>( "readability-use-std-min-max"); } diff --git a/clang-tools-extra/clang-tidy/readability/UseRethrowCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseRethrowCheck.cpp new file mode 100644 index 0000000000000..e5d2b4b634f60 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseRethrowCheck.cpp @@ -0,0 +1,43 @@ +//===----------------------------------------------------------------------===// +// +// 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 "UseRethrowCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +void UseRethrowCheck::registerMatchers(MatchFinder *Finder) { + auto CatchVar = varDecl(isExceptionVariable(), hasType(referenceType())); + auto RefToVar = declRefExpr(to(CatchVar)); + + Finder->addMatcher( + cxxThrowExpr(has(expr(anyOf( + ignoringParenImpCasts(RefToVar), + cxxConstructExpr( + hasDeclaration(cxxConstructorDecl(anyOf( + isCopyConstructor(), isMoveConstructor()))), + hasArgument(0, ignoringParenImpCasts(RefToVar))))))) + .bind("throw"), + this); +} + +void UseRethrowCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedThrow = Result.Nodes.getNodeAs<CXXThrowExpr>("throw"); + + if (!MatchedThrow) + return; + + diag(MatchedThrow->getThrowLoc(), + "throwing a copy of the caught exception; use a bare 'throw' to rethrow " + "the original exception object") + << FixItHint::CreateReplacement(MatchedThrow->getSourceRange(), "throw"); +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/UseRethrowCheck.h b/clang-tools-extra/clang-tidy/readability/UseRethrowCheck.h new file mode 100644 index 0000000000000..a77b04dbd1dc8 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseRethrowCheck.h @@ -0,0 +1,34 @@ +//===----------------------------------------------------------------------===// +// +// 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_READABILITY_USERETHROWCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USERETHROWCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Detects cases where a caught exception is explicitly re-thrown as a new +/// copy, and suggests using a bare `throw;` instead. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/readability/use-rethrow.html +class UseRethrowCheck : public ClangTidyCheck { +public: + UseRethrowCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USERETHROWCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ad2ee2ae4b35e..5a4d94844fc92 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -175,6 +175,12 @@ New checks Checks for presence or absence of trailing commas in enum definitions and initializer lists. +- New :doc:`readability-use-rethrow + <clang-tidy/checks/readability/use-rethrow>` check. + + Detects cases where a caught exception object is re-thrown using the variable + name, and suggests using a bare ``throw;`` instead. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2b5be931271ec..9720f15193a50 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -434,6 +434,7 @@ Clang-Tidy Checks :doc:`readability-uniqueptr-delete-release <readability/uniqueptr-delete-release>`, "Yes" :doc:`readability-uppercase-literal-suffix <readability/uppercase-literal-suffix>`, "Yes" :doc:`readability-use-anyofallof <readability/use-anyofallof>`, + :doc:`readability-use-rethrow <readability/use-rethrow>`, "Yes" :doc:`readability-use-concise-preprocessor-directives <readability/use-concise-preprocessor-directives>`, "Yes" :doc:`readability-use-std-min-max <readability/use-std-min-max>`, "Yes" :doc:`zircon-temporary-objects <zircon/temporary-objects>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-rethrow.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-rethrow.rst new file mode 100644 index 0000000000000..12cf6d44b58d1 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-rethrow.rst @@ -0,0 +1,32 @@ +.. title:: clang-tidy - readability-use-rethrow + +readability-use-rethrow +======================= + +Detects cases where a caught exception is explicitly re-thrown as a new copy, and suggests using a bare ``throw;`` instead. + +Throwing a caught exception by its variable name (e.g., ``throw e;``) instead of using a bare ``throw;`` creates a new copy of the exception. This can lead to object slicing if the exception was derived from the caught type, and it alters the original stack trace of the exception. + +This check only flags exceptions caught by reference (``const`` or non-``const``) to avoid false positives where an exception is caught by value, modified, and then thrown again. + +Example: + +.. code-block:: c++ + + try { + // ... + } catch (const std::exception &e) { + log(e.what()); + throw e; // Warning: throwing a copy of the caught exception + } + +Is transformed to: + +.. code-block:: c++ + + try { + // ... + } catch (const std::exception &e) { + log(e.what()); + throw; + } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-rethrow.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-rethrow.cpp new file mode 100644 index 0000000000000..901eb5d8493df --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-rethrow.cpp @@ -0,0 +1,56 @@ +// RUN: %check_clang_tidy %s readability-use-rethrow %t + +namespace std { +class exception { +public: + virtual ~exception() = default; + virtual const char* what() const { return "exception"; } +}; +} // namespace std + +void log(const char*); +void f(); + +void test_const_reference() { + try { + f(); + } catch (const std::exception &e) { + log(e.what()); + throw e; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: throwing a copy of the caught exception; use a bare 'throw' to rethrow the original exception object [readability-use-rethrow] + // CHECK-FIXES: throw; + } +} + +void test_non_const_reference() { + try { + f(); + } catch (std::exception &e) { + throw e; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: throwing a copy of the caught exception; use a bare 'throw' to rethrow the original exception object [readability-use-rethrow] + // CHECK-FIXES: throw; + } +} + +void test_catch_by_value() { + try { + f(); + } catch (int i) { + i = 67; + // We should NOT warn here. + throw i; + } catch (std::exception e) { + // Should NOT warn here either, as 'e' was caught by value. + throw e; + } +} + +void test_unrelated_throw() { + try { + f(); + } catch (const std::exception &e) { + std::exception other; + // Should NOT warn because we are throwing a different object. + throw other; + } +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
