PiotrZSL updated this revision to Diff 534059.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.
Herald added a project: clang.
Review fixes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153423/new/
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
clang/include/clang/Basic/ExceptionSpecificationType.h
Index: clang/include/clang/Basic/ExceptionSpecificationType.h
===================================================================
--- clang/include/clang/Basic/ExceptionSpecificationType.h
+++ clang/include/clang/Basic/ExceptionSpecificationType.h
@@ -50,6 +50,11 @@
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,
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
@@ -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, 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
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,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::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 @@
<< 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
