-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[C++20] Destroying delete and deleted destructors #118800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[C++20] Destroying delete and deleted destructors #118800
Conversation
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.
@llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) ChangesWhen 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. Full diff: https://github.com/llvm/llvm-project/pull/118800.diff 3 Files Affected:
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.
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 nit, else LGTM.
* Added a standards reference * Added a test case * Fixed an issue the new test case identified
It turns out someone filed an issue for this.
A virtual destructor still needs to be accessible to a destroying delete because the actual delete operator called depends on the dynamic type of the pointer, not the static type.
As I just noted in #118687, I think we also need similar treatment for |
FYI, I'm going to be out on vacation after today, so if this is ready to land, someone else can feel free to press the button. Otherwise, I can pick this up again when I'm back. |
Ping @zygoloid |
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG other than the function name.
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.
Fixes #46818