PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp, isuckatcs, JonasToth,
baloghadamsoftware.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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: https://github.com/llvm/llvm-project/issues/40583
https://github.com/llvm/llvm-project/issues/55143
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D153423
Files:
clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -152,7 +152,7 @@
}
// FIXME: In this case 'a' is convertible to the handler and should be caught
-// but in reality it's thrown. Note that clang doesn't report a warning for
+// but in reality it's thrown. Note that clang doesn't report a warning for
// this either.
void throw_catch_multi_ptr_5() noexcept {
try {
@@ -249,7 +249,7 @@
void throw_derived_catch_base_ptr_c() noexcept {
try {
derived d;
- throw &d;
+ throw &d;
} catch(const base *) {
}
}
@@ -259,7 +259,7 @@
try {
derived d;
const derived *p = &d;
- throw p;
+ throw p;
} catch(base *) {
}
}
@@ -282,7 +282,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private' which should not throw exceptions
try {
B b;
- throw b;
+ throw b;
} catch(A) {
}
}
@@ -291,7 +291,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private_ptr' which should not throw exceptions
try {
B b;
- throw &b;
+ throw &b;
} catch(A *) {
}
}
@@ -300,7 +300,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected' which should not throw exceptions
try {
C c;
- throw c;
+ throw c;
} catch(A) {
}
}
@@ -309,7 +309,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected_ptr' which should not throw exceptions
try {
C c;
- throw &c;
+ throw &c;
} catch(A *) {
}
}
@@ -318,7 +318,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous' which should not throw exceptions
try {
E e;
- throw e;
+ throw e;
} catch(A) {
}
}
@@ -327,7 +327,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous_ptr' which should not throw exceptions
try {
E e;
- throw e;
+ throw e;
} catch(A) {
}
}
@@ -703,3 +703,28 @@
throw 1;
return 0;
}
+
+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
+};
+
+}}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
@@ -183,7 +183,6 @@
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;
}
};
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
@@ -11,7 +11,7 @@
* Move assignment operators
* The ``main()`` functions
* ``swap()`` functions
-* Functions marked with ``throw()`` or ``noexcept``
+* Functions marked with ``throw()``, ``noexcept``, ``noexcept(true)``
* Other functions given as option
A destructor throwing an exception may result in undefined behavior, resource
@@ -22,6 +22,12 @@
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 and 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
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -254,7 +254,7 @@
- Improved :doc:`bugprone-exception-escape
<clang-tidy/checks/bugprone/exception-escape>` to not emit warnings for
- forward declarations of functions.
+ forward declarations of functions and explicitly declared throwing functions.
- Fixed :doc:`bugprone-exception-escape<clang-tidy/checks/bugprone/exception-escape>`
for coroutines where previously a warning would be emitted with coroutines
Index: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -15,15 +15,27 @@
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) {
+ switch (Node.getExceptionSpecType()) {
+ case EST_Dynamic:
+ case EST_MSAny:
+ case EST_NoexceptFalse:
+ return Node.getExceptionSpecSourceRange().isValid();
+ default:
+ return false;
+ }
+}
+
} // namespace
-namespace tidy::bugprone {
ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), RawFunctionsThatShouldNotThrow(Options.get(
@@ -53,10 +65,12 @@
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 +91,4 @@
<< MatchedDecl;
}
-} // namespace tidy::bugprone
-} // namespace clang
+} // namespace clang::tidy::bugprone
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
