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

[Peter Collingbourne]

Author: pcc
Date: Mon May 20 09:12:25 2013
New Revision: 182270

URL: http://llvm.org/viewvc/llvm-project?rev=182270&view=rev
Log:
[ms-cxxabi] Look up operator delete() at every virtual dtor declaration.

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.  The result of the lookup should be the same
no matter which declaration is used (except in weird corner cases).

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();
};

As Richard points out, every virtual function declared in a TU
(including this virtual destructor) is odr-used, so it must be defined
in any program which declares it, or the program is ill formed, no
diagnostic required.  Because we know that any definition of this
destructor will cause the lookup to fail, the compiler can choose to
issue a diagnostic here.

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

Added:
    cfe/trunk/test/SemaCXX/microsoft-dtor-lookup.cpp
Modified:
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=182270&r1=182269&r2=182270&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon May 20 09:12:25 2013
@@ -5786,6 +5786,15 @@ static FunctionDecl* CreateNewFunctionDe
         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;
 
@@ -11154,10 +11163,18 @@ void Sema::ActOnFields(Scope* S,
           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);

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=182270&r1=182269&r2=182270&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon May 20 09:12:25 2013
@@ -5896,7 +5896,7 @@ void Sema::CheckConstructor(CXXConstruct
 bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
   CXXRecordDecl *RD = Destructor->getParent();
   
-  if (Destructor->isVirtual()) {
+  if (!Destructor->getOperatorDelete() && Destructor->isVirtual()) {
     SourceLocation Loc;
     
     if (!Destructor->isImplicit())

Added: cfe/trunk/test/SemaCXX/microsoft-dtor-lookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/microsoft-dtor-lookup.cpp?rev=182270&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/microsoft-dtor-lookup.cpp (added)
+++ cfe/trunk/test/SemaCXX/microsoft-dtor-lookup.cpp Mon May 20 09:12:25 2013
@@ -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}}
+};