[C++20] Destroying delete can cause a type to be noexcept when deleting

[AaronBallman]

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

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

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

---

Full diff: https://github.com/llvm/llvm-project/pull/118687.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/lib/Sema/SemaExceptionSpec.cpp (+3-2)
• (added) clang/test/SemaCXX/noexcept-destroying-delete.cpp (+21)

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)));

[shafik]

We already have

https://github.com/llvm/llvm-project/blob/main/clang/test/CXX/expr/expr.unary/expr.unary.noexcept/sema.cpp

maybe the test is better there and wrapped in a GH118660 namespace?

[AaronBallman]

That test file has a lot of dynamic exception specification code in it, so it's leaning heavily on -std=c++11.

[zygoloid]

Hm, the issue raised in #118800 is relevant here too, isn't it? If the destructor is virtual, we want to look at its exception specification, not the one on the destroying operator delete, because delete p actually invokes the virtual deleting destructor, not the statically selected deallocation function.

Maybe it's best to handle that in #118800, but I think it basically means the changes we made here aren't right :(