[cxx-interop] Use "c function conventions" for static methods and ctors (not "cxx method conventions").

Otherwise we will incorrectly use the "self" type's conventions when lowering the parameters (off-by-one).

Author: zoecarver

lib/SIL/IR/SILFunctionType.cpp

@@ -3005,6 +3005,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.
@@ -3109,15 +3116,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_ : $@convention(c) (@in_guaranteed Int32, @in_guaranteed Int32) -> @out ReferenceParams
+
+// clang name: ReferenceParams::staticMethod
+// CHECK: sil [clang ReferenceParams.staticMethod] @_ZN15ReferenceParams12staticMethodERKiS1_ : $@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()