Never call a destroying operator delete when cleaning up from an

exception thrown during construction in a new-expression.

Instead, when performing deallocation function lookup for a
new-expression, ignore all destroying operator delete candidates, and
fall back to global operator delete if there is no member operator
delete other than a destroying operator delete.

Use of destroying operator delete only makes sense when there is an
object to destroy, which there isn't in this case. The language wording
doesn't cover this case; this oversight has been reported to WG21, with
the approach in this patch as the proposed fix.

Author: zygoloid

clang/lib/Sema/SemaExprCXX.cpp

@@ -2630,8 +2630,24 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
   if (FoundDelete.isAmbiguous())
     return true; // FIXME: clean up expressions?
 
+  // Filter out any destroying operator deletes. We can't possibly call such a
+  // function in this context, because we're handling the case where the object
+  // was not successfully constructed.
+  // FIXME: This is not covered by the language rules yet.
+  {
+    LookupResult::Filter Filter = FoundDelete.makeFilter();
+    while (Filter.hasNext()) {
+      auto *FD = dyn_cast<FunctionDecl>(Filter.next()->getUnderlyingDecl());
+      if (FD && FD->isDestroyingOperatorDelete())
+        Filter.erase();
+    }
+    Filter.done();
+  }
+
   bool FoundGlobalDelete = FoundDelete.empty();
   if (FoundDelete.empty()) {
+    FoundDelete.clear(LookupOrdinaryName);
+
     if (DeleteScope == AFS_Class)
       return true;
 

clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp

@@ -1,11 +1,11 @@
-// RUN: %clang_cc1 -std=c++2a -emit-llvm %s -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ITANIUM,CHECK-64BIT
-// RUN: %clang_cc1 -std=c++2a -emit-llvm %s -triple x86_64-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI64,CHECK-64BIT
-// RUN: %clang_cc1 -std=c++2a -emit-llvm %s -triple i386-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI32,CHECK-32BIT
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ITANIUM,CHECK-64BIT
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple x86_64-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI64,CHECK-64BIT
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple i386-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI32,CHECK-32BIT
 
 // PR46908: ensure the IR passes the verifier with optimizations enabled.
-// RUN: %clang_cc1 -std=c++2a -emit-llvm-only %s -triple x86_64-linux-gnu -O2
-// RUN: %clang_cc1 -std=c++2a -emit-llvm-only %s -triple x86_64-windows -O2
-// RUN: %clang_cc1 -std=c++2a -emit-llvm-only %s -triple i386-windows -O2
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple x86_64-linux-gnu -O2
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple x86_64-windows -O2
+// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple i386-windows -O2
 
 namespace std {
   using size_t = decltype(sizeof(0));
@@ -102,6 +102,41 @@ void delete_D(D *d) { delete d; }
 // CHECK-NOT: call
 // CHECK: }
 
+struct J {
+  J(); // might throw
+  void operator delete(J *, std::destroying_delete_t);
+};
+
+// CHECK-ITANIUM-LABEL: define {{.*}}@_Z1j
+// CHECK-MSABI-LABEL: define {{.*}}@"?j@@
+J *j() {
+  // CHECK-ITANIUM: invoke {{.*}}@_ZN1JC1Ev(
+  // CHECK-ITANIUM: call {{.*}}@_ZdlPv(
+  // CHECK-NOT: }
+  // CHECK-MSABI: invoke {{.*}}@"??0J@@Q{{AE|EAA}}@XZ"(
+  // CHECK-MSABI: call {{.*}}@"??3@YAXP{{E?}}AX@Z"(
+  return new J;
+  // CHECK: }
+}
+
+struct K {
+  K(); // might throw
+  void operator delete(void *);
+  void operator delete(K *, std::destroying_delete_t);
+};
+
+// CHECK-ITANIUM-LABEL: define {{.*}}@_Z1k
+// CHECK-MSABI-LABEL: define {{.*}}@"?k@@
+K *k() {
+  // CHECK-ITANIUM: invoke {{.*}}@_ZN1KC1Ev(
+  // CHECK-ITANIUM: call {{.*}}@_ZN1KdlEPv(
+  // CHECK-NOT: }
+  // CHECK-MSABI: invoke {{.*}}@"??0K@@Q{{AE|EAA}}@XZ"(
+  // CHECK-MSABI: call {{.*}}@"??3K@@SAXP{{E?}}AX@Z"(
+  return new K;
+  // CHECK: }
+}
+
 struct E { void *data; };
 struct F { void operator delete(F *, std::destroying_delete_t, std::size_t, std::align_val_t); void *data; };
 struct alignas(16) G : E, F { void *data; };
@@ -127,8 +162,8 @@ H::~H() { call_in_dtor(); }
 // CHECK-ITANIUM-NOT: call
 // CHECK-ITANIUM: }
 
-// CHECK-MSABI64: define {{.*}} @"??_GH@@UEAAPEAXI@Z"(
-// CHECK-MSABI32: define {{.*}} @"??_GH@@UAEPAXI@Z"(
+// CHECK-MSABI64-LABEL: define {{.*}} @"??_GH@@UEAAPEAXI@Z"(
+// CHECK-MSABI32-LABEL: define {{.*}} @"??_GH@@UAEPAXI@Z"(
 // CHECK-MSABI-NOT: call{{ }}
 // CHECK-MSABI: load i32
 // CHECK-MSABI: icmp eq i32 {{.*}}, 0
@@ -158,8 +193,8 @@ I::~I() { call_in_dtor(); }
 // CHECK-ITANIUM-NOT: call
 // CHECK-ITANIUM: }
 
-// CHECK-MSABI64: define {{.*}} @"??_GI@@UEAAPEAXI@Z"(
-// CHECK-MSABI32: define {{.*}} @"??_GI@@UAEPAXI@Z"(
+// CHECK-MSABI64-LABEL: define {{.*}} @"??_GI@@UEAAPEAXI@Z"(
+// CHECK-MSABI32-LABEL: define {{.*}} @"??_GI@@UAEPAXI@Z"(
 // CHECK-MSABI-NOT: call{{ }}
 // CHECK-MSABI: load i32
 // CHECK-MSABI: icmp eq i32 {{.*}}, 0

clang/test/SemaCXX/cxx2a-destroying-delete.cpp

@@ -58,8 +58,8 @@ namespace delete_selection {
   struct C {
     C();
     void *operator new(std::size_t);
-    void operator delete(void*) = delete;
-    void operator delete(C *, std::destroying_delete_t) = delete; // expected-note 0-1 {{deleted here}}
+    void operator delete(void*) = delete; // expected-note 0-1 {{deleted here}}
+    void operator delete(C *, std::destroying_delete_t) = delete;
   };
   // TODO: We only diagnose the use of a deleted operator delete when exceptions
   // are enabled. Otherwise we don't bother doing the lookup.
@@ -167,3 +167,23 @@ namespace dtor_access {
 
   void g() { delete (T *)new U; } // expected-error {{calling a protected destructor}}
 }
+
+namespace delete_from_new {
+  struct A {
+    A(); // might throw
+    void operator delete(A *, std::destroying_delete_t) = delete;
+  };
+  struct B {
+    B(); // might throw
+    void operator delete(void *) = delete; // #member-delete-from-new
+    void operator delete(B *, std::destroying_delete_t) = delete;
+  };
+  void f() {
+    new A; // calls ::operator delete
+    new B; // calls B::operator delete
+#ifdef __EXCEPTIONS
+  // expected-error@-2 {{attempt to use a deleted function}}
+  // expected-note@#member-delete-from-new {{deleted here}}
+#endif
+  }
+}