https://github.com/MitalAshok created
https://github.com/llvm/llvm-project/pull/92814
A new warning -Wdelete-array is issued for the now-accepted delete of an array,
which becomes a pointer to the first element after an array-to-pointer
conversion.
The GNU extension of deleting a void pointer is still accepted, but if that
void pointer comes from a conversion operator, a conversion to a pointer to an
object type takes priority before conversions are rechecked to allow conversion
to a void pointer. This means that previously ambiguous contextual conversions
where there was a conversion to a void pointer and an object pointer now
unambiguously pick the conversion to an object pointer.
>From 43e9f8fe5cdb19c0f57a00b352592e56e470ffe7 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mi...@mitalashok.co.uk>
Date: Mon, 20 May 2024 20:18:48 +0100
Subject: [PATCH] [Clang] Change how the argument of a delete expression is
converted
A new warning -Wdelete-array is issued for the now-accepted delete of an array,
which becomes a pointer to the first element after an array-to-pointer
conversion.
The GNU extension of deleting a void pointer is still accepted, but if that
void pointer comes from a conversion operator, a conversion to a pointer to an
object type takes priority before conversions are rechecked to allow conversion
to a void pointer.
This means that previously ambiguous contextual conversions where there was a
conversion to a void pointer and an object pointer now unambiguously pick the
conversion to an object pointer.
---
clang/docs/ReleaseNotes.rst | 4 +
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td | 1 +
clang/lib/Sema/SemaExprCXX.cpp | 187 ++++++++++--------
clang/test/CXX/drs/cwg5xx.cpp | 9 +-
.../test/Parser/cxx2c-delete-with-message.cpp | 4 +-
clang/www/cxx_dr_status.html | 2 +-
7 files changed, 117 insertions(+), 91 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 81e9d0423f96a..6e769f1f99ceb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -749,6 +749,10 @@ Bug Fixes to C++ Support
- Clang now correctly diagnoses when the current instantiation is used as an
incomplete base class.
- Clang no longer treats ``constexpr`` class scope function template
specializations of non-static members
as implicitly ``const`` in language modes after C++11.
+- Fix delete-expression operand not undergoing array-to-pointer conversion.
Now warn ``-Wdelete-array`` when
+ trying to delete an array object.
+- Fix GNU extension that allows deleting ``void *`` making some deletes of
class type ambiguous when there
+ is an object pointer and void pointer conversion operator. Now chooses the
object pointer conversion.
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index 4cb4f3d999f7a..410c31a25db03 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -168,6 +168,7 @@ def UndefinedFuncTemplate :
DiagGroup<"undefined-func-template">;
def MissingNoEscape : DiagGroup<"missing-noescape">;
def DefaultedFunctionDeleted : DiagGroup<"defaulted-function-deleted">;
+def DeleteArray : DiagGroup<"delete-array">;
def DeleteIncomplete : DiagGroup<"delete-incomplete">;
def DeleteNonAbstractNonVirtualDtor :
DiagGroup<"delete-non-abstract-non-virtual-dtor">;
def DeleteAbstractNonVirtualDtor :
DiagGroup<"delete-abstract-non-virtual-dtor">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e3c65cba4886a..580f8e57804fa 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7930,6 +7930,7 @@ def ext_default_init_const : ExtWarn<
"is a Microsoft extension">,
InGroup<MicrosoftConstInit>;
def err_delete_operand : Error<"cannot delete expression of type %0">;
+def warn_delete_array : Warning<"deleting array of type %0">,
InGroup<DeleteArray>;
def ext_delete_void_ptr_operand : ExtWarn<
"cannot delete expression with pointer-to-'void' type %0">,
InGroup<DeleteIncomplete>;
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index f543e006060d6..9a1e149a4af8e 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -3675,13 +3675,60 @@ void Sema::AnalyzeDeleteExprMismatch(FieldDecl *Field,
SourceLocation DeleteLoc,
}
}
+namespace {
+class DeleteConverter : public Sema::ContextualImplicitConverter {
+public:
+ bool AllowVoidPointer = false;
+
+ DeleteConverter() : ContextualImplicitConverter(false, true) {}
+
+ bool match(QualType ConvType) override {
+ return ConvType->isObjectPointerType() &&
+ (AllowVoidPointer || !ConvType->isVoidPointerType());
+ }
+
+ using SDB = Sema::SemaDiagnosticBuilder;
+
+ SDB diagnoseNoMatch(Sema &S, SourceLocation Loc, QualType T) override {
+ return S.Diag(Loc, diag::err_delete_operand) << T;
+ }
+
+ SDB diagnoseIncomplete(Sema &S, SourceLocation Loc, QualType T) override {
+ return S.Diag(Loc, diag::err_delete_incomplete_class_type) << T;
+ }
+
+ SDB diagnoseExplicitConv(Sema &S, SourceLocation Loc, QualType T,
+ QualType ConvTy) override {
+ return S.Diag(Loc, diag::err_delete_explicit_conversion) << T << ConvTy;
+ }
+
+ SDB noteExplicitConv(Sema &S, CXXConversionDecl *Conv,
+ QualType ConvTy) override {
+ return S.Diag(Conv->getLocation(), diag::note_delete_conversion) << ConvTy;
+ }
+
+ SDB diagnoseAmbiguous(Sema &S, SourceLocation Loc, QualType T) override {
+ return S.Diag(Loc, diag::err_ambiguous_delete_operand) << T;
+ }
+
+ SDB noteAmbiguous(Sema &S, CXXConversionDecl *Conv,
+ QualType ConvTy) override {
+ return S.Diag(Conv->getLocation(), diag::note_delete_conversion) << ConvTy;
+ }
+
+ SDB diagnoseConversion(Sema &S, SourceLocation Loc, QualType T,
+ QualType ConvTy) override {
+ llvm_unreachable("conversion functions are permitted");
+ }
+};
+} // namespace
+
/// ActOnCXXDelete - Parsed a C++ 'delete' expression (C++ 5.3.5), as in:
/// @code ::delete ptr; @endcode
/// or
/// @code delete [] ptr; @endcode
-ExprResult
-Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
- bool ArrayForm, Expr *ExE) {
+ExprResult Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
+ bool ArrayForm, Expr *Ex) {
// C++ [expr.delete]p1:
// The operand shall have a pointer type, or a class type having a single
// non-explicit conversion function to a pointer type. The result has type
@@ -3689,88 +3736,61 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool
UseGlobal,
//
// DR599 amends "pointer type" to "pointer to object type" in both cases.
- ExprResult Ex = ExE;
FunctionDecl *OperatorDelete = nullptr;
bool ArrayFormAsWritten = ArrayForm;
bool UsualArrayDeleteWantsSize = false;
- if (!Ex.get()->isTypeDependent()) {
- // Perform lvalue-to-rvalue cast, if needed.
- Ex = DefaultLvalueConversion(Ex.get());
- if (Ex.isInvalid())
+ if (!Ex->isTypeDependent()) {
+ if (ExprResult Conv = DefaultLvalueConversion(Ex); Conv.isInvalid())
return ExprError();
-
- QualType Type = Ex.get()->getType();
-
- class DeleteConverter : public ContextualImplicitConverter {
- public:
- DeleteConverter() : ContextualImplicitConverter(false, true) {}
-
- bool match(QualType ConvType) override {
- // FIXME: If we have an operator T* and an operator void*, we must pick
- // the operator T*.
- if (const PointerType *ConvPtrType = ConvType->getAs<PointerType>())
- if (ConvPtrType->getPointeeType()->isIncompleteOrObjectType())
- return true;
- return false;
- }
-
- SemaDiagnosticBuilder diagnoseNoMatch(Sema &S, SourceLocation Loc,
- QualType T) override {
- return S.Diag(Loc, diag::err_delete_operand) << T;
- }
-
- SemaDiagnosticBuilder diagnoseIncomplete(Sema &S, SourceLocation Loc,
- QualType T) override {
- return S.Diag(Loc, diag::err_delete_incomplete_class_type) << T;
- }
-
- SemaDiagnosticBuilder diagnoseExplicitConv(Sema &S, SourceLocation Loc,
- QualType T,
- QualType ConvTy) override {
- return S.Diag(Loc, diag::err_delete_explicit_conversion) << T <<
ConvTy;
- }
-
- SemaDiagnosticBuilder noteExplicitConv(Sema &S, CXXConversionDecl *Conv,
- QualType ConvTy) override {
- return S.Diag(Conv->getLocation(), diag::note_delete_conversion)
- << ConvTy;
- }
-
- SemaDiagnosticBuilder diagnoseAmbiguous(Sema &S, SourceLocation Loc,
- QualType T) override {
- return S.Diag(Loc, diag::err_ambiguous_delete_operand) << T;
+ else
+ Ex = Conv.get();
+ QualType Type = Ex->getType();
+
+ if (Type->isRecordType()) {
+ DeleteConverter Converter;
+ // Suppress diagnostics the first time around
+ Converter.Suppress = true;
+
+ if (ExprResult Conv =
+ PerformContextualImplicitConversion(StartLoc, Ex, Converter);
+ !Conv.isInvalid() && Conv.get() != Ex) {
+ Ex = Conv.get();
+ } else {
+ // As an extension, allow void pointers
+ Converter.AllowVoidPointer = true;
+ Converter.Suppress = false;
+ Conv = PerformContextualImplicitConversion(StartLoc, Ex, Converter);
+ if (Conv.isInvalid() || Conv.get() == Ex)
+ return ExprError();
+ Ex = Conv.get();
}
- SemaDiagnosticBuilder noteAmbiguous(Sema &S, CXXConversionDecl *Conv,
- QualType ConvTy) override {
- return S.Diag(Conv->getLocation(), diag::note_delete_conversion)
- << ConvTy;
- }
+ Type = Ex->getType();
+ assert(Converter.match(Type) && "PerformContextualImplicitConversion "
+ "returned something of the wrong type");
+ } else {
+ if (Type->isArrayType())
+ Diag(StartLoc, diag::warn_delete_array) << Type;
- SemaDiagnosticBuilder diagnoseConversion(Sema &S, SourceLocation Loc,
- QualType T,
- QualType ConvTy) override {
- llvm_unreachable("conversion functions are permitted");
+ if (ExprResult Conv = DefaultFunctionArrayLvalueConversion(Ex);
+ Conv.isInvalid())
+ return ExprError();
+ else
+ Ex = Conv.get();
+ Type = Ex->getType();
+ if (!Type->isObjectPointerType()) {
+ Diag(StartLoc, diag::err_delete_operand) << Type;
+ return ExprError();
}
- } Converter;
-
- Ex = PerformContextualImplicitConversion(StartLoc, Ex.get(), Converter);
- if (Ex.isInvalid())
- return ExprError();
- Type = Ex.get()->getType();
- if (!Converter.match(Type))
- // FIXME: PerformContextualImplicitConversion should return ExprError
- // itself in this case.
- return ExprError();
+ }
QualType Pointee = Type->castAs<PointerType>()->getPointeeType();
QualType PointeeElem = Context.getBaseElementType(Pointee);
if (Pointee.getAddressSpace() != LangAS::Default &&
!getLangOpts().OpenCLCPlusPlus)
- return Diag(Ex.get()->getBeginLoc(),
- diag::err_address_space_qualified_delete)
+ return Diag(Ex->getBeginLoc(), diag::err_address_space_qualified_delete)
<< Pointee.getUnqualifiedType()
<< Pointee.getQualifiers().getAddressSpaceAttributePrintValue();
@@ -3780,16 +3800,16 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool
UseGlobal,
// 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();
+ << Type << Ex->getSourceRange();
} else if (Pointee->isFunctionType() || Pointee->isVoidType() ||
Pointee->isSizelessType()) {
return ExprError(Diag(StartLoc, diag::err_delete_operand)
- << Type << Ex.get()->getSourceRange());
+ << Type << Ex->getSourceRange());
} else if (!Pointee->isDependentType()) {
// 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())) {
+ if (!RequireCompleteType(StartLoc, Pointee, diag::warn_delete_incomplete,
+ Ex)) {
if (const RecordType *RT = PointeeElem->getAs<RecordType>())
PointeeRD = cast<CXXRecordDecl>(RT->getDecl());
}
@@ -3797,7 +3817,7 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool
UseGlobal,
if (Pointee->isArrayType() && !ArrayForm) {
Diag(StartLoc, diag::warn_delete_array_type)
- << Type << Ex.get()->getSourceRange()
+ << Type << Ex->getSourceRange()
<< FixItHint::CreateInsertion(getLocForEndOfToken(StartLoc), "[]");
ArrayForm = true;
}
@@ -3867,7 +3887,7 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool
UseGlobal,
bool IsVirtualDelete = false;
if (PointeeRD) {
if (CXXDestructorDecl *Dtor = LookupDestructor(PointeeRD)) {
- CheckDestructorAccess(Ex.get()->getExprLoc(), Dtor,
+ CheckDestructorAccess(Ex->getExprLoc(), Dtor,
PDiag(diag::err_access_dtor) << PointeeElem);
IsVirtualDelete = Dtor->isVirtual();
}
@@ -3888,17 +3908,20 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool
UseGlobal,
Qs.removeCVRQualifiers();
QualType Unqual = Context.getPointerType(
Context.getQualifiedType(Pointee.getUnqualifiedType(), Qs));
- Ex = ImpCastExprToType(Ex.get(), Unqual, CK_NoOp);
+ Ex = ImpCastExprToType(Ex, Unqual, CK_NoOp).get();
}
- Ex = PerformImplicitConversion(Ex.get(), ParamType, AA_Passing);
- if (Ex.isInvalid())
+ if (ExprResult Conv =
+ PerformImplicitConversion(Ex, ParamType, AA_Passing);
+ Conv.isInvalid())
return ExprError();
+ else
+ Ex = Conv.get();
}
}
- CXXDeleteExpr *Result = new (Context) CXXDeleteExpr(
- Context.VoidTy, UseGlobal, ArrayForm, ArrayFormAsWritten,
- UsualArrayDeleteWantsSize, OperatorDelete, Ex.get(), StartLoc);
+ CXXDeleteExpr *Result = new (Context)
+ CXXDeleteExpr(Context.VoidTy, UseGlobal, ArrayForm, ArrayFormAsWritten,
+ UsualArrayDeleteWantsSize, OperatorDelete, Ex, StartLoc);
AnalyzeDeleteExprMismatch(Result);
return Result;
}
diff --git a/clang/test/CXX/drs/cwg5xx.cpp b/clang/test/CXX/drs/cwg5xx.cpp
index 9d890f981348a..64ea340ae25d2 100644
--- a/clang/test/CXX/drs/cwg5xx.cpp
+++ b/clang/test/CXX/drs/cwg5xx.cpp
@@ -1230,11 +1230,11 @@ namespace cwg598 { // cwg598: yes
int &t = h(N::i);
}
-namespace cwg599 { // cwg599: partial
+namespace cwg599 { // cwg599: 19
typedef int Fn();
struct S { operator void*(); };
struct T { operator Fn*(); };
- struct U { operator int*(); operator void*(); }; // #cwg599-U
+ struct U { operator int*(); operator void*(); };
struct V { operator int*(); operator Fn*(); };
void f(void *p, void (*q)(), S s, T t, U u, V v) {
delete p;
@@ -1245,12 +1245,7 @@ namespace cwg599 { // cwg599: partial
// expected-error@-1 {{cannot delete expression with pointer-to-'void'
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
- // extension allowing deletion of pointers to void.
delete u;
- // expected-error@-1 {{ambiguous conversion of delete expression of type
'U' to a pointer}}
- // expected-note@#cwg599-U {{conversion to pointer type 'int *'}}
- // expected-note@#cwg599-U {{conversion to pointer type 'void *'}}
delete v;
}
}
diff --git a/clang/test/Parser/cxx2c-delete-with-message.cpp
b/clang/test/Parser/cxx2c-delete-with-message.cpp
index 1767a080a7dcd..36575bf42e824 100644
--- a/clang/test/Parser/cxx2c-delete-with-message.cpp
+++ b/clang/test/Parser/cxx2c-delete-with-message.cpp
@@ -46,6 +46,8 @@ U b = delete ("hello"), c, d = delete ("hello"); //
expected-error 2 {{only func
struct C {
T e = delete ("hello"); // expected-error {{'= delete' is a function
definition and must occur in a standalone declaration}}
- U f = delete ("hello"); // expected-error {{cannot delete expression of type
'const char[6]'}}
+ U f = delete ("hello");
+// expected-warning@-1 {{deleting array of type 'const char[6]'}}
+// expected-error@-2 {{cannot initialize a member subobject of type 'U' (aka
'int') with an rvalue of type 'void'}}
};
}
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 9d458330f5376..2bcab4be98b40 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -3636,7 +3636,7 @@ <h2 id="cxxdr">C++ defect report implementation
status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/599.html";>599</a></td>
<td>CD2</td>
<td>Deleting a null function pointer</td>
- <td class="partial" align="center">Partial</td>
+ <td class="unreleased" align="center">Clang 19</td>
</tr>
<tr id="600">
<td><a href="https://cplusplus.github.io/CWG/issues/600.html";>600</a></td>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
