From: Peter Collingbourne <peter@pcc.me.uk>
Date: Mon, 20 May 2013 14:12:25 +0000 (+0000)
Subject: [ms-cxxabi] Look up operator delete() at every virtual dtor declaration.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f51cfb89b3fe317318e434db4856b06a90afc126;p=clang

[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

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

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 50951a03f0..49c8fe5e71 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -5786,6 +5786,15 @@ static FunctionDecl* CreateNewFunctionDecl(Sema &SemaRef, Declarator &D,
         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);
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 437a9da422..573738cedb 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -5896,7 +5896,7 @@ void Sema::CheckConstructor(CXXConstructorDecl *Constructor) {
 bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
   CXXRecordDecl *RD = Destructor->getParent();
   
-  if (Destructor->isVirtual()) {
+  if (!Destructor->getOperatorDelete() && Destructor->isVirtual()) {
     SourceLocation Loc;
     
     if (!Destructor->isImplicit())
diff --git a/test/SemaCXX/microsoft-dtor-lookup.cpp b/test/SemaCXX/microsoft-dtor-lookup.cpp
new file mode 100644
index 0000000000..e7a60970e1
--- /dev/null
+++ b/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}}
+};