https://github.com/andykaylor updated https://github.com/llvm/llvm-project/pull/229606
>From 165dd1faa88b77055fe3d14d8c35bc887b44354a Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Tue, 6 Oct 2026 16:07:13 -0700 Subject: [PATCH 1/3] [CIR] Handle destroying operator delete in deleting destructors When a class's operator delete is a destroying operator delete, the deleting destructor must call it and must not call the complete destructor, because the operator delete takes over destruction of the object as well as deallocation of its storage. CIR previously reported this case as not yet implemented. Implementing this exposed a problem where we were guarding the complete destructor call with `haveInsertPoint()` but that didn't really do what it was meant to do because CIR doesn't use the absence of an insertion point to indicate that we've branched through the return block the way classic codegen does. Instead, I am sinking the code that emits the complete destructor into enterDtorCleanups() and letting that function decide when it should happen. The deleting destructor that takes an implicit parameter is still NYI. Assisted-by: Cursor / various models --- clang/lib/CIR/CodeGen/CIRGenClass.cpp | 30 ++++--- clang/lib/CIR/CodeGen/CIRGenFunction.cpp | 7 +- .../CIR/CodeGen/destroying-delete-dtor.cpp | 79 +++++++++++++++++++ 3 files changed, 100 insertions(+), 16 deletions(-) create mode 100644 clang/test/CIR/CodeGen/destroying-delete-dtor.cpp diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp index 28db1763fd3cb5..3890d62819fb09 100644 --- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp @@ -1035,28 +1035,38 @@ class DestroyField final : public EHScopeStack::Cleanup { /// destructors on members and base classes in reverse order of their /// construction. /// -/// For a deleting destructor, this also handles the case where a destroying -/// operator delete completely overrides the definition. +/// For a deleting destructor, this instead pushes the cleanup that calls +/// operator delete and delegates to the complete destructor. It also handles +/// the case where a destroying operator delete completely overrides the +/// definition. void CIRGenFunction::enterDtorCleanups(const CXXDestructorDecl *dd, CXXDtorType dtorType) { assert((!dd->isTrivial() || dd->hasAttr<DLLExportAttr>()) && "Should not emit dtor epilogue for non-exported trivial dtor!"); - // The deleting-destructor phase just needs to call the appropriate - // operator delete that Sema picked up. + // The deleting-destructor phase calls the appropriate operator delete + // that Sema picked up. if (dtorType == Dtor_Deleting) { assert(dd->getOperatorDelete() && "operator delete missing - EnterDtorCleanups"); if (cxxStructorImplicitParamValue) { cgm.errorNYI(dd->getSourceRange(), "deleting destructor with vtt"); + } else if (dd->getOperatorDelete()->isDestroyingOperatorDelete()) { + const CXXRecordDecl *classDecl = dd->getParent(); + emitDeleteCall(dd->getOperatorDelete(), loadThisForDtorDelete(*this, dd), + getContext().getCanonicalTagType(classDecl)); + // A destroying operator delete destroys the object itself, so skip + // the delegation to the complete destructor below. + return; } else { - if (dd->getOperatorDelete()->isDestroyingOperatorDelete()) { - cgm.errorNYI(dd->getSourceRange(), - "deleting destructor with destroying operator delete"); - } else { - ehStack.pushCleanup<CallDtorDelete>(NormalAndEHCleanup); - } + ehStack.pushCleanup<CallDtorDelete>(NormalAndEHCleanup); } + + // Delegate to the complete destructor. operator delete runs when + // the caller's cleanup scope exits. + QualType thisTy = dd->getFunctionObjectParameterType(); + emitCXXDestructorCall(dd, Dtor_Complete, /*forVirtualBase=*/false, + /*delegating=*/false, loadCXXThisAddress(), thisTy); return; } diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp index 9d0cdcf1fb76b7..04355f8b785e01 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp @@ -924,17 +924,12 @@ void CIRGenFunction::emitDestructorBody(FunctionArgList &args) { // The call to operator delete in a deleting destructor happens // outside of the function-try-block, which means it's always // possible to delegate the destructor body to the complete - // destructor. Do so. + // destructor. enterDtorCleanups does that if necessary. if (dtorType == Dtor_Deleting || dtorType == Dtor_VectorDeleting) { if (cxxStructorImplicitParamValue && dtorType == Dtor_VectorDeleting) cgm.errorNYI(dtor->getSourceRange(), "emitConditionalArrayDtorCall"); RunCleanupsScope dtorEpilogue(*this); enterDtorCleanups(dtor, Dtor_Deleting); - if (haveInsertPoint()) { - QualType thisTy = dtor->getFunctionObjectParameterType(); - emitCXXDestructorCall(dtor, Dtor_Complete, /*forVirtualBase=*/false, - /*delegating=*/false, loadCXXThisAddress(), thisTy); - } return; } diff --git a/clang/test/CIR/CodeGen/destroying-delete-dtor.cpp b/clang/test/CIR/CodeGen/destroying-delete-dtor.cpp new file mode 100644 index 00000000000000..5ca7fc520d1884 --- /dev/null +++ b/clang/test/CIR/CodeGen/destroying-delete-dtor.cpp @@ -0,0 +1,79 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -fclangir -mconstructor-aliases -emit-cir %s -o %t.cir +// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -fclangir -mconstructor-aliases -emit-llvm %s -o %t-cir.ll +// RUN: FileCheck --check-prefix=LLVM,LLVMCIR --input-file=%t-cir.ll %s +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -mconstructor-aliases -emit-llvm %s -o %t.ll +// RUN: FileCheck --check-prefix=LLVM,OGCG --input-file=%t.ll %s + +// Minimal in-source declarations of the standard-library bits needed for a +// destroying operator delete so this test does not depend on a system header. +namespace std { +struct destroying_delete_t { + explicit destroying_delete_t() = default; +}; +inline constexpr destroying_delete_t destroying_delete{}; +} // namespace std + +// The deleting destructor calls the destroying operator delete and does not +// call the complete destructor. The operator delete destroys the object. +struct A { + virtual ~A(); + void operator delete(A *, std::destroying_delete_t); +}; + +A::~A() {} + +void A::operator delete(A *, std::destroying_delete_t) {} + +// CIR-LABEL: cir.func {{.*}} @_ZN1AD0Ev +// CIR: %[[THIS_ADDR:.*]] = cir.alloca "this" +// CIR: %[[TAG:.*]] = cir.alloca "destroying.delete.tag" +// CIR: cir.store %[[ARG:.*]], %[[THIS_ADDR]] +// CIR: %[[THIS:.*]] = cir.load %[[THIS_ADDR]] +// CIR: cir.load{{.*}} %[[TAG]] +// CIR: cir.call @_ZN1AdlEPS_St19destroying_delete_t(%[[THIS]]) +// CIR-NOT: cir.call @_ZN1AD{{[12]}}Ev +// CIR: cir.return + +// LLVM-LABEL: define {{.*}} void @_ZN1AD0Ev( +// LLVM: %[[THIS_ADDR:.*]] = alloca ptr +// LLVMCIR: alloca %"struct.std::destroying_delete_t" +// OGCG-NOT: alloca %"struct.std::destroying_delete_t" +// LLVM: store ptr %[[ARG:.*]], ptr %[[THIS_ADDR]] +// LLVM: %[[THIS:.*]] = load ptr, ptr %[[THIS_ADDR]] +// LLVM: call void @_ZN1AdlEPS_St19destroying_delete_t(ptr noundef %[[THIS]]) +// LLVM-NOT: call {{.*}}@_ZN1AD{{[12]}}Ev +// LLVM: ret void + +// B inherits A's destroying operator delete. The deleting destructor adjusts +// 'this' to the A subobject and calls that operator delete, and does not call +// B's complete destructor. +struct Padding { + virtual void f(); +}; + +struct B : Padding, A { + ~B() override; +}; + +B::~B() {} + +// CIR-LABEL: cir.func {{.*}} @_ZN1BD0Ev +// CIR: %[[THIS_ADDR:.*]] = cir.alloca "this" +// CIR: cir.store %[[ARG:.*]], %[[THIS_ADDR]] +// CIR: %[[THIS:.*]] = cir.load %[[THIS_ADDR]] +// CIR: %[[A:.*]] = cir.base_class_addr {{.*}} %[[THIS]] [8] +// CIR: cir.call @_ZN1AdlEPS_St19destroying_delete_t(%[[A]]) +// CIR-NOT: cir.call @_ZN1BD{{[12]}}Ev +// CIR-NOT: cir.call @_ZN1AD{{[12]}}Ev +// CIR: cir.return + +// LLVM-LABEL: define {{.*}} void @_ZN1BD0Ev( +// LLVM: %[[THIS_ADDR:.*]] = alloca ptr +// LLVM: store ptr %[[ARG:.*]], ptr %[[THIS_ADDR]] +// LLVM: %[[THIS:.*]] = load ptr, ptr %[[THIS_ADDR]] +// LLVM: %[[A:.*]] = getelementptr {{.*}}i8, ptr %[[THIS]], {{i32|i64}} 8 +// LLVM: call void @_ZN1AdlEPS_St19destroying_delete_t(ptr noundef %[[A]]) +// LLVM-NOT: call {{.*}}@_ZN1BD{{[12]}}Ev +// LLVM-NOT: call {{.*}}@_ZN1AD{{[12]}}Ev +// LLVM: ret void >From daa3cc7dc74e5f1fcc02f36f71c7cb86a6a8778c Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Tue, 6 Oct 2026 17:17:26 -0700 Subject: [PATCH 2/3] Cleanup test checks --- clang/test/CIR/CodeGen/destroying-delete-dtor.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/clang/test/CIR/CodeGen/destroying-delete-dtor.cpp b/clang/test/CIR/CodeGen/destroying-delete-dtor.cpp index 5ca7fc520d1884..10868371c2034b 100644 --- a/clang/test/CIR/CodeGen/destroying-delete-dtor.cpp +++ b/clang/test/CIR/CodeGen/destroying-delete-dtor.cpp @@ -1,9 +1,9 @@ // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -fclangir -mconstructor-aliases -emit-cir %s -o %t.cir // RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -fclangir -mconstructor-aliases -emit-llvm %s -o %t-cir.ll -// RUN: FileCheck --check-prefix=LLVM,LLVMCIR --input-file=%t-cir.ll %s +// RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -mconstructor-aliases -emit-llvm %s -o %t.ll -// RUN: FileCheck --check-prefix=LLVM,OGCG --input-file=%t.ll %s +// RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s // Minimal in-source declarations of the standard-library bits needed for a // destroying operator delete so this test does not depend on a system header. @@ -27,18 +27,14 @@ void A::operator delete(A *, std::destroying_delete_t) {} // CIR-LABEL: cir.func {{.*}} @_ZN1AD0Ev // CIR: %[[THIS_ADDR:.*]] = cir.alloca "this" -// CIR: %[[TAG:.*]] = cir.alloca "destroying.delete.tag" // CIR: cir.store %[[ARG:.*]], %[[THIS_ADDR]] // CIR: %[[THIS:.*]] = cir.load %[[THIS_ADDR]] -// CIR: cir.load{{.*}} %[[TAG]] // CIR: cir.call @_ZN1AdlEPS_St19destroying_delete_t(%[[THIS]]) // CIR-NOT: cir.call @_ZN1AD{{[12]}}Ev // CIR: cir.return // LLVM-LABEL: define {{.*}} void @_ZN1AD0Ev( // LLVM: %[[THIS_ADDR:.*]] = alloca ptr -// LLVMCIR: alloca %"struct.std::destroying_delete_t" -// OGCG-NOT: alloca %"struct.std::destroying_delete_t" // LLVM: store ptr %[[ARG:.*]], ptr %[[THIS_ADDR]] // LLVM: %[[THIS:.*]] = load ptr, ptr %[[THIS_ADDR]] // LLVM: call void @_ZN1AdlEPS_St19destroying_delete_t(ptr noundef %[[THIS]]) >From 641df558b0675dd8eb71f264931e8734e908fa45 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Wed, 7 Oct 2026 13:44:51 -0700 Subject: [PATCH 3/3] Address review feedback --- clang/lib/CIR/CodeGen/CIRGenClass.cpp | 44 ++++++++++-------------- clang/lib/CIR/CodeGen/CIRGenFunction.cpp | 19 +++++++++- clang/lib/CIR/CodeGen/CIRGenFunction.h | 4 +++ 3 files changed, 40 insertions(+), 27 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp index 3890d62819fb09..1a275663b4ee83 100644 --- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp @@ -987,14 +987,13 @@ void CIRGenFunction::destroyCXXObject(CIRGenFunction &cgf, Address addr, /*delegating=*/false, addr, type); } -namespace { -mlir::Value loadThisForDtorDelete(CIRGenFunction &cgf, - const CXXDestructorDecl *dd) { +mlir::Value CIRGenFunction::loadThisForDtorDelete(const CXXDestructorDecl *dd) { if (Expr *thisArg = dd->getOperatorDeleteThisArg()) - return cgf.emitScalarExpr(thisArg); - return cgf.loadCXXThis(); + return emitScalarExpr(thisArg); + return loadCXXThis(); } +namespace { /// Call the operator delete associated with the current destructor. struct CallDtorDelete final : EHScopeStack::Cleanup { CallDtorDelete() {} @@ -1003,7 +1002,7 @@ struct CallDtorDelete final : EHScopeStack::Cleanup { const CXXDestructorDecl *dtor = cast<CXXDestructorDecl>(cgf.curFuncDecl); const CXXRecordDecl *classDecl = dtor->getParent(); cgf.emitDeleteCall(dtor->getOperatorDelete(), - loadThisForDtorDelete(cgf, dtor), + cgf.loadThisForDtorDelete(dtor), cgf.getContext().getCanonicalTagType(classDecl)); } }; @@ -1035,38 +1034,31 @@ class DestroyField final : public EHScopeStack::Cleanup { /// destructors on members and base classes in reverse order of their /// construction. /// -/// For a deleting destructor, this instead pushes the cleanup that calls -/// operator delete and delegates to the complete destructor. It also handles -/// the case where a destroying operator delete completely overrides the -/// definition. +/// For a deleting destructor, this pushes the cleanup that calls operator +/// delete. The caller handles a destroying operator delete. void CIRGenFunction::enterDtorCleanups(const CXXDestructorDecl *dd, CXXDtorType dtorType) { assert((!dd->isTrivial() || dd->hasAttr<DLLExportAttr>()) && "Should not emit dtor epilogue for non-exported trivial dtor!"); - // The deleting-destructor phase calls the appropriate operator delete - // that Sema picked up. + // The deleting-destructor phase just needs to call the appropriate + // operator delete that Sema picked up. if (dtorType == Dtor_Deleting) { assert(dd->getOperatorDelete() && "operator delete missing - EnterDtorCleanups"); if (cxxStructorImplicitParamValue) { - cgm.errorNYI(dd->getSourceRange(), "deleting destructor with vtt"); - } else if (dd->getOperatorDelete()->isDestroyingOperatorDelete()) { - const CXXRecordDecl *classDecl = dd->getParent(); - emitDeleteCall(dd->getOperatorDelete(), loadThisForDtorDelete(*this, dd), - getContext().getCanonicalTagType(classDecl)); - // A destroying operator delete destroys the object itself, so skip - // the delegation to the complete destructor below. - return; + // The implicit parameter of a deleting destructor is the Microsoft ABI + // flag word that selects whether and which operator delete is called. + assert(getTarget().getCXXABI().isMicrosoft() && + "only the Microsoft ABI passes an implicit deleting dtor param"); + cgm.errorNYI(dd->getSourceRange(), + "deleting destructor with conditional delete: MSVC ABI"); + } else { + assert(!dd->getOperatorDelete()->isDestroyingOperatorDelete() && + "destroying operator delete is handled by emitDestructorBody"); ehStack.pushCleanup<CallDtorDelete>(NormalAndEHCleanup); } - - // Delegate to the complete destructor. operator delete runs when - // the caller's cleanup scope exits. - QualType thisTy = dd->getFunctionObjectParameterType(); - emitCXXDestructorCall(dd, Dtor_Complete, /*forVirtualBase=*/false, - /*delegating=*/false, loadCXXThisAddress(), thisTy); return; } diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp index 04355f8b785e01..ff6a577e39c33d 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp @@ -924,12 +924,29 @@ void CIRGenFunction::emitDestructorBody(FunctionArgList &args) { // The call to operator delete in a deleting destructor happens // outside of the function-try-block, which means it's always // possible to delegate the destructor body to the complete - // destructor. enterDtorCleanups does that if necessary. + // destructor. Do so. if (dtorType == Dtor_Deleting || dtorType == Dtor_VectorDeleting) { if (cxxStructorImplicitParamValue && dtorType == Dtor_VectorDeleting) cgm.errorNYI(dtor->getSourceRange(), "emitConditionalArrayDtorCall"); + + // A destroying operator delete destroys the object and deallocates its + // storage, so the deleting destructor only calls the operator delete. + const FunctionDecl *operatorDelete = dtor->getOperatorDelete(); + if (operatorDelete->isDestroyingOperatorDelete()) { + if (cxxStructorImplicitParamValue) { + // The implicit parameter of a deleting destructor is the Microsoft ABI. + cgm.errorNYI(dtor->getSourceRange(), "emitConditionalArrayDtorCall"); + } + emitDeleteCall(operatorDelete, loadThisForDtorDelete(dtor), + getContext().getCanonicalTagType(dtor->getParent())); + return; + } + RunCleanupsScope dtorEpilogue(*this); enterDtorCleanups(dtor, Dtor_Deleting); + QualType thisTy = dtor->getFunctionObjectParameterType(); + emitCXXDestructorCall(dtor, Dtor_Complete, /*forVirtualBase=*/false, + /*delegating=*/false, loadCXXThisAddress(), thisTy); return; } diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index c6427d60a37926..86385ceb265510 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -881,6 +881,10 @@ class CIRGenFunction : public CIRGenTypeCache { /// base classes in reverse order of their construction. void enterDtorCleanups(const CXXDestructorDecl *dtor, CXXDtorType type); + /// Return the pointer to pass to the operator delete of the given + /// destructor, converted to the type of its first parameter if needed. + mlir::Value loadThisForDtorDelete(const CXXDestructorDecl *dd); + /// Determines whether an EH cleanup is required to destroy a type /// with the given destruction kind. /// TODO(cir): could be shared with Clang LLVM codegen _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
