[cxx-interop] std::string::push_back is broken (#41240)

* [cxx-interop] fix  std::string::push_back by fixing mapping between clang/swift self/this type for cxx methods

* cleanup

* Fix unit test

* XFail test: operators/member-inline on linux-android

* add `C++` in expandExternalSignatureTypes comment

* Fix rebase

* Fix mapping issue in externalizeArguments

* Improve cxx_interop_ir test regex

Author: Huddie

lib/IRGen/GenCall.cpp

@@ -1278,7 +1278,7 @@ static bool doesClangExpansionMatchSchema(IRGenModule &IGM,
 }
 
 /// Expand the result and parameter types to the appropriate LLVM IR
-/// types for C and Objective-C signatures.
+/// types for C, C++ and Objective-C signatures.
 void SignatureExpansion::expandExternalSignatureTypes() {
   assert(FnType->getLanguage() == SILFunctionLanguage::C);
 
@@ -1319,7 +1319,15 @@ void SignatureExpansion::expandExternalSignatureTypes() {
     paramTys.push_back(clangCtx.VoidPtrTy);
     break;
 
-  case SILFunctionTypeRepresentation::CXXMethod:
+  case SILFunctionTypeRepresentation::CXXMethod: {
+    // Cxx methods take their 'self' argument first.
+    auto &self = params.back();
+    auto clangTy = IGM.getClangType(self, FnType);
+    paramTys.push_back(clangTy);
+    params = params.drop_back();
+    break;
+  }
+
   case SILFunctionTypeRepresentation::CFunctionPointer:
     // No implicit arguments.
     break;
@@ -1437,9 +1445,14 @@ void SignatureExpansion::expandExternalSignatureTypes() {
     case clang::CodeGen::ABIArgInfo::IndirectAliased:
       llvm_unreachable("not implemented");
     case clang::CodeGen::ABIArgInfo::Indirect: {
-      assert(i >= clangToSwiftParamOffset &&
+      // When `i` is 0, if the clang offset is 1, that means we mapped the last
+      // Swift parameter (self) to the first Clang parameter (this). In this
+      // case, the corresponding Swift param is the last function parameter.
+      assert((i >= clangToSwiftParamOffset || clangToSwiftParamOffset == 1) &&
              "Unexpected index for indirect byval argument");
-      auto &param = params[i - clangToSwiftParamOffset];
+      auto &param = i < clangToSwiftParamOffset
+                        ? FnType->getParameters().back()
+                        : params[i - clangToSwiftParamOffset];
       auto paramTy = getSILFuncConventions().getSILType(
           param, IGM.getMaximalTypeExpansionContext());
       auto &paramTI = cast<FixedTypeInfo>(IGM.getTypeInfo(paramTy));
@@ -3544,7 +3557,8 @@ static void externalizeArguments(IRGenFunction &IGF, const Callee &callee,
   } else if (callee.getRepresentation() ==
              SILFunctionTypeRepresentation::CXXMethod) {
     // Skip the "self" param.
-    paramEnd--;
+    firstParam += 1;
+    params = params.drop_back();
   }
 
   if (fnType->getNumResults() > 0 &&

test/ClangImporter/cxx_interop_ir.swift

@@ -36,7 +36,7 @@ func accessNSMember() {
 // CHECK-LABEL: define hidden swiftcc i32 @"$s6cxx_ir12basicMethods1as5Int32VSpySo0D0VG_tF"(i8* %0)
 // CHECK: [[THIS_PTR1:%.*]] = bitcast i8* %0 to %TSo7MethodsV*
 // CHECK: [[THIS_PTR2:%.*]] = bitcast %TSo7MethodsV* [[THIS_PTR1]] to %class.Methods*
-// CHECK: [[RESULT:%.*]] = call {{(signext )?}}i32 @{{_ZN7Methods12SimpleMethodEi|"\?SimpleMethod@Methods@@QEAAHH@Z"}}(%class.Methods* [[THIS_PTR2]], i32{{( signext)?}} 4)
+// CHECK: [[RESULT:%.*]] = call {{(signext )?}}i32 @{{_ZN7Methods12SimpleMethodEi|"\?SimpleMethod@Methods@@QEAAHH@Z"}}(%class.Methods* [[THIS_PTR2]], i32 {{%?[0-9]+}})
 // CHECK: ret i32 [[RESULT]]
 func basicMethods(a: UnsafeMutablePointer<Methods>) -> Int32 {
   return a.pointee.SimpleMethod(4)
@@ -46,7 +46,7 @@ func basicMethods(a: UnsafeMutablePointer<Methods>) -> Int32 {
 // CHECK: [[THIS_PTR1:%.*]] = alloca %TSo7MethodsV, align {{4|8}}
 // CHECK: [[THIS_PTR2:%.*]] = bitcast %TSo7MethodsV* [[THIS_PTR1]] to %class.Methods*
 // CHECK: [[THIS_PTR3:%.*]] = bitcast %TSo7MethodsV* [[THIS_PTR1]] to %class.Methods*
-// CHECK: [[RESULT:%.*]] = call {{(signext )?}}i32 @{{_ZNK7Methods17SimpleConstMethodEi|"\?SimpleConstMethod@Methods@@QEBAHH@Z"}}(%class.Methods* [[THIS_PTR3]], i32{{( signext)?}} 3)
+// CHECK: [[RESULT:%.*]] = call {{(signext )?}}i32 @{{_ZNK7Methods17SimpleConstMethodEi|"\?SimpleConstMethod@Methods@@QEBAHH@Z"}}(%class.Methods* [[THIS_PTR3]], i32 {{%?[0-9]+}})
 // CHECK: ret i32 [[RESULT]]
 func basicMethodsConst(a: UnsafeMutablePointer<Methods>) -> Int32 {
   return a.pointee.SimpleConstMethod(3)
@@ -62,7 +62,7 @@ func basicMethodsStatic() -> Int32 {
 // CHECK-LABEL: define hidden swiftcc i32 @"$s6cxx_ir12basicMethods1as5Int32VSpySo8Methods2VG_tF"(i8* %0)
 // CHECK: [[THIS_PTR1:%.*]] = bitcast i8* %0 to %TSo8Methods2V*
 // CHECK: [[THIS_PTR2:%.*]] = bitcast %TSo8Methods2V* [[THIS_PTR1]] to %class.Methods2*
-// CHECK: [[RESULT:%.*]] = call {{(signext )?}}i32 @{{_ZN8Methods212SimpleMethodEi|"\?SimpleMethod@Methods2@@QEAAHH@Z"}}(%class.Methods2* [[THIS_PTR2]], i32{{( signext)?}} 4)
+// CHECK: [[RESULT:%.*]] = call {{(signext )?}}i32 @{{_ZN8Methods212SimpleMethodEi|"\?SimpleMethod@Methods2@@QEAAHH@Z"}}(%class.Methods2* [[THIS_PTR2]], i32 {{%?[0-9]+}})
 // CHECK: ret i32 [[RESULT]]
 func basicMethods(a: UnsafeMutablePointer<Methods2>) -> Int32 {
   return a.pointee.SimpleMethod(4)

test/Interop/Cxx/operators/member-inline.swift

@@ -1,6 +1,7 @@
 // RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -enable-cxx-interop)
 
 // REQUIRES: executable_test
+// XFAIL: OS=linux-android
 
 import MemberInline
 import StdlibUnittest

test/Interop/Cxx/stdlib/use-std-string.swift

@@ -17,13 +17,12 @@ StdStringTestSuite.test("init") {
     expectTrue(s.empty())
 }
 
-// LLVM module verification fails for calls to std::string::push_back: rdar://88343327
-// StdStringTestSuite.test("push back") {
-//     var s = CxxString()
-//     s.push_back(42)
-//     expectEqual(s.size(), 1)
-//     expectFalse(s.empty())
-//     expectEqual(s[0], 42)
-// }
+StdStringTestSuite.test("push back") {
+    var s = CxxString()
+    s.push_back(42)
+    expectEqual(s.size(), 1)
+    expectFalse(s.empty())
+    expectEqual(s[0], 42)
+}
 
 runAllTests()