https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/118800
>From 34d3d3000bc6096bbc9eb35ce85b6ceca50b91ca Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Thu, 5 Dec 2024 08:31:24 -0500 Subject: [PATCH 1/2] [C++20] Destroying delete and deleted destructors When a destroying delete overload is selected, the destructor is not automatically called. Therefore, the destructor can be deleted without causing the program to be ill-formed. --- clang/docs/ReleaseNotes.rst | 2 ++ clang/lib/Sema/SemaExprCXX.cpp | 6 +++-- .../CXX/expr/expr.unary/expr.delete/p10.cpp | 22 +++++++++++++++++-- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e484eb7a76e63a..4a72e4046e2d03 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -663,6 +663,8 @@ 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 incorrectly diagnosing use of a deleted destructor when the + selected overload of ``operator delete`` for that type is a destroying delete. Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index db9ea7fb66e05a..45840dfa31ac92 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -3792,13 +3792,15 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal, .HasSizeT; } - if (!PointeeRD->hasIrrelevantDestructor()) + if (!PointeeRD->hasIrrelevantDestructor() && + (!OperatorDelete || !OperatorDelete->isDestroyingOperatorDelete())) { if (CXXDestructorDecl *Dtor = LookupDestructor(PointeeRD)) { MarkFunctionReferenced(StartLoc, - const_cast<CXXDestructorDecl*>(Dtor)); + const_cast<CXXDestructorDecl *>(Dtor)); if (DiagnoseUseOfDecl(Dtor, StartLoc)) return ExprError(); } + } CheckVirtualDtorCall(PointeeRD->getDestructor(), StartLoc, /*IsDelete=*/true, /*CallCanBeVirtual=*/true, diff --git a/clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp b/clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp index aad2747dd32f24..b2c0a2c79695fc 100644 --- a/clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp +++ b/clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp @@ -1,7 +1,14 @@ -// RUN: %clang_cc1 -std=c++1z -verify %s +// RUN: %clang_cc1 -std=c++20 -verify %s using size_t = decltype(sizeof(0)); -namespace std { enum class align_val_t : size_t {}; } +namespace std { + enum class align_val_t : size_t {}; + struct destroying_delete_t { + explicit destroying_delete_t() = default; + }; + + inline constexpr destroying_delete_t destroying_delete{}; +} // Aligned version is preferred over unaligned version, // unsized version is preferred over sized version. @@ -23,3 +30,14 @@ struct alignas(Align) B { }; void f(B<__STDCPP_DEFAULT_NEW_ALIGNMENT__> *p) { delete p; } void f(B<__STDCPP_DEFAULT_NEW_ALIGNMENT__ * 2> *p) { delete p; } // expected-error {{deleted}} + +// Ensure that a deleted destructor is acceptable when the selected overload +// for operator delete is a destroying delete. See the comments in GH118660. +struct S { + ~S() = delete; + void operator delete(S *, std::destroying_delete_t) noexcept {} +}; + +void foo(S *s) { + delete s; // Was rejected, is intended to be accepted. +} >From 133a8aa2934f9d6ed733e74af65180131d59cc91 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Thu, 5 Dec 2024 10:14:46 -0500 Subject: [PATCH 2/2] Update based on review feedback * Added a standards reference * Added a test case * Fixed an issue the new test case identified --- clang/lib/Sema/SemaExprCXX.cpp | 13 +++++++++++-- clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp | 10 ++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 45840dfa31ac92..6ac44ae7af28c3 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -3792,6 +3792,14 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal, .HasSizeT; } + // C++20 [expr.delete]p6: If the value of the operand of the delete- + // expression is not a null pointer value and the selected deallocation + // function (see below) is not a destroying operator delete, the delete- + // expression will invoke the destructor (if any) for the object or the + // elements of the array being deleted. + // + // This means we should not look at the destructor for a destroying + // delete operator, as that destructor is never called. if (!PointeeRD->hasIrrelevantDestructor() && (!OperatorDelete || !OperatorDelete->isDestroyingOperatorDelete())) { if (CXXDestructorDecl *Dtor = LookupDestructor(PointeeRD)) { @@ -3831,9 +3839,10 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal, MarkFunctionReferenced(StartLoc, OperatorDelete); // Check access and ambiguity of destructor if we're going to call it. - // Note that this is required even for a virtual delete. + // Note that this is required even for a virtual delete, but not for a + // destroying delete operator. bool IsVirtualDelete = false; - if (PointeeRD) { + if (PointeeRD && !OperatorDelete->isDestroyingOperatorDelete()) { if (CXXDestructorDecl *Dtor = LookupDestructor(PointeeRD)) { CheckDestructorAccess(Ex.get()->getExprLoc(), Dtor, PDiag(diag::err_access_dtor) << PointeeElem); diff --git a/clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp b/clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp index b2c0a2c79695fc..aff3e2b449da37 100644 --- a/clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp +++ b/clang/test/CXX/expr/expr.unary/expr.delete/p10.cpp @@ -38,6 +38,16 @@ struct S { void operator delete(S *, std::destroying_delete_t) noexcept {} }; +struct T { + void operator delete(T *, std::destroying_delete_t) noexcept {} +private: + ~T(); +}; + void foo(S *s) { delete s; // Was rejected, is intended to be accepted. } + +void bar(T *t) { + delete t; // Was rejected, is intended to be accepted. +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits