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}}
+};
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}}
+};
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits