[PATCH] [ms-cxxabi] Look up operator delete() at every virtual dtor declaration.

[Peter Collingbourne]

Hi rsmith,

While the C++ standard requires that this lookup take place only at
the definition point of a virtual destructor (C++11 [class.dtor]p12),
the Microsoft ABI may require the compiler to emit a deleting
destructor for any virtual destructor declared in the TU, including
ones without a body, requiring an operator delete() lookup for every
virtual destructor declaration.  As far as I can tell, the result of
the lookup should be the same no matter which declaration is used.

This change will cause us to reject some valid TUs in Microsoft ABI
mode, e.g.:

struct A {
  void operator delete(void *);
};

struct B {
  void operator delete(void *);
};

struct C : A, B {
  virtual ~C();
};

However, because I believe there to be no valid definition of C::~C()
(and because cl.exe will also reject this TU if C::~C() is referenced
in any way), this should be tolerable.

http://llvm-reviews.chandlerc.com/D822

Files:
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCXX/microsoft-dtor-lookup.cpp

Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -5777,6 +5777,15 @@
         SemaRef.AdjustDestructorExceptionSpec(Record, NewDD);
       }
 
+      // The Microsoft ABI requires that we perform the destructor body
+      // checks (i.e. operator delete() lookup) at every declaration, as
+      // any translation unit may need to emit a deleting destructor.
+      if (SemaRef.Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+          !Record->isDependentType() && Record->getDefinition() &&
+          !Record->isBeingDefined()) {
+        SemaRef.CheckDestructor(NewDD);
+      }
+
       IsVirtualOkay = true;
       return NewDD;
 
@@ -11145,10 +11154,18 @@
           I.setAccess((*I)->getAccess());
         
         if (!CXXRecord->isDependentType()) {
-          // Adjust user-defined destructor exception spec.
-          if (getLangOpts().CPlusPlus11 &&
-              CXXRecord->hasUserDeclaredDestructor())
-            AdjustDestructorExceptionSpec(CXXRecord,CXXRecord->getDestructor());
+          if (CXXRecord->hasUserDeclaredDestructor()) {
+            // Adjust user-defined destructor exception spec.
+            if (getLangOpts().CPlusPlus11)
+              AdjustDestructorExceptionSpec(CXXRecord,
+                                            CXXRecord->getDestructor());
+
+            // The Microsoft ABI requires that we perform the destructor body
+            // checks (i.e. operator delete() lookup) at every declaration, as
+            // any translation unit may need to emit a deleting destructor.
+            if (Context.getTargetInfo().getCXXABI().isMicrosoft())
+              CheckDestructor(CXXRecord->getDestructor());
+          }
 
           // Add any implicitly-declared members to this class.
           AddImplicitlyDeclaredMembersToClass(CXXRecord);
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5896,7 +5896,7 @@
 bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
   CXXRecordDecl *RD = Destructor->getParent();
   
-  if (Destructor->isVirtual()) {
+  if (!Destructor->getOperatorDelete() && Destructor->isVirtual()) {
     SourceLocation Loc;
     
     if (!Destructor->isImplicit())
Index: test/SemaCXX/microsoft-dtor-lookup.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/microsoft-dtor-lookup.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple i686-pc-win32 -cxx-abi itanium -fsyntax-only %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -cxx-abi microsoft -verify %s
+
+// Should be accepted under the Itanium ABI (first RUN line) but rejected
+// under the Microsoft ABI (second RUN line), as Microsoft ABI requires
+// operator delete() lookups to be done at all virtual destructor declaration
+// points.
+
+struct A {
+  void operator delete(void *); // expected-note {{member found by ambiguous name lookup}}
+};
+
+struct B {
+  void operator delete(void *); // expected-note {{member found by ambiguous name lookup}}
+};
+
+struct C : A, B {
+  ~C();
+};
+
+struct VC : A, B {
+  virtual ~VC(); // expected-error {{member 'operator delete' found in multiple base classes of different types}}
+};
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D822.1.patch
Type: text/x-patch
Size: 3350 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130519/c63032a9/attachment.bin>