A couple minor issues with Sema for delete:

1. Attempting to delete an expression of incomplete class type should be an error, not a warning.

2. If someone tries to delete a pointer to an incomplete class type, make sure we actually emit
the delete expression after we warn.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@136161 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Basic/DiagnosticSemaKinds.td

@@ -3364,7 +3364,7 @@ def err_ambiguous_delete_operand : Error<"ambiguous conversion of delete "
                                          "expression of type %0 to a pointer">;
 def warn_delete_incomplete : Warning<
   "deleting pointer to incomplete type %0 may cause undefined behaviour">;
-def err_delete_incomplete_class_type : Warning<
+def err_delete_incomplete_class_type : Error<
   "deleting incomplete class type %0; no conversions to pointer type">;
 def warn_delete_array_type : Warning<
   "'delete' applied to a pointer-to-array type %0 treated as delete[]">;

lib/Sema/SemaExprCXX.cpp

@@ -1840,24 +1840,32 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
         << Type << Ex.get()->getSourceRange());
 
     QualType Pointee = Type->getAs<PointerType>()->getPointeeType();
+    QualType PointeeElem = Context.getBaseElementType(Pointee);
+
+    if (unsigned AddressSpace = Pointee.getAddressSpace())
+      return Diag(Ex.get()->getLocStart(), 
+                  diag::err_address_space_qualified_delete)
+               << Pointee.getUnqualifiedType() << AddressSpace;
+
+    CXXRecordDecl *PointeeRD = 0;
     if (Pointee->isVoidType() && !isSFINAEContext()) {
       // 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();
-    } else if (Pointee->isFunctionType() || Pointee->isVoidType())
+    } else if (Pointee->isFunctionType() || Pointee->isVoidType()) {
       return ExprError(Diag(StartLoc, diag::err_delete_operand)
         << Type << Ex.get()->getSourceRange());
-    else if (!Pointee->isDependentType() &&
-             RequireCompleteType(StartLoc, Pointee,
-                                 PDiag(diag::warn_delete_incomplete)
-                                   << Ex.get()->getSourceRange()))
-      return ExprError();
-    else if (unsigned AddressSpace = Pointee.getAddressSpace())
-      return Diag(Ex.get()->getLocStart(), 
-                  diag::err_address_space_qualified_delete)
-               << Pointee.getUnqualifiedType() << AddressSpace;
+    } else if (!Pointee->isDependentType()) {
+      if (!RequireCompleteType(StartLoc, Pointee,
+                               PDiag(diag::warn_delete_incomplete)
+                                 << Ex.get()->getSourceRange())) {
+        if (const RecordType *RT = PointeeElem->getAs<RecordType>())
+          PointeeRD = cast<CXXRecordDecl>(RT->getDecl());
+      }
+    }
+
     // C++ [expr.delete]p2:
     //   [Note: a pointer to a const type can be the operand of a
     //   delete-expression; it is not necessary to cast away the constness
@@ -1877,12 +1885,10 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
     DeclarationName DeleteName = Context.DeclarationNames.getCXXOperatorName(
                                       ArrayForm ? OO_Array_Delete : OO_Delete);
 
-    QualType PointeeElem = Context.getBaseElementType(Pointee);
-    if (const RecordType *RT = PointeeElem->getAs<RecordType>()) {
-      CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
-
+    if (PointeeRD) {
       if (!UseGlobal &&
-          FindDeallocationFunction(StartLoc, RD, DeleteName, OperatorDelete))
+          FindDeallocationFunction(StartLoc, PointeeRD, DeleteName,
+                                   OperatorDelete))
         return ExprError();
 
       // If we're allocating an array of records, check whether the
@@ -1900,8 +1906,8 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
           UsualArrayDeleteWantsSize = (OperatorDelete->getNumParams() == 2);
       }
 
-      if (!RD->hasTrivialDestructor())
-        if (CXXDestructorDecl *Dtor = LookupDestructor(RD)) {
+      if (!PointeeRD->hasTrivialDestructor())
+        if (CXXDestructorDecl *Dtor = LookupDestructor(PointeeRD)) {
           MarkDeclarationReferenced(StartLoc,
                                     const_cast<CXXDestructorDecl*>(Dtor));
           DiagnoseUseOfDecl(Dtor, StartLoc);
@@ -1915,8 +1921,9 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
       //   behavior is undefined.
       //
       // Note: a final class cannot be derived from, no issue there
-      if (!ArrayForm && RD->isPolymorphic() && !RD->hasAttr<FinalAttr>()) {
-        CXXDestructorDecl *dtor = RD->getDestructor();
+      if (!ArrayForm && PointeeRD->isPolymorphic() &&
+          !PointeeRD->hasAttr<FinalAttr>()) {
+        CXXDestructorDecl *dtor = PointeeRD->getDestructor();
         if (!dtor || !dtor->isVirtual())
           Diag(StartLoc, diag::warn_delete_non_virtual_dtor) << PointeeElem;
       }
@@ -1944,9 +1951,8 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
     MarkDeclarationReferenced(StartLoc, OperatorDelete);
     
     // Check access and ambiguity of operator delete and destructor.
-    if (const RecordType *RT = PointeeElem->getAs<RecordType>()) {
-      CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
-      if (CXXDestructorDecl *Dtor = LookupDestructor(RD)) {
+    if (PointeeRD) {
+      if (CXXDestructorDecl *Dtor = LookupDestructor(PointeeRD)) {
           CheckDestructorAccess(Ex.get()->getExprLoc(), Dtor, 
                       PDiag(diag::err_access_dtor) << PointeeElem);
       }

test/SemaCXX/new-delete.cpp

@@ -397,3 +397,15 @@ namespace ArrayNewNeedsDtor {
     return new B[5]; // expected-note {{implicit default destructor for 'ArrayNewNeedsDtor::B' first required here}}
   }
 }
+
+namespace DeleteIncompleteClass {
+  struct A; // expected-note {{forward declaration}}
+  extern A x;
+  void f() { delete x; } // expected-error {{deleting incomplete class type}}
+}
+
+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}}
+}