Merge pull request #63506 from zoecarver/runtime-crash-ref-in-ctor

Author: zoecarver

lib/SIL/IR/SILFunctionType.cpp

@@ -3073,6 +3073,13 @@ class CFunctionConventions : public CFunctionTypeConventions {
   }
 
   ResultConvention getResult(const TypeLowering &tl) const override {
+    // C++ constructors return indirectly.
+    // TODO: this may be different depending on the ABI, so we may have to
+    // check with clang here.
+    if (isa<clang::CXXConstructorDecl>(TheDecl)) {
+      return ResultConvention::Indirect;
+    }
+
     if (isCFTypedef(tl, TheDecl->getReturnType())) {
       // The CF attributes aren't represented in the type, so we need
       // to check them here.
@@ -3177,15 +3184,19 @@ static CanSILFunctionType getSILFunctionTypeForClangDecl(
   }
 
   if (auto method = dyn_cast<clang::CXXMethodDecl>(clangDecl)) {
-    AbstractionPattern origPattern = AbstractionPattern::getCXXMethod(origType, method,
-                                                                      foreignInfo.self);
-    bool isMutating =
-        TC.Context.getClangModuleLoader()->isCXXMethodMutating(method);
-    auto conventions = CXXMethodConventions(method, isMutating);
-    return getSILFunctionType(TC, TypeExpansionContext::minimal(), origPattern,
-                              substInterfaceType, extInfoBuilder, conventions,
-                              foreignInfo, constant, constant, None,
-                              ProtocolConformanceRef());
+    // Static methods and ctors should be lowered like plane functions
+    // (case below).
+    if (!isa<clang::CXXConstructorDecl>(method) || method->isStatic()) {
+      AbstractionPattern origPattern = AbstractionPattern::getCXXMethod(origType, method,
+                                                                        foreignInfo.self);
+      bool isMutating =
+          TC.Context.getClangModuleLoader()->isCXXMethodMutating(method);
+      auto conventions = CXXMethodConventions(method, isMutating);
+      return getSILFunctionType(TC, TypeExpansionContext::minimal(), origPattern,
+                                substInterfaceType, extInfoBuilder, conventions,
+                                foreignInfo, constant, constant, None,
+                                ProtocolConformanceRef());
+    }
   }
 
   if (auto func = dyn_cast<clang::FunctionDecl>(clangDecl)) {

test/Interop/Cxx/class/method/Inputs/methods.h

@@ -27,4 +27,13 @@ struct HasMethods {
   NonTrivialInWrapper constPassThroughAsWrapper(int a) const { return {a}; }
 };
 
+struct ReferenceParams {
+  int a;
+  int b;
+  ReferenceParams(const int &a, const int &b) : a(a), b(b) { }
+  static void staticMethod(const int &a, const int &b) {
+    ReferenceParams t{a, b};
+  }
+};
+
 #endif // TEST_INTEROP_CXX_CLASS_METHOD_METHODS_H

test/Interop/Cxx/class/method/methods-silgen.swift

@@ -0,0 +1,16 @@
+// RUN: %target-swift-emit-silgen -I %S/Inputs -enable-experimental-cxx-interop %s | %FileCheck %s
+
+import Methods
+
+// clang name: ReferenceParams::ReferenceParams
+// CHECK: sil [clang ReferenceParams.init] @{{_ZN15ReferenceParamsC1ERKiS1_|\?\?0ReferenceParams@@QEAA@AEBH0@Z}} : $@convention(c) (@in_guaranteed Int32, @in_guaranteed Int32) -> @out ReferenceParams
+
+// clang name: ReferenceParams::staticMethod
+// CHECK: sil [clang ReferenceParams.staticMethod] @{{_ZN15ReferenceParams12staticMethodERKiS1_|\?staticMethod@ReferenceParams@@SAXAEBH0@Z}} : $@convention(c) (@in_guaranteed Int32, @in_guaranteed Int32) -> ()
+
+public func use() {
+  let a = CInt(42)
+  let b = CInt(42)
+  _ = ReferenceParams(a, b)
+  ReferenceParams.staticMethod(a, b)
+}

test/Interop/Cxx/class/method/methods.swift

@@ -52,4 +52,20 @@ CxxMethodTestSuite.test("(Int) -> NonTrivialInWrapper") {
   expectEqual(42, instance.constPassThroughAsWrapper(42).value)
 }
 
+CxxMethodTestSuite.test("Constructor with ref params") {
+  let a = CInt(42)
+  let b = CInt(11)
+  var instance = ReferenceParams(a, b)
+
+  expectEqual(42, instance.a)
+  expectEqual(11, instance.b)
+}
+
+// Just make sure we don't crash
+CxxMethodTestSuite.test("Static method with ref params") {
+  let a = CInt(42)
+  let b = CInt(11)
+  ReferenceParams.staticMethod(a, b)
+}
+
 runAllTests()

test/Interop/Cxx/reference/const-ref-parameter.swift

@@ -52,9 +52,9 @@ func testFunction() {
 // CHECK-NEXT: apply [[FN4]]
 // CHECK-SAME: : $@convention(objc_method) (@in_guaranteed OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> Int32
 
-// CHECK: [[FN5:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxxC1ERK13OptionsStruct : $@convention(c) (OptionsStruct) -> @out OptionsConsumerCxx
+// CHECK: [[FN5:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxxC1ERK13OptionsStruct : $@convention(c) (@in_guaranteed OptionsStruct) -> @out OptionsConsumerCxx
 // CHECK-NEXT: apply [[FN5]]
-// CHECK-SAME: : $@convention(c) (OptionsStruct) -> @out OptionsConsumerCxx
+// CHECK-SAME: : $@convention(c) (@in_guaranteed OptionsStruct) -> @out OptionsConsumerCxx
 
 // CHECK: [[FN6:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxx12doOtherThingERK13OptionsStruct : $@convention(cxx_method) (@in_guaranteed OptionsStruct, @inout OptionsConsumerCxx) -> Float
 // CHECK-NEXT: apply [[FN6]]

test/Interop/Cxx/reference/reference-silgen-cxx-objc-ctors+init.swift

@@ -8,8 +8,8 @@ var b = IntWrapper(a)
 var c = ObjCSwiftBridge(embedded: b)
 
 // FIXME: the const-ref C++ Consructor here is not getting an @in_guaranteed or even an @in convention here.
-// CHECK: {{%[0-9]+}} = function_ref @_ZN10IntWrapperC1ERKi : $@convention(c) (Int32) -> @out IntWrapper
-// CHECK: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(c) (Int32) -> @out IntWrapper
+// CHECK: {{%[0-9]+}} = function_ref @_ZN10IntWrapperC1ERKi : $@convention(c) (@in_guaranteed Int32) -> @out IntWrapper
+// CHECK: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(c) (@in_guaranteed Int32) -> @out IntWrapper
 // CHECK: alloc_global @$s4main1cSo15ObjCSwiftBridgeCSgvp
 // CHECK: {{%[0-9]+}} = global_addr @$s4main1cSo15ObjCSwiftBridgeCSgvp : $*Optional<ObjCSwiftBridge>
 // CHECK: {{%[0-9]+}} = load {{%[0-9]+}} : $*IntWrapper