[clang-tidy] Allow explicit throwing in bugprone-exception-escape for special functions

Functions declared explicitly with noexcept(false) or throw(exception)
will be excluded from the analysis, as even though it is not recommended for
functions like swap, main, move constructors and assignment operators,
and destructors, it is a clear indication of the developer's intention and
should be respected.

Fixes: #40583, #55143

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D153423

diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
index 635cc2c..90bf523 100644 (file)
--- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -15,15 +15,21 @@
 
 using namespace clang::ast_matchers;
 
-namespace clang {
+namespace clang::tidy::bugprone {
 namespace {
+
 AST_MATCHER_P(FunctionDecl, isEnabled, llvm::StringSet<>,
               FunctionsThatShouldNotThrow) {
   return FunctionsThatShouldNotThrow.count(Node.getNameAsString()) > 0;
 }
+
+AST_MATCHER(FunctionDecl, isExplicitThrow) {
+  return isExplicitThrowExceptionSpec(Node.getExceptionSpecType()) &&
+         Node.getExceptionSpecSourceRange().isValid();
+}
+
 } // namespace
 
-namespace tidy::bugprone {
 ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
                                            ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), RawFunctionsThatShouldNotThrow(Options.get(
@@ -53,10 +59,12 @@ void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       functionDecl(isDefinition(),
-                   anyOf(isNoThrow(), cxxDestructorDecl(),
-                         cxxConstructorDecl(isMoveConstructor()),
-                         cxxMethodDecl(isMoveAssignmentOperator()),
-                         hasName("main"), hasName("swap"),
+                   anyOf(isNoThrow(),
+                         allOf(anyOf(cxxDestructorDecl(),
+                                     cxxConstructorDecl(isMoveConstructor()),
+                                     cxxMethodDecl(isMoveAssignmentOperator()),
+                                     isMain(), hasName("swap")),
+                               unless(isExplicitThrow())),
                          isEnabled(FunctionsThatShouldNotThrow)))
           .bind("thrower"),
       this);
@@ -77,5 +85,4 @@ void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) {
         << MatchedDecl;
 }
 
-} // namespace tidy::bugprone
-} // namespace clang
+} // namespace clang::tidy::bugprone

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index abe815e..1c542d4 100644 (file)
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -274,13 +274,10 @@ Changes in existing checks
   Global options of the same name should be used instead.
 
 - Improved :doc:`bugprone-exception-escape
-  <clang-tidy/checks/bugprone/exception-escape>` to not emit warnings for
-  forward declarations of functions and additionally modified it to skip
-  ``noexcept`` functions during call stack analysis.
-
-- Fixed :doc:`bugprone-exception-escape<clang-tidy/checks/bugprone/exception-escape>`
-  for coroutines where previously a warning would be emitted with coroutines
-  throwing exceptions in their bodies.
+  <clang-tidy/checks/bugprone/exception-escape>` check to not emit warnings for
+  forward declarations of functions, explicitly declared throwing functions,
+  coroutines throwing exceptions in their bodies and skip ``noexcept``
+  functions during call stack analysis.
 
 - Improved :doc:`bugprone-fold-init-type
   <clang-tidy/checks/bugprone/fold-init-type>` to handle iterators that do not

diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
index 52f3cef..e6aa8e0 100644 (file)
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
@@ -22,6 +22,12 @@ are always possible to implement in a non throwing way. Non throwing ``swap()``
 operations are also used to create move operations. A throwing ``main()``
 function also results in unexpected termination.
 
+Functions declared explicitly with ``noexcept(false)`` or ``throw(exception)``
+will be excluded from the analysis, as even though it is not recommended for
+functions like ``swap()``, ``main()``, move constructors, move assignment operators
+and destructors, it is a clear indication of the developer's intention and
+should be respected.
+
 WARNING! This check may be expensive on large source files.
 
 Options

diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
index 2e748a2..222577b 100644 (file)
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
@@ -183,7 +183,6 @@ struct Promise<Task, void, ThrowInPromiseConstructor, ThrowInInitialSuspend,
 
 struct Evil {
   ~Evil() noexcept(false) {
-    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function '~Evil' which should not throw exceptions
     throw 42;
   }
 };

diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
index 39cf547..7099545 100644 (file)
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -722,3 +722,27 @@ void calls_non_and_throwing() noexcept {
   test_basic_throw();
 }
 
+namespace PR55143 { namespace PR40583 {
+
+struct test_explicit_throw {
+    test_explicit_throw() throw(int) { throw 42; }
+    test_explicit_throw(const test_explicit_throw&) throw(int) { throw 42; }
+    test_explicit_throw(test_explicit_throw&&) throw(int) { throw 42; }
+    test_explicit_throw& operator=(const test_explicit_throw&) throw(int) { throw 42; }
+    test_explicit_throw& operator=(test_explicit_throw&&) throw(int) { throw 42; }
+    ~test_explicit_throw() throw(int) { throw 42; }
+};
+
+struct test_implicit_throw {
+    test_implicit_throw() { throw 42; }
+    test_implicit_throw(const test_implicit_throw&) { throw 42; }
+    test_implicit_throw(test_implicit_throw&&) { throw 42; }
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'test_implicit_throw' which should not throw exceptions
+    test_implicit_throw& operator=(const test_implicit_throw&) { throw 42; }
+    test_implicit_throw& operator=(test_implicit_throw&&) { throw 42; }
+    // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: an exception may be thrown in function 'operator=' which should not throw exceptions
+    ~test_implicit_throw() { throw 42; }
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function '~test_implicit_throw' which should not throw exceptions
+};
+
+}}

diff --git a/clang/include/clang/Basic/ExceptionSpecificationType.h b/clang/include/clang/Basic/ExceptionSpecificationType.h
index 5616860..d3c9e9c 100644 (file)
--- a/clang/include/clang/Basic/ExceptionSpecificationType.h
+++ b/clang/include/clang/Basic/ExceptionSpecificationType.h
@@ -50,6 +50,11 @@ inline bool isUnresolvedExceptionSpec(ExceptionSpecificationType ESpecType) {
   return ESpecType == EST_Unevaluated || ESpecType == EST_Uninstantiated;
 }
 
+inline bool isExplicitThrowExceptionSpec(ExceptionSpecificationType ESpecType) {
+  return ESpecType == EST_Dynamic || ESpecType == EST_MSAny ||
+         ESpecType == EST_NoexceptFalse;
+}
+
 /// Possible results from evaluation of a noexcept expression.
 enum CanThrowResult {
   CT_Cannot,