Merge pull request #59529 from hyp/eng/mut-method-cpp

hyp · web-flow · commit 15bef5e671b7 · 2022-06-17T15:11:34.000-07:00
[interop][SwiftToCxx] support mutating struct methods in C++
diff --git a/lib/PrintAsClang/DeclAndTypePrinter.cpp b/lib/PrintAsClang/DeclAndTypePrinter.cpp
@@ -908,7 +908,9 @@ class DeclAndTypePrinter::Implementation
     if (selfTypeDeclContext) {
       additionalParams.push_back(
           {DeclAndTypeClangFunctionPrinter::AdditionalParam::Role::Self,
-           (*selfTypeDeclContext)->getDeclaredType()});
+           (*selfTypeDeclContext)->getDeclaredType(),
+           /*isIndirect=*/
+           isa<FuncDecl>(FD) ? cast<FuncDecl>(FD)->isMutating() : false});
     }
     funcPrinter.printFunctionSignature(
         FD, funcABI.getSymbolName(), resultTy,
diff --git a/lib/PrintAsClang/PrintClangFunction.cpp b/lib/PrintAsClang/PrintClangFunction.cpp
@@ -230,7 +230,8 @@ void DeclAndTypeClangFunctionPrinter::printFunctionSignature(
     ClangSyntaxPrinter(os).printBaseName(modifiers.qualifierContext);
     os << "::";
   }
-  os << name << '(';
+  ClangSyntaxPrinter(os).printIdentifier(name);
+  os << '(';
 
   bool HasParams = false;
   // Indirect result is passed in as the first parameter.
@@ -276,8 +277,13 @@ void DeclAndTypeClangFunctionPrinter::printFunctionSignature(
       delegate.prefixIndirectParamValueTypeInC = [](raw_ostream &os) {
         os << "SWIFT_CONTEXT ";
       };
-      print(param.type, OptionalTypeKind::OTK_None, "_self", /*isInOut*/ false,
-            delegate);
+      if (param.isIndirect) {
+        (*delegate.prefixIndirectParamValueTypeInC)(os);
+        os << "void * _Nonnull _self";
+      } else {
+        print(param.type, OptionalTypeKind::OTK_None, "_self",
+              /*isInOut*/ false, delegate);
+      }
     });
   }
   if (kind == FunctionSignatureKind::CFunctionProto && !HasParams) {
@@ -288,14 +294,14 @@ void DeclAndTypeClangFunctionPrinter::printFunctionSignature(
 }
 
 void DeclAndTypeClangFunctionPrinter::printCxxToCFunctionParameterUse(
-    Type type, StringRef name, bool isInOut,
+    Type type, StringRef name, bool isInOut, bool isIndirect,
     llvm::Optional<AdditionalParam::Role> paramRole) {
   auto namePrinter = [&]() { ClangSyntaxPrinter(os).printIdentifier(name); };
   if (!isKnownCxxType(type, typeMapping)) {
     if (auto *structDecl = type->getStructOrBoundGenericStruct()) {
       ClangValueTypePrinter(os, cPrologueOS, typeMapping, interopContext)
           .printParameterCxxToCUseScaffold(
-              structDecl->isResilient() ||
+              isIndirect || structDecl->isResilient() ||
                   interopContext.getIrABIDetails().shouldPassIndirectly(type),
               structDecl, namePrinter, isInOut,
               /*isSelf=*/paramRole &&
@@ -331,6 +337,7 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
     if (params->size()) {
       if (hasParams)
         os << ", ";
+      hasParams = true;
       size_t index = 1;
       interleaveComma(*params, os, [&](const ParamDecl *param) {
         if (param->hasName()) {
@@ -351,6 +358,7 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
       interleaveComma(additionalParams, os, [&](const AdditionalParam &param) {
         assert(param.role == AdditionalParam::Role::Self);
         printCxxToCFunctionParameterUse(param.type, "*this", /*isInOut=*/false,
+                                        /*isIndirect=*/param.isIndirect,
                                         param.role);
       });
     }
@@ -396,18 +404,24 @@ void DeclAndTypeClangFunctionPrinter::printCxxMethod(
     modifiers.qualifierContext = typeDeclContext;
   printFunctionSignature(FD, FD->getName().getBaseIdentifier().get(), resultTy,
                          FunctionSignatureKind::CxxInlineThunk, {}, modifiers);
-
-  // FIXME: Add support for mutating methods.
-  os << " const";
+  bool isMutating = false;
+  if (auto *funcDecl = dyn_cast<FuncDecl>(FD))
+    isMutating = funcDecl->isMutating();
+  if (!isMutating)
+    os << " const";
   if (!isDefinition) {
     os << ";\n";
     return;
   }
+
   os << " {\n";
   // FIXME: should it be objTy for resultTy?
   printCxxThunkBody(swiftSymbolName, resultTy, FD->getParameters(),
-                    {AdditionalParam{AdditionalParam::Role::Self,
-                                     typeDeclContext->getDeclaredType()}});
+                    {AdditionalParam{
+                        AdditionalParam::Role::Self,
+                        typeDeclContext->getDeclaredType(),
+                        /*isIndirect=*/isMutating,
+                    }});
   os << "  }\n";
 }
 
@@ -450,6 +464,7 @@ void DeclAndTypeClangFunctionPrinter::printCxxPropertyAccessorMethod(
   // FIXME: should it be objTy for resultTy?
   printCxxThunkBody(swiftSymbolName, resultTy, accessor->getParameters(),
                     {AdditionalParam{AdditionalParam::Role::Self,
-                                     typeDeclContext->getDeclaredType()}});
+                                     typeDeclContext->getDeclaredType(),
+                                     /*isIndirect=*/false}});
   os << "  }\n";
 }
diff --git a/lib/PrintAsClang/PrintClangFunction.h b/lib/PrintAsClang/PrintClangFunction.h
@@ -56,6 +56,8 @@ class DeclAndTypeClangFunctionPrinter {
 
     Role role;
     Type type;
+    // Should self be passed indirectly?
+    bool isIndirect = false;
   };
 
   /// Optional modifiers that can be applied to function signature.
@@ -96,7 +98,7 @@ class DeclAndTypeClangFunctionPrinter {
 
 private:
   void printCxxToCFunctionParameterUse(
-      Type type, StringRef name, bool isInOut,
+      Type type, StringRef name, bool isInOut, bool isIndirect = false,
       llvm::Optional<AdditionalParam::Role> paramRole = None);
 
   raw_ostream &os;
diff --git a/test/Interop/SwiftToCxx/methods/mutating-method-in-cxx-execution.cpp b/test/Interop/SwiftToCxx/methods/mutating-method-in-cxx-execution.cpp
@@ -0,0 +1,45 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend %S/mutating-method-in-cxx.swift -typecheck -module-name Methods -clang-header-expose-public-decls -emit-clang-header-path %t/methods.h
+
+// RUN: %target-interop-build-clangxx -c %s -I %t -o %t/swift-methods-execution.o
+// RUN: %target-interop-build-swift %S/mutating-method-in-cxx.swift -o %t/swift-methods-execution -Xlinker %t/swift-methods-execution.o -module-name Methods -Xfrontend -entry-point-function-name -Xfrontend swiftMain
+
+// RUN: %target-codesign %t/swift-methods-execution
+// RUN: %target-run %t/swift-methods-execution | %FileCheck %s
+
+// REQUIRES: executable_test
+
+#include <assert.h>
+#include "methods.h"
+
+int main() {
+  using namespace Methods;
+
+  auto smallStruct = createSmallStruct(10.0f);
+  smallStruct.dump();
+// CHECK: small x = 10.0;
+
+  smallStruct.scale(0.25f).dump();
+  smallStruct.dump();
+// CHECK-NEXT: small x = 10.0;
+// CHECK-NEXT: small x = 2.5;
+
+  smallStruct.invert();
+  smallStruct.dump();
+// CHECK-NEXT: small x = -2.5;
+
+  auto largeStruct = createLargeStruct();
+  largeStruct.dump();
+// CHECK-NEXT: 1, -5, 9, 11, 48879, -77
+
+  largeStruct.double_();
+  largeStruct.dump();
+// CHECK-NEXT: 2, -10, 18, 22, 97758, -154
+
+  largeStruct.scale(-3, 10).dump();
+  largeStruct.dump();
+// CHECK-NEXT: -6, -100, -54, 220, -293274, -1540
+// CHECK-NEXT: -6, -100, -54, 220, -293274, -1540
+  return 0;
+}
diff --git a/test/Interop/SwiftToCxx/methods/mutating-method-in-cxx.swift b/test/Interop/SwiftToCxx/methods/mutating-method-in-cxx.swift
@@ -0,0 +1,104 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend %s -typecheck -module-name Methods -clang-header-expose-public-decls -emit-clang-header-path %t/methods.h
+// RUN: %FileCheck %s < %t/methods.h
+
+// RUN: %check-interop-cxx-header-in-clang(%t/methods.h)
+
+public struct LargeStruct {
+    var x1, x2, x3, x4, x5, x6: Int
+
+    public func dump() {
+        print("\(x1), \(x2), \(x3), \(x4), \(x5), \(x6)")
+    }
+
+    public mutating func double() {
+        x1 *= 2
+        x2 *= 2
+        x3 *= 2
+        x4 *= 2
+        x5 *= 2
+        x6 *= 2
+    }
+
+    public mutating func scale(_ x: Int, _ y: Int) -> LargeStruct {
+        x1 *= x
+        x2 *= y
+        x3 *= x
+        x4 *= y
+        x5 *= x
+        x6 *= y
+        return self
+    }
+}
+
+public struct SmallStruct {
+    var x: Float
+
+    public func dump() {
+        print("small x = \(x);")
+    }
+
+    public mutating func scale(_ y: Float) -> SmallStruct {
+        x *= y
+        return SmallStruct(x: x / y)
+    }
+
+    public mutating func invert() {
+        x = -x
+    }
+}
+
+// CHECK: SWIFT_EXTERN void $s7Methods11LargeStructV4dumpyyF(SWIFT_CONTEXT const void * _Nonnull _self) SWIFT_NOEXCEPT SWIFT_CALL; // dump()
+// CHECK: SWIFT_EXTERN void $s7Methods11LargeStructV6doubleyyF(SWIFT_CONTEXT void * _Nonnull _self) SWIFT_NOEXCEPT SWIFT_CALL; // double()
+// CHECK: SWIFT_EXTERN void $s7Methods11LargeStructV5scaleyACSi_SitF(SWIFT_INDIRECT_RESULT void * _Nonnull, ptrdiff_t x, ptrdiff_t y, SWIFT_CONTEXT void * _Nonnull _self) SWIFT_NOEXCEPT SWIFT_CALL; // scale(_:_:)
+
+// CHECK: SWIFT_EXTERN void $s7Methods11SmallStructV4dumpyyF(struct swift_interop_stub_Methods_SmallStruct _self) SWIFT_NOEXCEPT SWIFT_CALL; // dump()
+// CHECK: SWIFT_EXTERN struct swift_interop_stub_Methods_SmallStruct $s7Methods11SmallStructV5scaleyACSfF(float y, SWIFT_CONTEXT void * _Nonnull _self) SWIFT_NOEXCEPT SWIFT_CALL; // scale(_:)
+// CHECK: SWIFT_EXTERN void $s7Methods11SmallStructV6invertyyF(SWIFT_CONTEXT void * _Nonnull _self) SWIFT_NOEXCEPT SWIFT_CALL; // invert()
+
+// CHECK: class LargeStruct final {
+// CHECK: inline LargeStruct(LargeStruct &&) = default;
+// CHECK-NEXT:   inline void dump() const;
+// CHECK-NEXT:   inline void double_();
+// CHECK-NEXT:   inline LargeStruct scale(swift::Int x, swift::Int y);
+// CHECK-NEXT: private
+
+// CHECK:        inline void LargeStruct::dump() const {
+// CHECK-NEXT:   return _impl::$s7Methods11LargeStructV4dumpyyF(_getOpaquePointer());
+// CHECK-NEXT:   }
+// CHECK-NEXT:   inline void LargeStruct::double_() {
+// CHECK-NEXT:   return _impl::$s7Methods11LargeStructV6doubleyyF(_getOpaquePointer());
+// CHECK-NEXT:   }
+// CHECK-NEXT:   inline LargeStruct LargeStruct::scale(swift::Int x, swift::Int y) {
+// CHECK-NEXT:   return _impl::_impl_LargeStruct::returnNewValue([&](void * _Nonnull result) {
+// CHECK-NEXT:     _impl::$s7Methods11LargeStructV5scaleyACSi_SitF(result, x, y, _getOpaquePointer());
+// CHECK-NEXT:   });
+// CHECK-NEXT:   }
+
+// CHECK: class SmallStruct final {
+// CHECK:   inline SmallStruct(SmallStruct &&) = default;
+// CHECK-NEXT:   inline void dump() const;
+// CHECK-NEXT:   inline SmallStruct scale(float y);
+// CHECK-NEXT:   inline void invert();
+// CHECK-NEXT: private:
+
+// CHECK:        inline void SmallStruct::dump() const {
+// CHECK-NEXT:   return _impl::$s7Methods11SmallStructV4dumpyyF(_impl::swift_interop_passDirect_Methods_SmallStruct(_getOpaquePointer()));
+// CHECK-NEXT:   }
+// CHECK-NEXT:   inline SmallStruct SmallStruct::scale(float y) {
+// CHECK-NEXT:   return _impl::_impl_SmallStruct::returnNewValue([&](char * _Nonnull result) {
+// CHECK-NEXT:     _impl::swift_interop_returnDirect_Methods_SmallStruct(result, _impl::$s7Methods11SmallStructV5scaleyACSfF(y, _getOpaquePointer()));
+// CHECK-NEXT:   });
+// CHECK-NEXT:   }
+// CHECK-NEXT:   inline void SmallStruct::invert() {
+// CHECK-NEXT:   return _impl::$s7Methods11SmallStructV6invertyyF(_getOpaquePointer());
+// CHECK-NEXT:   }
+
+
+public func createLargeStruct() -> LargeStruct {
+    return LargeStruct(x1: 1, x2: -5, x3: 9, x4: 11, x5: 0xbeef, x6: -77)
+}
+
+public func createSmallStruct(x: Float) -> SmallStruct {
+    return SmallStruct(x: x)
+}
