Hi rsmith, pcc,
This is a follow-up to Richard's comments from
http://llvm-reviews.chandlerc.com/D2401#comment-2: "I wonder if we're being too
eager when performing operator delete lookup for a class with a virtual
destructor. Instead of performing the lookup when the destructor is declared,
could we defer it until the vtable is used?"
The patch is an attempt to do this. It does clean up the tests nicely.
pcc's patch that added this check is here:
http://llvm-reviews.chandlerc.com/D822. With my patch we will no longer reject
his example:
struct B {
void operator delete(void *);
};
struct C : A, B {
virtual ~C();
};
.. unless C's vtable is marked used.
Besides just moving the check, I also updated
CXXDestructorDecl::{get,set}OperatorDelete() to work with the first
declaration. Without this, for some reason, in the code below we would set
OperatorDelete on the function *definition* (because that's where we perform
the check), and then fail an assert in codegen for the function *declaration*.
I'm not sure why we end up in that situation on win32 and not linux :/
struct F {
virtual ~F();
};
F::~F() {
}
F f;
http://llvm-reviews.chandlerc.com/D2851
Files:
include/clang/AST/DeclCXX.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
test/CXX/drs/dr2xx.cpp
test/CXX/special/class.dtor/p9.cpp
test/SemaCXX/microsoft-dtor-lookup.cpp
Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -2307,8 +2307,12 @@
bool isImplicitlyDeclared);
static CXXDestructorDecl *CreateDeserialized(ASTContext & C, unsigned ID);
- void setOperatorDelete(FunctionDecl *OD) { OperatorDelete = OD; }
- const FunctionDecl *getOperatorDelete() const { return OperatorDelete; }
+ void setOperatorDelete(FunctionDecl *OD) {
+ cast<CXXDestructorDecl>(getFirstDecl())->OperatorDelete = OD;
+ }
+ const FunctionDecl *getOperatorDelete() const {
+ return cast<CXXDestructorDecl>(getFirstDecl())->OperatorDelete;
+ }
// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -6286,15 +6286,6 @@
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() && !NewDD->isDeleted()) {
- SemaRef.CheckDestructor(NewDD);
- }
-
IsVirtualOkay = true;
return NewDD;
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -4479,15 +4479,6 @@
}
}
}
-
- if (Record->hasUserDeclaredDestructor()) {
- // The Microsoft ABI requires that we perform the destructor body
- // checks (i.e. operator delete() lookup) in any translataion unit, as
- // any translation unit may need to emit a deleting destructor.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
- !Record->getDestructor()->isDeleted())
- CheckDestructor(Record->getDestructor());
- }
}
// C++11 [dcl.constexpr]p8: A constexpr specifier for a non-static member
@@ -12345,6 +12336,17 @@
// Otherwise, we can early exit.
return;
}
+ } else {
+ // The Microsoft ABI requires that we perform the destructor body
+ // checks (i.e. operator delete() lookup) when the vtable is marked used, as
+ // any translation unit may need to emit a deleting destructor.
+ // If it has a definition, we do the check at that point instead.
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+ Class->hasUserDeclaredDestructor() &&
+ !Class->getDestructor()->isDefined() &&
+ !Class->getDestructor()->isDeleted()) {
+ CheckDestructor(Class->getDestructor());
+ }
}
// Local classes need to have their virtual members marked
Index: test/CXX/drs/dr2xx.cpp
===================================================================
--- test/CXX/drs/dr2xx.cpp
+++ test/CXX/drs/dr2xx.cpp
@@ -1,10 +1,6 @@
-// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -triple %itanium_abi_triple
-// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -triple %itanium_abi_triple
-// RUN: %clang_cc1 -std=c++1y %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -triple %itanium_abi_triple
-
-// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -triple %ms_abi_triple -DMSABI
-// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -triple %ms_abi_triple -DMSABI
-// RUN: %clang_cc1 -std=c++1y %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -triple %ms_abi_triple -DMSABI
+// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++1y %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
// PR13819 -- __SIZE_TYPE__ is incompatible.
typedef __SIZE_TYPE__ size_t; // expected-error 0-1 {{extension}}
@@ -538,30 +534,17 @@
namespace dr252 { // dr252: yes
struct A {
-#ifdef MSABI
- // expected-note@+2 {{found}}
-#endif
void operator delete(void*); // expected-note {{found}}
};
struct B {
-#ifdef MSABI
- // expected-note@+2 {{found}}
-#endif
void operator delete(void*); // expected-note {{found}}
};
struct C : A, B {
-#ifdef MSABI
- // expected-error@+2 {{'operator delete' found in multiple base classes}}
-#endif
virtual ~C();
};
C::~C() {} // expected-error {{'operator delete' found in multiple base classes}}
struct D {
-#ifdef MSABI
- // expected-note@+3 {{here}} MSABI
- // expected-error@+3 {{no suitable member 'operator delete'}}
-#endif
void operator delete(void*, int); // expected-note {{here}}
virtual ~D();
};
@@ -577,10 +560,6 @@
struct F {
// If both functions are available, the first one is a placement delete.
void operator delete(void*, size_t);
-#ifdef MSABI
- // expected-note@+3 {{here}}
- // expected-error@+3 {{attempt to use a deleted function}}
-#endif
void operator delete(void*) = delete; // expected-error 0-1{{extension}} expected-note {{here}}
virtual ~F();
};
Index: test/CXX/special/class.dtor/p9.cpp
===================================================================
--- test/CXX/special/class.dtor/p9.cpp
+++ test/CXX/special/class.dtor/p9.cpp
@@ -29,18 +29,12 @@
namespace test1 {
class A {
public:
-#ifdef MSABI
- // expected-note@+2 {{declared here}}
-#endif
static void operator delete(void *p) {}; // expected-note {{member 'operator delete' declared here}}
virtual ~A();
};
class B : protected A {
public:
-#ifdef MSABI
- // expected-note@+2 {{declared here}}
-#endif
static void operator delete(void *, size_t) {}; // expected-note {{member 'operator delete' declared here}}
~B();
};
@@ -50,9 +44,6 @@
using A::operator delete;
using B::operator delete;
-#ifdef MSABI
- // expected-error@+2 {{multiple suitable 'operator delete' functions in 'C'}}
-#endif
~C();
};
@@ -62,22 +53,12 @@
// ...at the point of definition of a virtual destructor...
namespace test2 {
struct A {
-#ifdef MSABI
- // expected-error@+3 {{no suitable member 'operator delete' in 'A'}}
- // expected-note@+3 {{declared here}}
-#endif
virtual ~A();
static void operator delete(void*, const int &);
};
struct B {
-#ifdef MSABI
- // expected-error@+2 {{no suitable member 'operator delete' in 'B'}}
-#endif
virtual ~B();
-#ifdef MSABI
- // expected-note@+2 {{declared here}}
-#endif
static void operator delete(void*, const int &); // expected-note {{declared here}}
};
B::~B() {} // expected-error {{no suitable member 'operator delete' in 'B'}}
Index: test/SemaCXX/microsoft-dtor-lookup.cpp
===================================================================
--- test/SemaCXX/microsoft-dtor-lookup.cpp
+++ test/SemaCXX/microsoft-dtor-lookup.cpp
@@ -1,12 +1,11 @@
// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only %s
-// RUN: %clang_cc1 -triple %ms_abi_triple -verify -DMSVC_ABI %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -verify %s
namespace Test1 {
// 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.
+// operator delete() lookups to be done when vtables are marked used.
struct A {
void operator delete(void *); // expected-note {{member found by ambiguous name lookup}}
@@ -24,6 +23,10 @@
virtual ~VC(); // expected-error {{member 'operator delete' found in multiple base classes of different types}}
};
+void f(VC vc) {
+ // This marks VC's vtable used.
+}
+
}
namespace Test2 {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
