llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Andrey Karlov (negativ) <details> <summary>Changes</summary> While flagging empty `catch` blocks is generally a great rule, it produces false positives for destructors, where this pattern is often the only correct implementation. ### The Rationale: - Destructors are frequently called during stack unwinding after another exception has already been thrown. - If a destructor itself throws while another exception is active, the C++ runtime immediately calls `std::terminate`. - Therefore, to guarantee program stability, any code within a destructor that could potentially throw must be wrapped in a `try...catch` block. - Since there's often no adequate way to recover or report an error from a destructor (for e.g. for `std::bad_alloc`), "swallowing" the exception is the standard/safest approach. ### Proposal: Skip checks for `catch` blocks within destructors. --- Full diff: https://github.com/llvm/llvm-project/pull/161379.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp (+1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp (+9) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp index eebab847d1070..48dc3bdfdf49e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp @@ -90,6 +90,7 @@ void EmptyCatchCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( cxxCatchStmt(unless(isExpansionInSystemHeader()), unless(isInMacro()), unless(hasCaughtType(IgnoredExceptionType)), + unless(hasAncestor(cxxDestructorDecl())), hasHandler(compoundStmt( statementCountIs(0), unless(hasAnyTextFromList(IgnoreCatchWithKeywords))))) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c3a6d2f9b2890..426c97225c0e1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -244,6 +244,10 @@ Changes in existing checks correcting a spelling mistake on its option ``NamePrefixSuffixSilenceDissimilarityTreshold``. +- Improved :doc:`bugprone-empty-catch + <clang-tidy/checks/bugprone/empty-catch>` check by allowing empty + ``catch`` blocks in destructors. + - Improved :doc:`bugprone-exception-escape <clang-tidy/checks/bugprone/exception-escape>` check's handling of lambdas: exceptions from captures are now diagnosed, exceptions in the bodies of 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 index 8ab38229b6dbf..1319496269d86 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp @@ -65,3 +65,12 @@ void functionWithComment2() { // @IGNORE: relax its safe } } + +struct StructWithEmptyCatchInDestructor { + ~StructWithEmptyCatchInDestructor() { + try { + } + catch (...) { + } + } +}; `````````` </details> https://github.com/llvm/llvm-project/pull/161379 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
