[clang] [C++20] Destroying delete can cause a type to be noexcept when deleting (PR #118687)
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 5 08:54:30 PST 2024
https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/118687
>From 10985bcd5ae13f4533a9df4fa3035bd49e49af15 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Wed, 4 Dec 2024 14:31:48 -0500
Subject: [PATCH 1/5] [C++20] Deleting destructors can be noexcept
Given a `noexcept` operator with an operand that calls `delete`, Clang
was not considering whether the selected `operator delete` function was
a destroying delete or not when inspecting whether the deleted object
type has a throwing destructor. Thus, the operator would return `false`
for a type with a potentially throwing destructor even though that
destructor would not be called due to the destroying delete. Clang now
takes the kind of delete operator into consideration.
Fixes #118660
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/lib/Sema/SemaExceptionSpec.cpp | 5 +++--
.../SemaCXX/noexcept-destroying-delete.cpp | 21 +++++++++++++++++++
3 files changed, 27 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/noexcept-destroying-delete.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 755418e9550cf4..ea5f8f921c4d31 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -654,6 +654,9 @@ Bug Fixes in This Version
- Fixed a crash when GNU statement expression contains invalid statement (#GH113468).
- Fixed a failed assertion when using ``__attribute__((noderef))`` on an
``_Atomic``-qualified type (#GH116124).
+- No longer return ``false`` for ``noexcept`` expressions involving a
+ ``delete`` which resolves to a destroying delete but the type of the object
+ being deleted as a potentially throwing destructor (#GH118660).
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index ecfd79a50542c4..3e55e8b3d4d2ee 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -1205,11 +1205,12 @@ CanThrowResult Sema::canThrow(const Stmt *S) {
if (DTy.isNull() || DTy->isDependentType()) {
CT = CT_Dependent;
} else {
- CT = canCalleeThrow(*this, DE, DE->getOperatorDelete());
+ const FunctionDecl *OperatorDelete = DE->getOperatorDelete();
+ CT = canCalleeThrow(*this, DE, OperatorDelete);
if (const RecordType *RT = DTy->getAs<RecordType>()) {
const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
const CXXDestructorDecl *DD = RD->getDestructor();
- if (DD)
+ if (DD && !OperatorDelete->isDestroyingOperatorDelete())
CT = mergeCanThrow(CT, canCalleeThrow(*this, DE, DD));
}
if (CT == CT_Can)
diff --git a/clang/test/SemaCXX/noexcept-destroying-delete.cpp b/clang/test/SemaCXX/noexcept-destroying-delete.cpp
new file mode 100644
index 00000000000000..6a0902bfc01467
--- /dev/null
+++ b/clang/test/SemaCXX/noexcept-destroying-delete.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions -Wno-unevaluated-expression -std=c++20 %s
+// expected-no-diagnostics
+
+namespace std {
+ struct destroying_delete_t {
+ explicit destroying_delete_t() = default;
+ };
+
+ inline constexpr destroying_delete_t destroying_delete{};
+}
+
+struct Explicit {
+ ~Explicit() noexcept(false) {}
+
+ void operator delete(Explicit*, std::destroying_delete_t) noexcept {
+ }
+};
+
+Explicit *qn = nullptr;
+// This assertion used to fail, see GH118660
+static_assert(noexcept(delete(qn)));
>From 1de3f1fa012bfeb6693d2bad7b762bee02e4e556 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Wed, 4 Dec 2024 14:56:21 -0500
Subject: [PATCH 2/5] Address review feedback
* Fixes a typo in the release notes
* Minor optimization
---
clang/docs/ReleaseNotes.rst | 2 +-
clang/lib/Sema/SemaExceptionSpec.cpp | 15 ++++++++-------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ea5f8f921c4d31..381f48e2cd206b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -656,7 +656,7 @@ Bug Fixes in This Version
``_Atomic``-qualified type (#GH116124).
- No longer return ``false`` for ``noexcept`` expressions involving a
``delete`` which resolves to a destroying delete but the type of the object
- being deleted as a potentially throwing destructor (#GH118660).
+ being deleted has a potentially throwing destructor (#GH118660).
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 3e55e8b3d4d2ee..e0797209c2c9ee 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -1206,15 +1206,16 @@ CanThrowResult Sema::canThrow(const Stmt *S) {
CT = CT_Dependent;
} else {
const FunctionDecl *OperatorDelete = DE->getOperatorDelete();
- CT = canCalleeThrow(*this, DE, OperatorDelete);
- if (const RecordType *RT = DTy->getAs<RecordType>()) {
- const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
- const CXXDestructorDecl *DD = RD->getDestructor();
- if (DD && !OperatorDelete->isDestroyingOperatorDelete())
+ if (!OperatorDelete->isDestroyingOperatorDelete()) {
+ CT = canCalleeThrow(*this, DE, OperatorDelete);
+ if (const RecordType *RT = DTy->getAs<RecordType>()) {
+ const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
+ const CXXDestructorDecl *DD = RD->getDestructor();
CT = mergeCanThrow(CT, canCalleeThrow(*this, DE, DD));
+ }
+ if (CT == CT_Can)
+ return CT;
}
- if (CT == CT_Can)
- return CT;
}
return mergeCanThrow(CT, canSubStmtsThrow(*this, DE));
}
>From f463154f228d1db4422c806213e5e94b46875113 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Wed, 4 Dec 2024 15:07:52 -0500
Subject: [PATCH 3/5] Fix think-o and add test coverage to ensure it doesn't
happen again
---
clang/lib/Sema/SemaExceptionSpec.cpp | 2 +-
clang/test/SemaCXX/noexcept-destroying-delete.cpp | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index e0797209c2c9ee..5d658c0383bab8 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -1206,8 +1206,8 @@ CanThrowResult Sema::canThrow(const Stmt *S) {
CT = CT_Dependent;
} else {
const FunctionDecl *OperatorDelete = DE->getOperatorDelete();
+ CT = canCalleeThrow(*this, DE, OperatorDelete);
if (!OperatorDelete->isDestroyingOperatorDelete()) {
- CT = canCalleeThrow(*this, DE, OperatorDelete);
if (const RecordType *RT = DTy->getAs<RecordType>()) {
const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
const CXXDestructorDecl *DD = RD->getDestructor();
diff --git a/clang/test/SemaCXX/noexcept-destroying-delete.cpp b/clang/test/SemaCXX/noexcept-destroying-delete.cpp
index 6a0902bfc01467..92ccbc1fb3f96b 100644
--- a/clang/test/SemaCXX/noexcept-destroying-delete.cpp
+++ b/clang/test/SemaCXX/noexcept-destroying-delete.cpp
@@ -19,3 +19,15 @@ struct Explicit {
Explicit *qn = nullptr;
// This assertion used to fail, see GH118660
static_assert(noexcept(delete(qn)));
+
+struct ThrowingDestroyingDelete {
+ ~ThrowingDestroyingDelete() noexcept(false) {}
+
+ void operator delete(ThrowingDestroyingDelete*, std::destroying_delete_t) noexcept(false) {
+ }
+};
+
+ThrowingDestroyingDelete *pn = nullptr;
+// noexcept should return false here because the destroying delete itself is a
+// potentially throwing function.
+static_assert(!noexcept(delete(pn)));
>From b9be245c623dd168a33e83d887d0c3705c86f110 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Thu, 5 Dec 2024 07:52:44 -0500
Subject: [PATCH 4/5] Re-add accidentally removed code; fixes precommit CI test
---
clang/lib/Sema/SemaExceptionSpec.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 5d658c0383bab8..d773a45227b68d 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -1211,7 +1211,8 @@ CanThrowResult Sema::canThrow(const Stmt *S) {
if (const RecordType *RT = DTy->getAs<RecordType>()) {
const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
const CXXDestructorDecl *DD = RD->getDestructor();
- CT = mergeCanThrow(CT, canCalleeThrow(*this, DE, DD));
+ if (DD)
+ CT = mergeCanThrow(CT, canCalleeThrow(*this, DE, DD));
}
if (CT == CT_Can)
return CT;
>From ecf14d139439383227eead913a3ceac722c8b7a0 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Thu, 5 Dec 2024 11:53:59 -0500
Subject: [PATCH 5/5] Update based on review feedback; NFC
---
clang/lib/Sema/SemaExceptionSpec.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index d773a45227b68d..6a9f43d6f5215e 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -1208,10 +1208,8 @@ CanThrowResult Sema::canThrow(const Stmt *S) {
const FunctionDecl *OperatorDelete = DE->getOperatorDelete();
CT = canCalleeThrow(*this, DE, OperatorDelete);
if (!OperatorDelete->isDestroyingOperatorDelete()) {
- if (const RecordType *RT = DTy->getAs<RecordType>()) {
- const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
- const CXXDestructorDecl *DD = RD->getDestructor();
- if (DD)
+ if (const auto *RD = DTy->getAsCXXRecordDecl()) {
+ if (const CXXDestructorDecl *DD = RD->getDestructor())
CT = mergeCanThrow(CT, canCalleeThrow(*this, DE, DD));
}
if (CT == CT_Can)
More information about the cfe-commits
mailing list