[cfe-commits] r83080 - in /cfe/trunk: include/clang/AST/DeclCXX.h lib/AST/DeclCXX.cpp lib/CodeGen/CGCXXExpr.cpp lib/Sema/SemaExprCXX.cpp test/SemaCXX/new-delete.cpp

Douglas Gregor dgregor at apple.com
Tue Sep 29 11:16:17 PDT 2009


Author: dgregor
Date: Tue Sep 29 13:16:17 2009
New Revision: 83080

URL: http://llvm.org/viewvc/llvm-project?rev=83080&view=rev
Log:
Handle C++ delete expressions when the overloaded delete operator is a
"usual deallocation function" with two arguments. CodeGen will have to
handle this case specifically, since the value for the second argument
(the size of the allocated object) may have to be computed at run
time.

Fixes the Sema part of PR4782.

Modified:
    cfe/trunk/include/clang/AST/DeclCXX.h
    cfe/trunk/lib/AST/DeclCXX.cpp
    cfe/trunk/lib/CodeGen/CGCXXExpr.cpp
    cfe/trunk/lib/Sema/SemaExprCXX.cpp
    cfe/trunk/test/SemaCXX/new-delete.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=83080&r1=83079&r2=83080&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Sep 29 13:16:17 2009
@@ -785,6 +785,11 @@
     return (CD->begin_overridden_methods() != CD->end_overridden_methods());
   }
   
+  /// \brief Determine whether this is a usual deallocation function
+  /// (C++ [basic.stc.dynamic.deallocation]p2), which is an overloaded
+  /// delete or delete[] operator with a particular signature.
+  bool isUsualDeallocationFunction() const;
+  
   const CXXMethodDecl *getCanonicalDecl() const {
     return cast<CXXMethodDecl>(FunctionDecl::getCanonicalDecl());
   }

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=83080&r1=83079&r2=83080&view=diff

==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Tue Sep 29 13:16:17 2009
@@ -451,6 +451,40 @@
                                isStatic, isInline);
 }
 
+bool CXXMethodDecl::isUsualDeallocationFunction() const {
+  if (getOverloadedOperator() != OO_Delete &&
+      getOverloadedOperator() != OO_Array_Delete)
+    return false;
+  
+  // C++ [basic.stc.dynamic.deallocation]p2:
+  //   If a class T has a member deallocation function named operator delete 
+  //   with exactly one parameter, then that function is a usual (non-placement)
+  //   deallocation function. [...]
+  if (getNumParams() == 1)
+    return true;
+  
+  // C++ [basic.stc.dynamic.deallocation]p2:
+  //   [...] If class T does not declare such an operator delete but does 
+  //   declare a member deallocation function named operator delete with 
+  //   exactly two parameters, the second of which has type std::size_t (18.1),
+  //   then this function is a usual deallocation function.
+  ASTContext &Context = getASTContext();
+  if (getNumParams() != 2 ||
+      !Context.hasSameType(getParamDecl(1)->getType(), Context.getSizeType()))
+    return false;
+                 
+  // This function is a usual deallocation function if there are no 
+  // single-parameter deallocation functions of the same kind.
+  for (DeclContext::lookup_const_result R = getDeclContext()->lookup(getDeclName());
+       R.first != R.second; ++R.first) {
+    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(*R.first))
+      if (FD->getNumParams() == 1)
+        return false;
+  }
+  
+  return true;
+}
+
 typedef llvm::DenseMap<const CXXMethodDecl*,
                        std::vector<const CXXMethodDecl *> *>
                        OverriddenMethodsMapTy;

Modified: cfe/trunk/lib/CodeGen/CGCXXExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXExpr.cpp?rev=83080&r1=83079&r2=83080&view=diff

==============================================================================
--- cfe/trunk/lib/CodeGen/CGCXXExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCXXExpr.cpp Tue Sep 29 13:16:17 2009
@@ -239,10 +239,18 @@
     return;
   };
 
-  QualType DeleteTy =
-    E->getArgument()->getType()->getAs<PointerType>()->getPointeeType();
+  // Get at the argument before we performed the implicit conversion
+  // to void*.
+  const Expr *Arg = E->getArgument();
+  while (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Arg)) {
+    if (ICE->getCastKind() != CastExpr::CK_UserDefinedConversion &&
+        ICE->getType()->isVoidPointerType())
+      Arg = ICE->getSubExpr();
+  }
+  
+  QualType DeleteTy = Arg->getType()->getAs<PointerType>()->getPointeeType();
 
-  llvm::Value *Ptr = EmitScalarExpr(E->getArgument());
+  llvm::Value *Ptr = EmitScalarExpr(Arg);
 
   // Null check the pointer.
   llvm::BasicBlock *DeleteNotNull = createBasicBlock("delete.notnull");

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=83080&r1=83079&r2=83080&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Sep 29 13:16:17 2009
@@ -589,6 +589,7 @@
                                   DeclarationName Name, Expr** Args,
                                   unsigned NumArgs, DeclContext *Ctx,
                                   bool AllowMissing, FunctionDecl *&Operator) {
+  // FIXME: Change to use LookupQualifiedName!
   DeclContext::lookup_iterator Alloc, AllocEnd;
   llvm::tie(Alloc, AllocEnd) = Ctx->lookup(Name);
   if (Alloc == AllocEnd) {
@@ -602,9 +603,13 @@
   for (; Alloc != AllocEnd; ++Alloc) {
     // Even member operator new/delete are implicitly treated as
     // static, so don't use AddMemberCandidate.
-    if (FunctionDecl *Fn = dyn_cast<FunctionDecl>(*Alloc))
+    if (FunctionDecl *Fn = dyn_cast<FunctionDecl>(*Alloc)) {
       AddOverloadCandidate(Fn, Args, NumArgs, Candidates,
                            /*SuppressUserConversions=*/false);
+      continue;
+    } 
+    
+    // FIXME: Handle function templates
   }
 
   // Do the resolution.
@@ -616,7 +621,7 @@
     // The first argument is size_t, and the first parameter must be size_t,
     // too. This is checked on declaration and can be assumed. (It can't be
     // asserted on, though, since invalid decls are left in there.)
-    for (unsigned i = 1; i < NumArgs; ++i) {
+    for (unsigned i = 0; i < NumArgs; ++i) {
       // FIXME: Passing word to diagnostic.
       if (PerformCopyInitialization(Args[i],
                                     FnDecl->getParamDecl(i)->getType(),
@@ -844,27 +849,27 @@
                                    << Ex->getSourceRange()))
       return ExprError();
 
-    // FIXME: This should be shared with the code for finding the delete
-    // operator in ActOnCXXNew.
-    IntegerLiteral Size(llvm::APInt::getNullValue(
-                        Context.Target.getPointerWidth(0)),
-                        Context.getSizeType(),
-                        SourceLocation());
-    ImplicitCastExpr Cast(Context.getPointerType(Context.VoidTy),
-                          CastExpr::CK_Unknown, &Size, false);
-    Expr *DeleteArg = &Cast;
-
     DeclarationName DeleteName = Context.DeclarationNames.getCXXOperatorName(
                                       ArrayForm ? OO_Array_Delete : OO_Delete);
 
     if (Pointee->isRecordType() && !UseGlobal) {
       CXXRecordDecl *Record
         = cast<CXXRecordDecl>(Pointee->getAs<RecordType>()->getDecl());
-      // FIXME: We fail to find inherited overloads.
-      if (FindAllocationOverload(StartLoc, SourceRange(), DeleteName,
-                                 &DeleteArg, 1, Record, /*AllowMissing=*/true,
-                                 OperatorDelete))
-        return ExprError();
+      
+      // Try to find operator delete/operator delete[] in class scope.
+      LookupResult Found = LookupQualifiedName(Record, DeleteName, 
+                                               LookupOrdinaryName);
+      // FIXME: Diagnose ambiguity properly
+      assert(!Found.isAmbiguous() && "Ambiguous delete/delete[] not handled");
+      for (LookupResult::iterator F = Found.begin(), FEnd = Found.end();
+           F != FEnd; ++F) {
+        if (CXXMethodDecl *Delete = dyn_cast<CXXMethodDecl>(*F))
+          if (Delete->isUsualDeallocationFunction()) {
+            OperatorDelete = Delete;
+            break;
+          }
+      }
+      
       if (!Record->hasTrivialDestructor())
         if (const CXXDestructorDecl *Dtor = Record->getDestructor(Context))
           MarkDeclarationReferenced(StartLoc,
@@ -876,7 +881,7 @@
       DeclareGlobalNewDelete();
       DeclContext *TUDecl = Context.getTranslationUnitDecl();
       if (FindAllocationOverload(StartLoc, SourceRange(), DeleteName,
-                                 &DeleteArg, 1, TUDecl, /*AllowMissing=*/false,
+                                 &Ex, 1, TUDecl, /*AllowMissing=*/false,
                                  OperatorDelete))
         return ExprError();
     }

Modified: cfe/trunk/test/SemaCXX/new-delete.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/new-delete.cpp?rev=83080&r1=83079&r2=83080&view=diff

==============================================================================
--- cfe/trunk/test/SemaCXX/new-delete.cpp (original)
+++ cfe/trunk/test/SemaCXX/new-delete.cpp Tue Sep 29 13:16:17 2009
@@ -115,3 +115,20 @@
   delete x1;
   delete x2; // expected-error{{ambiguous conversion of delete expression of type 'struct X2' to a pointer}}
 }
+
+// PR4782
+class X3 {
+public:
+  static void operator delete(void * mem, unsigned long size);
+};
+
+class X4 {
+public:
+  static void release(X3 *x);
+  static void operator delete(void * mem, unsigned long size);
+};
+
+
+void X4::release(X3 *x) {
+  delete x;
+}





More information about the cfe-commits mailing list