Hi rjmccall,
Hi John,
This small patch fixes http://llvm.org/PR16735
I've decided to go the same way Reid went with EmitCXXDestructors earlier, i.e.
abstract out emission of constructors to CGCXXABI rather than making the
conditions more complex in CGM.
Can you please review this patch?
Thanks,
Timur
http://llvm-reviews.chandlerc.com/D1248
Files:
lib/CodeGen/CGCXX.cpp
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/microsoft-abi-structors.cpp
Index: lib/CodeGen/CGCXX.cpp
===================================================================
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -172,20 +172,6 @@
return false;
}
-void CodeGenModule::EmitCXXConstructors(const CXXConstructorDecl *D) {
- // The constructor used for constructing this as a complete class;
- // constucts the virtual bases, then calls the base constructor.
- if (!D->getParent()->isAbstract()) {
- // We don't need to emit the complete ctor if the class is abstract.
- EmitGlobal(GlobalDecl(D, Ctor_Complete));
- }
-
- // The constructor used for constructing this as a base class;
- // ignores virtual bases.
- if (getTarget().getCXXABI().hasConstructorVariants())
- EmitGlobal(GlobalDecl(D, Ctor_Base));
-}
-
void CodeGenModule::EmitCXXConstructor(const CXXConstructorDecl *ctor,
CXXCtorType ctorType) {
// The complete constructor is equivalent to the base constructor
Index: lib/CodeGen/CGCXXABI.h
===================================================================
--- lib/CodeGen/CGCXXABI.h
+++ lib/CodeGen/CGCXXABI.h
@@ -234,6 +234,9 @@
virtual llvm::BasicBlock *EmitCtorCompleteObjectHandler(CodeGenFunction &CGF,
const CXXRecordDecl
*RD);
+ /// Emit constructor variants required by this ABI.
+ virtual void EmitCXXConstructors(const CXXConstructorDecl *D) = 0;
+
/// Build the signature of the given destructor variant by adding
/// any required parameters. For convenience, ArgTys has been initialized
/// with the type of 'this' and ResTy has been initialized with the type of
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2878,7 +2878,7 @@
cast<FunctionDecl>(D)->isLateTemplateParsed())
return;
- EmitCXXConstructors(cast<CXXConstructorDecl>(D));
+ getCXXABI().EmitCXXConstructors(cast<CXXConstructorDecl>(D));
break;
case Decl::CXXDestructor:
if (cast<FunctionDecl>(D)->isLateTemplateParsed())
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1033,10 +1033,6 @@
void EmitLinkageSpec(const LinkageSpecDecl *D);
void CompleteDIClassType(const CXXMethodDecl* D);
- /// EmitCXXConstructors - Emit constructors (base, complete) from a
- /// C++ constructor Decl.
- void EmitCXXConstructors(const CXXConstructorDecl *D);
-
/// EmitCXXConstructor - Emit a single constructor with the given type from
/// a C++ constructor Decl.
void EmitCXXConstructor(const CXXConstructorDecl *D, CXXCtorType Type);
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -112,6 +112,8 @@
CanQualType &ResTy,
SmallVectorImpl<CanQualType> &ArgTys);
+ void EmitCXXConstructors(const CXXConstructorDecl *D);
+
void BuildDestructorSignature(const CXXDestructorDecl *Dtor,
CXXDtorType T,
CanQualType &ResTy,
@@ -772,6 +774,22 @@
ArgTys.push_back(Context.getPointerType(Context.VoidPtrTy));
}
+void ItaniumCXXABI::EmitCXXConstructors(const CXXConstructorDecl *D) {
+ // Just make sure we're in sync with TargetCXXABI.
+ assert(CGM.getTarget().getCXXABI().hasConstructorVariants());
+
+ // The constructor used for constructing this as a complete class;
+ // constucts the virtual bases, then calls the base constructor.
+ if (!D->getParent()->isAbstract()) {
+ // We don't need to emit the complete ctor if the class is abstract.
+ CGM.EmitGlobal(GlobalDecl(D, Ctor_Complete));
+ }
+
+ // The constructor used for constructing this as a base class;
+ // ignores virtual bases.
+ CGM.EmitGlobal(GlobalDecl(D, Ctor_Base));
+}
+
/// The generic ABI passes 'this', plus a VTT if it's destroying a
/// base subobject.
void ItaniumCXXABI::BuildDestructorSignature(const CXXDestructorDecl *Dtor,
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -66,6 +66,8 @@
llvm::BasicBlock *EmitCtorCompleteObjectHandler(CodeGenFunction &CGF,
const CXXRecordDecl *RD);
+ void EmitCXXConstructors(const CXXConstructorDecl *D);
+
// Background on MSVC destructors
// ==============================
//
@@ -376,6 +378,11 @@
return SkipVbaseCtorsBB;
}
+void MicrosoftCXXABI::EmitCXXConstructors(const CXXConstructorDecl *D) {
+ // There's only one constructor type in this ABI.
+ CGM.EmitGlobal(GlobalDecl(D, Ctor_Complete));
+}
+
void MicrosoftCXXABI::EmitVBPtrStores(CodeGenFunction &CGF,
const CXXRecordDecl *RD) {
llvm::Value *ThisInt8Ptr =
Index: test/CodeGenCXX/microsoft-abi-structors.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-structors.cpp
+++ test/CodeGenCXX/microsoft-abi-structors.cpp
@@ -221,6 +221,15 @@
// CHECK: ret
}
+// PR16735 - even abstract classes should have a constructor emitted.
+struct F {
+ F();
+ virtual void f() = 0;
+};
+
+F::F() {}
+// CHECK: define x86_thiscallcc %"struct.constructors::F"*
@"\01??0F@constructors@@QAE@XZ"
+
} // end namespace constructors
namespace dtors {
Index: lib/CodeGen/CGCXX.cpp
===================================================================
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -172,20 +172,6 @@
return false;
}
-void CodeGenModule::EmitCXXConstructors(const CXXConstructorDecl *D) {
- // The constructor used for constructing this as a complete class;
- // constucts the virtual bases, then calls the base constructor.
- if (!D->getParent()->isAbstract()) {
- // We don't need to emit the complete ctor if the class is abstract.
- EmitGlobal(GlobalDecl(D, Ctor_Complete));
- }
-
- // The constructor used for constructing this as a base class;
- // ignores virtual bases.
- if (getTarget().getCXXABI().hasConstructorVariants())
- EmitGlobal(GlobalDecl(D, Ctor_Base));
-}
-
void CodeGenModule::EmitCXXConstructor(const CXXConstructorDecl *ctor,
CXXCtorType ctorType) {
// The complete constructor is equivalent to the base constructor
Index: lib/CodeGen/CGCXXABI.h
===================================================================
--- lib/CodeGen/CGCXXABI.h
+++ lib/CodeGen/CGCXXABI.h
@@ -234,6 +234,9 @@
virtual llvm::BasicBlock *EmitCtorCompleteObjectHandler(CodeGenFunction &CGF,
const CXXRecordDecl *RD);
+ /// Emit constructor variants required by this ABI.
+ virtual void EmitCXXConstructors(const CXXConstructorDecl *D) = 0;
+
/// Build the signature of the given destructor variant by adding
/// any required parameters. For convenience, ArgTys has been initialized
/// with the type of 'this' and ResTy has been initialized with the type of
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2878,7 +2878,7 @@
cast<FunctionDecl>(D)->isLateTemplateParsed())
return;
- EmitCXXConstructors(cast<CXXConstructorDecl>(D));
+ getCXXABI().EmitCXXConstructors(cast<CXXConstructorDecl>(D));
break;
case Decl::CXXDestructor:
if (cast<FunctionDecl>(D)->isLateTemplateParsed())
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1033,10 +1033,6 @@
void EmitLinkageSpec(const LinkageSpecDecl *D);
void CompleteDIClassType(const CXXMethodDecl* D);
- /// EmitCXXConstructors - Emit constructors (base, complete) from a
- /// C++ constructor Decl.
- void EmitCXXConstructors(const CXXConstructorDecl *D);
-
/// EmitCXXConstructor - Emit a single constructor with the given type from
/// a C++ constructor Decl.
void EmitCXXConstructor(const CXXConstructorDecl *D, CXXCtorType Type);
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -112,6 +112,8 @@
CanQualType &ResTy,
SmallVectorImpl<CanQualType> &ArgTys);
+ void EmitCXXConstructors(const CXXConstructorDecl *D);
+
void BuildDestructorSignature(const CXXDestructorDecl *Dtor,
CXXDtorType T,
CanQualType &ResTy,
@@ -772,6 +774,22 @@
ArgTys.push_back(Context.getPointerType(Context.VoidPtrTy));
}
+void ItaniumCXXABI::EmitCXXConstructors(const CXXConstructorDecl *D) {
+ // Just make sure we're in sync with TargetCXXABI.
+ assert(CGM.getTarget().getCXXABI().hasConstructorVariants());
+
+ // The constructor used for constructing this as a complete class;
+ // constucts the virtual bases, then calls the base constructor.
+ if (!D->getParent()->isAbstract()) {
+ // We don't need to emit the complete ctor if the class is abstract.
+ CGM.EmitGlobal(GlobalDecl(D, Ctor_Complete));
+ }
+
+ // The constructor used for constructing this as a base class;
+ // ignores virtual bases.
+ CGM.EmitGlobal(GlobalDecl(D, Ctor_Base));
+}
+
/// The generic ABI passes 'this', plus a VTT if it's destroying a
/// base subobject.
void ItaniumCXXABI::BuildDestructorSignature(const CXXDestructorDecl *Dtor,
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -66,6 +66,8 @@
llvm::BasicBlock *EmitCtorCompleteObjectHandler(CodeGenFunction &CGF,
const CXXRecordDecl *RD);
+ void EmitCXXConstructors(const CXXConstructorDecl *D);
+
// Background on MSVC destructors
// ==============================
//
@@ -376,6 +378,11 @@
return SkipVbaseCtorsBB;
}
+void MicrosoftCXXABI::EmitCXXConstructors(const CXXConstructorDecl *D) {
+ // There's only one constructor type in this ABI.
+ CGM.EmitGlobal(GlobalDecl(D, Ctor_Complete));
+}
+
void MicrosoftCXXABI::EmitVBPtrStores(CodeGenFunction &CGF,
const CXXRecordDecl *RD) {
llvm::Value *ThisInt8Ptr =
Index: test/CodeGenCXX/microsoft-abi-structors.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-structors.cpp
+++ test/CodeGenCXX/microsoft-abi-structors.cpp
@@ -221,6 +221,15 @@
// CHECK: ret
}
+// PR16735 - even abstract classes should have a constructor emitted.
+struct F {
+ F();
+ virtual void f() = 0;
+};
+
+F::F() {}
+// CHECK: define x86_thiscallcc %"struct.constructors::F"* @"\01??0F@constructors@@QAE@XZ"
+
} // end namespace constructors
namespace dtors {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits