Add a constructor thunk if required to add additional constructor

martinboehme · zoecarver · commit b2c5a3eeed4e · 2020-10-09T10:42:48.000-07:00
arguments.

Also add more IR tests and make various other changes.
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -3439,14 +3439,19 @@ namespace {
 
       bool hasReferenceableFields = !members.empty();
 
-      if (hasZeroInitializableStorage &&
-          !Impl.SwiftContext.LangOpts.EnableCXXInterop) {
-        // Add constructors for the struct.
+      const clang::CXXRecordDecl *cxxRecordDecl =
+          dyn_cast<clang::CXXRecordDecl>(decl);
+      if (hasZeroInitializableStorage && !cxxRecordDecl) {
+        // Add default constructor for the struct if compiling in C mode.
+        // If we're compiling for C++, we'll import the C++ default constructor
+        // (if there is one), so we don't need to synthesize one here.
         ctors.push_back(createDefaultConstructor(Impl, result));
       }
 
+      bool hasUserDeclaredConstructor =
+          cxxRecordDecl && cxxRecordDecl->hasUserDeclaredConstructor();
       if (hasReferenceableFields && hasMemberwiseInitializer &&
-          !Impl.SwiftContext.LangOpts.EnableCXXInterop) {
+          !hasUserDeclaredConstructor) {
         // The default zero initializer suppresses the implicit value
         // constructor that would normally be formed, so we have to add that
         // explicitly as well.
@@ -3893,7 +3898,6 @@ namespace {
 
       AbstractFunctionDecl *result = nullptr;
       if (auto *ctordecl = dyn_cast<clang::CXXConstructorDecl>(decl)) {
-        // TODO: Is failable, throws etc. correct?
         DeclName ctorName(Impl.SwiftContext, DeclBaseName::createConstructor(),
                           bodyParams);
         result = Impl.createDeclWithClangNode<ConstructorDecl>(
diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp
@@ -430,7 +430,7 @@ namespace {
 
   private:
     void expand(SILParameterInfo param);
-    llvm::Type *addIndirectResult(bool noCapture = true);
+    llvm::Type *addIndirectResult();
 
     SILFunctionConventions getSILFuncConventions() const {
       return SILFunctionConventions(FnType, IGM.getSILModule());
@@ -484,8 +484,7 @@ llvm::Type *SignatureExpansion::addIndirectResult() {
   auto resultType = getSILFuncConventions().getSILResultType(
       IGM.getMaximalTypeExpansionContext());
   const TypeInfo &resultTI = IGM.getTypeInfo(resultType);
-  addIndirectResultAttributes(IGM, Attrs, ParamIRTypes.size(), claimSRet(),
-                              noCapture);
+  addIndirectResultAttributes(IGM, Attrs, ParamIRTypes.size(), claimSRet());
   addPointerParameter(resultTI.getStorageType());
   return IGM.VoidTy;
 }
@@ -1272,6 +1271,16 @@ void SignatureExpansion::expandExternalSignatureTypes() {
   SmallVector<clang::CanQualType,4> paramTys;
   auto const &clangCtx = IGM.getClangASTContext();
 
+  bool formalIndirectResult = FnType->getNumResults() > 0 &&
+                              FnType->getSingleResult().isFormalIndirect();
+  if (formalIndirectResult) {
+    auto resultType = getSILFuncConventions().getSingleSILResultType(
+        IGM.getMaximalTypeExpansionContext());
+    auto clangTy =
+        IGM.getClangASTContext().getPointerType(IGM.getClangType(resultType));
+    paramTys.push_back(clangTy);
+  }
+
   switch (FnType->getRepresentation()) {
   case SILFunctionTypeRepresentation::ObjCMethod: {
     // ObjC methods take their 'self' argument first, followed by an
@@ -1336,17 +1345,8 @@ void SignatureExpansion::expandExternalSignatureTypes() {
   }
 
   // If we return indirectly, that is the first parameter type.
-  bool formalIndirect = FnType->getNumResults() > 0 &&
-                        FnType->getSingleResult().isFormalIndirect();
-  if (formalIndirect || returnInfo.isIndirect()) {
-    // Specify "nocapture" if we're returning the result indirectly for
-    // low-level ABI reasons, as the called function never sees the implicit
-    // output parameter.
-    // On the other hand, if the result is a formal indirect result in SIL, that
-    // means that the Clang function has an explicit output parameter (e.g. it's
-    // a C++ constructor), which it might capture -- so don't specify
-    // "nocapture" in that case.
-    addIndirectResult(/* noCapture = */ returnInfo.isIndirect());
+  if (returnInfo.isIndirect()) {
+    addIndirectResult();
   }
 
   size_t firstParamToLowerNormally = 0;
@@ -1432,6 +1432,13 @@ void SignatureExpansion::expandExternalSignatureTypes() {
     }
   }
 
+  if (formalIndirectResult) {
+    // If the result is a formal indirect result in SIL, that means that the
+    // Clang function has an explicit output parameter (e.g. it's a C++
+    // constructor), which it might capture -- so don't specify "nocapture".
+    addIndirectResultAttributes(IGM, Attrs, 0, claimSRet(), /* nocapture = */ false);
+  }
+
   if (returnInfo.isIndirect() || returnInfo.isIgnore()) {
     ResultIRType = IGM.VoidTy;
   } else {
@@ -2812,6 +2819,11 @@ static void externalizeArguments(IRGenFunction &IGF, const Callee &callee,
                 == SILFunctionTypeRepresentation::Block) {
     // Ignore the physical block-object parameter.
     firstParam += 1;
+  // Or the indirect result parameter.
+  } else if (fnType->getNumResults() > 0 &&
+             fnType->getSingleResult().isFormalIndirect()) {
+    // Ignore the indirect result parameter.
+    firstParam += 1;
   }
 
   for (unsigned i = firstParam, e = FI.arg_size(); i != e; ++i) {
diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp
@@ -38,6 +38,8 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/GlobalDecl.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
+#include "clang/CodeGen/ModuleBuilder.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/GlobalAlias.h"
@@ -2761,6 +2763,77 @@ void IRGenModule::emitDynamicReplacementOriginalFunctionThunk(SILFunction *f) {
     IGF.Builder.CreateRet(Res);
 }
 
+/// If the calling convention for `ctor` doesn't match the calling convention
+/// that we assumed for it when we imported it as `initializer`, emit and
+/// return a thunk that conforms to the assumed calling convention. The thunk
+/// is marked `alwaysinline`, so it doesn't generate any runtime overhead.
+/// If the assumed calling convention was correct, just return `ctor`.
+///
+/// See also comments in CXXMethodConventions in SIL/IR/SILFunctionType.cpp.
+static llvm::Constant *
+emitCXXConstructorThunkIfNeeded(IRGenModule &IGM, SILFunction *initializer,
+                                const clang::CXXConstructorDecl *ctor,
+                                const LinkEntity &entity,
+                                llvm::Constant *ctorAddress) {
+  Signature signature = IGM.getSignature(initializer->getLoweredFunctionType());
+
+  llvm::FunctionType *assumedFnType = signature.getType();
+  llvm::FunctionType *ctorFnType =
+      cast<llvm::FunctionType>(ctorAddress->getType()->getPointerElementType());
+
+  if (assumedFnType == ctorFnType) {
+    return ctorAddress;
+  }
+
+  // The thunk has private linkage, so it doesn't need to have a predictable
+  // mangled name -- we just need to make sure the name is unique.
+  llvm::SmallString<32> name;
+  llvm::raw_svector_ostream stream(name);
+  stream << "__swift_cxx_ctor";
+  entity.mangle(stream);
+
+  llvm::Function *thunk = llvm::Function::Create(
+      assumedFnType, llvm::Function::PrivateLinkage, name, &IGM.Module);
+
+  thunk->setCallingConv(llvm::CallingConv::C);
+
+  llvm::AttrBuilder attrBuilder;
+  IGM.constructInitialFnAttributes(attrBuilder);
+  attrBuilder.addAttribute(llvm::Attribute::AlwaysInline);
+  llvm::AttributeList attr = signature.getAttributes().addAttributes(
+      IGM.getLLVMContext(), llvm::AttributeList::FunctionIndex, attrBuilder);
+  thunk->setAttributes(attr);
+
+  IRGenFunction subIGF(IGM, thunk);
+  if (IGM.DebugInfo)
+    IGM.DebugInfo->emitArtificialFunction(subIGF, thunk);
+
+  SmallVector<llvm::Value *, 8> Args;
+  for (auto i = thunk->arg_begin(), e = thunk->arg_end(); i != e; ++i) {
+    auto *argTy = i->getType();
+    auto *paramTy = ctorFnType->getParamType(i - thunk->arg_begin());
+    if (paramTy != argTy)
+      Args.push_back(subIGF.coerceValue(i, paramTy, IGM.DataLayout));
+    else
+      Args.push_back(i);
+  }
+
+  clang::CodeGen::ImplicitCXXConstructorArgs implicitArgs =
+      clang::CodeGen::getImplicitCXXConstructorArgs(IGM.ClangCodeGen->CGM(),
+                                                    ctor);
+  for (size_t i = 0; i < implicitArgs.Prefix.size(); ++i) {
+    Args.insert(Args.begin() + 1 + i, implicitArgs.Prefix[i]);
+  }
+  for (const auto &arg : implicitArgs.Suffix) {
+    Args.push_back(arg);
+  }
+
+  subIGF.Builder.CreateCall(ctorFnType, ctorAddress, Args);
+  subIGF.Builder.CreateRetVoid();
+
+  return thunk;
+}
+
 /// Find the entry point for a SIL function.
 llvm::Function *IRGenModule::getAddrOfSILFunction(
     SILFunction *f, ForDefinition_t forDefinition,
@@ -2789,6 +2862,11 @@ llvm::Function *IRGenModule::getAddrOfSILFunction(
   if (auto clangDecl = f->getClangDecl()) {
     auto globalDecl = getClangGlobalDeclForFunction(clangDecl);
     clangAddr = getAddrOfClangGlobalDecl(globalDecl, forDefinition);
+
+    if (auto ctor = dyn_cast<clang::CXXConstructorDecl>(clangDecl)) {
+      clangAddr =
+          emitCXXConstructorThunkIfNeeded(*this, f, ctor, entity, clangAddr);
+    }
   }
 
   bool isDefinition = f->isDefinition();
diff --git a/test/Interop/Cxx/class/Inputs/cxx-constructors.h b/test/Interop/Cxx/class/Inputs/cxx-constructors.h
@@ -17,6 +17,18 @@ struct DefaultConstructorDeleted {
 };
 
 struct ConstructorWithParam {
-  ConstructorWithParam(int val) : x(val) {}
+  ConstructorWithParam(int val) : x(val + 42) {}
   int x;
 };
+
+struct Base {};
+
+struct ArgType {
+  int i = 42;
+};
+
+struct HasVirtualBase : public virtual Base {
+  HasVirtualBase() = delete;
+  HasVirtualBase(ArgType Arg) {}
+  int i;
+};
diff --git a/test/Interop/Cxx/class/cxx-constructors-executable.swift b/test/Interop/Cxx/class/cxx-constructors-executable.swift
@@ -1,7 +1,4 @@
-// RUN: %empty-directory(%t)
-// RUN: %target-build-swift %s -I %S/Inputs/ -o %t/cxx_interop -Xfrontend -enable-cxx-interop
-// RUN: %target-codesign %t/cxx_interop
-// RUN: %target-run %t/cxx_interop
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-cxx-interop)
 //
 // REQUIRES: executable_test
 
@@ -29,9 +26,9 @@ CxxConstructorTestSuite.test("MemberOfClassType") {
 }
 
 CxxConstructorTestSuite.test("ConstructorWithParam") {
-  let instance = ConstructorWithParam(123456)
+  let instance = ConstructorWithParam(2)
 
-  expectEqual(123456, instance.x)
+  expectEqual(44, instance.x)
 }
 
 runAllTests()
diff --git a/test/Interop/Cxx/class/cxx-constructors-ir.swift b/test/Interop/Cxx/class/cxx-constructors-ir.swift
@@ -1,14 +1,42 @@
-// RUN: %target-swift-frontend -I %S/Inputs -enable-cxx-interop -emit-ir %s | %FileCheck %s
+// Target-specific tests for C++ constructor call code generation.
+
+// RUN: %swift -module-name Swift -target x86_64-apple-macosx10.9 -dump-clang-diagnostics -I %S/Inputs -enable-cxx-interop -emit-ir %s -parse-stdlib -parse-as-library -disable-legacy-type-info | %FileCheck %s -check-prefix=ITANIUM_X64
+// RUN: %swift -module-name Swift -target armv7-none-linux-androideabi -dump-clang-diagnostics -I %S/Inputs -enable-cxx-interop -emit-ir %s -parse-stdlib -parse-as-library -disable-legacy-type-info | %FileCheck %s -check-prefix=ITANIUM_ARM
+// RUN: %swift -module-name Swift -target x86_64-unknown-windows-msvc -dump-clang-diagnostics -I %S/Inputs -enable-cxx-interop -emit-ir %s -parse-stdlib -parse-as-library -disable-legacy-type-info | %FileCheck %s -check-prefix=MICROSOFT_X64
 
 import CxxConstructors
 
-// Note:
-// - The `this` parameter should carry a `noalias` attribute, as it is
-//   guaranteed that nothing will alias the object before it has been fully
-//   constructed. It should also carry an `sret` attribute to indicate that this
-//   is an out parameter for a structure being returned by the function.
-// - The `this` parameter should _not_ carry a `nocapture` attribute (unlike
-//   Swift constructors that return their result indirectly) because the C++ has
-//   explicit access to `this` and may capture it.
-// CHECK: call void @_ZN20ConstructorWithParamC2Ei(%struct.ConstructorWithParam* noalias sret %{{[0-9]+}}, i32 42)
-let _ = ConstructorWithParam(42)
+typealias Void = ()
+
+public func createHasVirtualBase() -> HasVirtualBase {
+  // - The `this` parameter should carry a `noalias` attribute, as it is
+  //   guaranteed that nothing will alias the object before it has been fully
+  //   constructed. It should also carry an `sret` attribute to indicate that
+  //   this is an out parameter for a structure being returned by the function.
+  //   Note that this doesn't apply on ABIs (Itanium ARM, Microsoft x64)
+  //   where we insert an (inlined) thunk; in this case, we're getting the
+  //   attributes of the constructor that was generated by Clang, which doesn't
+  //   insert these attributes.
+  //
+  // - The `this` parameter should _not_ carry a `nocapture` attribute (unlike
+  //   Swift constructors that return their result indirectly) because the C++
+  //   constructor has explicit access to `this` and may capture it.
+  //
+  // ITANIUM_X64: define swiftcc { i8*, i32 } @"$ss20createHasVirtualBaseSo0bcD0VyF"()
+  // ITANIUM_X64-NOT: define
+  // ITANIUM_X64: call void @_ZN14HasVirtualBaseC1E7ArgType(%struct.HasVirtualBase* noalias sret %{{[0-9]+}}, i32 %{{[0-9]+}})
+  //
+  // ITANIUM_ARM: define protected swiftcc { i8*, i32 } @"$ss20createHasVirtualBaseSo0bcD0VyF"()
+  // To verify that the thunk is inlined, make sure there's no intervening
+  // `define`, i.e. the call to the C++ constructor happens in
+  // createHasVirtualBase(), not some later function.
+  // ITANIUM_ARM-NOT: define
+  // Note `this` return type.
+  // ITANIUM_ARM: call %struct.HasVirtualBase* @_ZN14HasVirtualBaseC1E7ArgType(%struct.HasVirtualBase* %{{[0-9]+}}, [1 x i32] %{{[0-9]+}})
+  //
+  // MICROSOFT_X64: define dllexport swiftcc { i8*, i32 } @"$ss20createHasVirtualBaseSo0bcD0VyF"()
+  // MICROSOFT_X64-NOT: define
+  // Note `this` return type and implicit "most derived" argument.
+  // MICROSOFT_X64: call %struct.HasVirtualBase* @"??0HasVirtualBase@@QEAA@UArgType@@@Z"(%struct.HasVirtualBase* %{{[0-9]+}}, i32 %{{[0-9]+}}, i32 1)
+  return HasVirtualBase(ArgType())
+}
diff --git a/test/Interop/Cxx/class/cxx-constructors-module-interface.swift b/test/Interop/Cxx/class/cxx-constructors-module-interface.swift
@@ -7,14 +7,28 @@
 // CHECK-NEXT: struct ImplicitDefaultConstructor {
 // CHECK-NEXT:   var x: Int32
 // CHECK-NEXT:   init()
+// CHECK-NEXT:   init(x: Int32)
 // CHECK-NEXT: }
 // CHECK-NEXT: struct MemberOfClassType {
 // CHECK-NEXT:   var member: ImplicitDefaultConstructor
 // CHECK-NEXT:   init()
+// CHECK-NEXT:   init(member: ImplicitDefaultConstructor)
 // CHECK-NEXT: }
 // CHECK-NEXT: struct DefaultConstructorDeleted {
 // CHECK-NEXT: }
 // CHECK-NEXT: struct ConstructorWithParam {
 // CHECK-NEXT:   var x: Int32
 // CHECK-NEXT:   init(_ val: Int32)
 // CHECK-NEXT: }
+// CHECK-NEXT: struct Base {
+// CHECK-NEXT:   init()
+// CHECK-NEXT: }
+// CHECK-NEXT: struct ArgType {
+// CHECK-NEXT:   var i: Int32
+// CHECK-NEXT:   init()
+// CHECK-NEXT:   init(i: Int32)
+// CHECK-NEXT: }
+// CHECK-NEXT: struct HasVirtualBase {
+// CHECK-NEXT:   var i: Int32
+// CHECK-NEXT:   init(_ Arg: ArgType)
+// CHECK-NEXT: }
