[clang] Implement P3144R2 "Deleting a Pointer to an Incomplete Type..." #97733

Endilll · 2024-07-04T14:19:10Z

This patch implements (not yet published) P3144R2 "Deleting a Pointer to an Incomplete Type Should be Ill-formed". Wording changes (not yet merged into the working draft) read:

7.6.2.9 [expr.delete] Delete
If the object being deleted has incomplete class type at the point of deletion and the complete class has a
non-trivial destructor or a deallocation function, the behavior is undefined, the program is ill-formed.

We preserve status quo of emitting a warning when deleting a pointer to incomplete type up to, and including, C++23, but make it ill-formed since C++26. Same goes for deleting pointers to void, which has been allowed as an extension.

…hould be Ill-formed"

llvmbot · 2024-07-04T14:19:42Z

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

This patch implements (not yet published) P3144R2 "Deleting a Pointer to an Incomplete Type Should be Ill-formed". Wording changes (not yet merged into the working draft) read:
> 7.6.2.9 [expr.delete] Delete
> If the object being deleted has incomplete class type at the point of deletion <del>and the complete class has a
non-trivial destructor or a deallocation function, the behavior is undefined</del>, <ins>the program is ill-formed</ins>.

We preserve status quo of emitting a warning when deleting a pointer to incomplete type up to, and including, C++23, but make it ill-formed since C++26. Same goes for deleting pointers to void, which has been allowed as an extension.

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

4 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
(modified) clang/lib/Sema/SemaExprCXX.cpp (+9-3)
(modified) clang/test/CXX/drs/cwg5xx.cpp (+13-9)
(modified) clang/test/SemaCXX/new-delete.cpp (+17-9)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3df64b2ecef1b..5c3e311ac6382 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7991,6 +7991,8 @@ def err_ambiguous_delete_operand : Error<
 def warn_delete_incomplete : Warning<
   "deleting pointer to incomplete type %0 may cause undefined behavior">,
   InGroup<DeleteIncomplete>;
+def err_delete_incomplete : Error<
+  "cannot delete pointer to incomplete type %0">;
 def err_delete_incomplete_class_type : Error<
   "deleting incomplete class type %0; no conversions to pointer type">;
 def err_delete_explicit_conversion : Error<
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 69074f92a0286..fcf2189a308a8 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -3719,8 +3719,11 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
       // The C++ standard bans deleting a pointer to a non-object type, which
       // effectively bans deletion of "void*". However, most compilers support
       // this, so we treat it as a warning unless we're in a SFINAE context.
-      Diag(StartLoc, diag::ext_delete_void_ptr_operand)
-        << Type << Ex.get()->getSourceRange();
+      // But we still prohibit this since C++26.
+      Diag(StartLoc, LangOpts.CPlusPlus26 ? diag::err_delete_incomplete
+                                          : diag::ext_delete_void_ptr_operand)
+          << (LangOpts.CPlusPlus26 ? Pointee : Type)
+          << Ex.get()->getSourceRange();
     } else if (Pointee->isFunctionType() || Pointee->isVoidType() ||
                Pointee->isSizelessType()) {
       return ExprError(Diag(StartLoc, diag::err_delete_operand)
@@ -3729,7 +3732,10 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
       // FIXME: This can result in errors if the definition was imported from a
       // module but is hidden.
       if (!RequireCompleteType(StartLoc, Pointee,
-                               diag::warn_delete_incomplete, Ex.get())) {
+                               LangOpts.CPlusPlus26
+                                   ? diag::err_delete_incomplete
+                                   : diag::warn_delete_incomplete,
+                               Ex.get())) {
         if (const RecordType *RT = PointeeElem->getAs<RecordType>())
           PointeeRD = cast<CXXRecordDecl>(RT->getDecl());
       }
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index 9d890f981348a..6a0bb7a196669 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1,9 +1,10 @@
-// RUN: %clang_cc1 -std=c++98 %s -verify=expected,cxx98-11,cxx98-14,cxx98-17,cxx98 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++11 %s -verify=expected,cxx98-11,cxx98-14,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++14 %s -verify=expected,cxx98-14,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++17 %s -verify=expected,since-cxx17,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++20 %s -verify=expected,since-cxx20,since-cxx17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
-// RUN: %clang_cc1 -std=c++23 %s -verify=expected,since-cxx23,since-cxx20,since-cxx17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++98 %s -verify=expected,cxx98-23,cxx98-11,cxx98-14,cxx98-17,cxx98 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++11 %s -verify=expected,cxx98-23,cxx98-11,cxx98-14,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++14 %s -verify=expected,cxx98-23,cxx98-14,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++17 %s -verify=expected,cxx98-23,since-cxx17,cxx98-17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++20 %s -verify=expected,cxx98-23,since-cxx20,since-cxx17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++23 %s -verify=expected,cxx98-23,since-cxx23,since-cxx20,since-cxx17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++2c %s -verify=expected,since-cxx26,since-cxx23,since-cxx20,since-cxx17,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
 
 #if __cplusplus == 199711L
 #define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
@@ -901,7 +902,8 @@ namespace cwg573 { // cwg573: no
   void *d = reinterpret_cast<void*>(c);
   // cxx98-error@-1 {{cast between pointer-to-function and pointer-to-object is an extension}}
   void f() { delete a; }
-  // expected-error@-1 {{cannot delete expression with pointer-to-'void' type 'void *'}}
+  // cxx98-23-error@-1 {{cannot delete expression with pointer-to-'void' type 'void *'}}
+  // since-cxx26-error@-2 {{cannot delete pointer to incomplete type 'void'}}
   int n = d - a;
   // expected-error@-1 {{arithmetic on pointers to void}}
   // FIXME: This is ill-formed.
@@ -1238,11 +1240,13 @@ namespace cwg599 { // cwg599: partial
   struct V { operator int*(); operator Fn*(); };
   void f(void *p, void (*q)(), S s, T t, U u, V v) {
     delete p;
-    // expected-error@-1 {{cannot delete expression with pointer-to-'void' type 'void *'}}
+    // cxx98-23-error@-1 {{cannot delete expression with pointer-to-'void' type 'void *'}}
+    // since-cxx26-error@-2 {{cannot delete pointer to incomplete type 'void'}}
     delete q;
     // expected-error@-1 {{cannot delete expression of type 'void (*)()'}}
     delete s;
-    // expected-error@-1 {{cannot delete expression with pointer-to-'void' type 'void *'}}
+    // cxx98-23-error@-1 {{cannot delete expression with pointer-to-'void' type 'void *'}}
+    // since-cxx26-error@-2 {{cannot delete pointer to incomplete type 'void'}}
     delete t;
     // expected-error@-1 {{cannot delete expression of type 'T'}}
     // FIXME: This is valid, but is rejected due to a non-conforming GNU
diff --git a/clang/test/SemaCXX/new-delete.cpp b/clang/test/SemaCXX/new-delete.cpp
index 1a99c6aac604f..ec6ad43476f94 100644
--- a/clang/test/SemaCXX/new-delete.cpp
+++ b/clang/test/SemaCXX/new-delete.cpp
@@ -1,8 +1,10 @@
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++98
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++11
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++14
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx17,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++17
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx17,cxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++20
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx98-23,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++98
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx98-23,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++11
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx98-23,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++14
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx98-23,cxx17,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++17
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx98-23,cxx17,cxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++20
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx98-23,cxx17,cxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++23
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,since-cxx26,cxx17,cxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++2c
 
 // FIXME Location is (frontend)
 // cxx17-note@*:* {{candidate function not viable: requires 2 arguments, but 3 were provided}}
@@ -172,8 +174,12 @@ void bad_deletes()
 {
   delete 0; // expected-error {{cannot delete expression of type 'int'}}
   delete [0] (int*)0; // expected-error {{expected variable name or 'this' in lambda capture list}}
-  delete (void*)0; // expected-warning {{cannot delete expression with pointer-to-'void' type 'void *'}}
-  delete (T*)0; // expected-warning {{deleting pointer to incomplete type}}
+  delete (void*)0;
+  // cxx98-23-warning@-1 {{cannot delete expression with pointer-to-'void' type 'void *'}}
+  // since-cxx26-error@-2 {{cannot delete pointer to incomplete type 'void'}}
+  delete (T*)0;
+  // cxx98-23-warning@-1 {{deleting pointer to incomplete type}}
+  // since-cxx26-error@-2 {{cannot delete pointer to incomplete type 'T'}}
   ::S::delete (int*)0; // expected-error {{expected unqualified-id}}
 }
 
@@ -513,8 +519,10 @@ namespace DeleteIncompleteClass {
 
 namespace DeleteIncompleteClassPointerError {
   struct A; // expected-note {{forward declaration}}
-  void f(A *x) { 1+delete x; } // expected-warning {{deleting pointer to incomplete type}} \
-                               // expected-error {{invalid operands to binary expression}}
+  void f(A *x) { 1+delete x; }
+  // expected-error@-1 {{invalid operands to binary expression}}
+  // cxx98-23-warning@-2 {{deleting pointer to incomplete type}}
+  // since-cxx26-error@-3 {{cannot delete pointer to incomplete type 'A'}}
 }
 
 namespace PR10504 {

Sirraide

LGTM, generally speaking, but do we maybe want change the old diagnostic to mention that it’s ‘incompatible w/ C++26’ or however we usually phrase that?

Sirraide · 2024-07-04T14:26:06Z

Oh also, this still needs a release note and an entry in the C++ support docs, doesn’t it?

clang/include/clang/Basic/DiagnosticSemaKinds.td

Endilll · 2024-07-05T09:08:56Z

LGTM, generally speaking, but do we maybe want change the old diagnostic to mention that it’s ‘incompatible w/ C++26’ or however we usually phrase that?

Done.

Oh also, this still needs a release note and an entry in the C++ support docs, doesn’t it?

Done.

I also added -Wc++2c-compat flag and a corresponding diagnostic group.

cor3ntin · 2024-07-05T09:54:46Z

Still missing a change to cxx_status.html

cor3ntin

LGTM, thanks

…." (llvm#97733) This patch implements (not yet published) [P3144R2](https://wiki.edg.com/pub/Wg21stlouis2024/StrawPolls/p3144r2.pdf) "Deleting a Pointer to an Incomplete Type Should be Ill-formed". Wording changes (not yet merged into the working draft) read: > 7.6.2.9 [expr.delete] Delete > If the object being deleted has incomplete class type at the point of deletion <del>and the complete class has a non-trivial destructor or a deallocation function, the behavior is undefined</del>, <ins>the program is ill-formed</ins>. We preserve status quo of emitting a warning when deleting a pointer to incomplete type up to, and including, C++23, but make it ill-formed since C++26. Same goes for deleting pointers to `void`, which has been allowed as an extension.

The changes introduced in llvm#97733 accidentally prevented to delete an incomplete enum (the validity of which has been confirmed by CWG2925 Fixes llvm#99278

The changes introduced in #97733 accidentally prevented to delete an incomplete enum (the validity of which has been confirmed by CWG2925 Fixes #99278

[clang] Implement P3144R2 "Deleting a Pointer to an Incomplete Type S…

f009148

…hould be Ill-formed"

Endilll added clang:frontend Language frontend issues, e.g. anything involving "Sema" c++26 labels Jul 4, 2024

Endilll requested review from cor3ntin, AaronBallman and Sirraide July 4, 2024 14:19

llvmbot added the clang Clang issues not falling into any other category label Jul 4, 2024

Sirraide approved these changes Jul 4, 2024

View reviewed changes

cor3ntin reviewed Jul 4, 2024

View reviewed changes

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved

Address feedback

bdcc4ab

Change C++ Status page accordingly

3a28e52

cor3ntin approved these changes Jul 5, 2024

View reviewed changes

Endilll added 2 commits July 5, 2024 13:54

Add missing whitespace to docs that was breaking the build

60f00a9

Address CI failures

fd2d27f

llvmbot added the clang:openmp OpenMP related changes to Clang label Jul 5, 2024

Endilll merged commit 788731c into llvm:main Jul 5, 2024
9 checks passed

Endilll deleted the p3144-delete-incomplete branch July 5, 2024 18:27

cor3ntin mentioned this pull request Dec 3, 2024

[Clang] Deleting an incomplete enum type is not an error #118455

Merged

cor3ntin mentioned this pull request Dec 7, 2024

[Clang] Deleting an incomplete enum type is not an error #119077

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] Implement P3144R2 "Deleting a Pointer to an Incomplete Type..." #97733

[clang] Implement P3144R2 "Deleting a Pointer to an Incomplete Type..." #97733

Uh oh!

Endilll commented Jul 4, 2024

Uh oh!

llvmbot commented Jul 4, 2024

Uh oh!

Sirraide left a comment

Uh oh!

Sirraide commented Jul 4, 2024

Uh oh!

Uh oh!

Endilll commented Jul 5, 2024 •

edited

Loading

Uh oh!

cor3ntin commented Jul 5, 2024

Uh oh!

cor3ntin left a comment

Uh oh!

Uh oh!

Uh oh!

[clang] Implement P3144R2 "Deleting a Pointer to an Incomplete Type..." #97733

[clang] Implement P3144R2 "Deleting a Pointer to an Incomplete Type..." #97733

Uh oh!

Conversation

Endilll commented Jul 4, 2024

Uh oh!

llvmbot commented Jul 4, 2024

Uh oh!

Sirraide left a comment

Choose a reason for hiding this comment

Uh oh!

Sirraide commented Jul 4, 2024

Uh oh!

Uh oh!

Endilll commented Jul 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cor3ntin commented Jul 5, 2024

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Endilll commented Jul 5, 2024 •

edited

Loading