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 <aa...@aaronballman.com> Date: Wed, 4 Dec 2024 14:31:48 -0500 Subject: [PATCH 1/3] [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 <aa...@aaronballman.com> Date: Wed, 4 Dec 2024 14:56:21 -0500 Subject: [PATCH 2/3] 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 <aa...@aaronballman.com> Date: Wed, 4 Dec 2024 15:07:52 -0500 Subject: [PATCH 3/3] 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))); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits