[clang] ff40c34 - [clang-tidy] Allow explicit throwing in bugprone-exception-escape for special functions
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 18 14:11:15 PDT 2023
Author: Piotr Zegar
Date: 2023-07-18T21:11:08Z
New Revision: ff40c34881772e0641027327cd1791f4acacf6ff
URL: https://github.com/llvm/llvm-project/commit/ff40c34881772e0641027327cd1791f4acacf6ff
DIFF: https://github.com/llvm/llvm-project/commit/ff40c34881772e0641027327cd1791f4acacf6ff.diff
LOG: [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
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
index 635cc2ca05d120..90bf523ffb00b6 100644
--- 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 abe815e7cba756..1c542d4c9f2f30 100644
--- 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 52f3ceff281494..e6aa8e001492a6 100644
--- 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 2e748a2374bf9e..222577b124dced 100644
--- 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 39cf5476eb6ea9..7099545fce15bb 100644
--- 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 5616860555c8a4..d3c9e9cd063b42 100644
--- 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,
More information about the cfe-commits
mailing list