[PATCH] Fix PR16735 - emit the constructor for abstract classes

[Timur Iskhodzhanov]

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 at constructors@@QAE at XZ"
+
 } // end namespace constructors
 
 namespace dtors {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1248.1.patch
Type: text/x-patch
Size: 5675 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130731/6deeb02d/attachment.bin>