Merge pull request #60359 from hyp/eng/i/class3

hyp · web-flow · commit 09dcbdecb118 · 2022-08-03T07:16:20.000+01:00
[interop][SwiftToCxx] copy construct &amp; assign should do ARC operations for class reference values
diff --git a/lib/PrintAsClang/PrintClangClassType.cpp b/lib/PrintAsClang/PrintClangClassType.cpp
@@ -31,32 +31,24 @@ void ClangClassTypePrinter::printClassTypeDecl(
                            os << ";\n";
                          });
 
+  StringRef baseClassName = "RefCountedClass";
+  StringRef baseClassQualifiedName = "swift::_impl::RefCountedClass";
+
   os << "class ";
   printer.printBaseName(typeDecl);
   // FIXME: Add support for inherintance.
-  os << " final";
+  os << " final : public " << baseClassQualifiedName;
   os << " {\n";
   os << "public:\n";
 
-  // Destructor releases the object.
-  os << "  inline ~";
-  printer.printBaseName(typeDecl);
-  os << "() { swift::" << cxx_synthesis::getCxxImplNamespaceName()
-     << "::swift_release(_opaquePointer); }\n";
-
-  // FIXME: move semantics should be restricted?
-  os << "  inline ";
-  printer.printBaseName(typeDecl);
-  os << "(";
-  printer.printBaseName(typeDecl);
-  os << "&&) noexcept = default;\n";
+  os << "  using " << baseClassName << "::" << baseClassName << ";\n";
+  os << "  using " << baseClassName << "::operator=;\n";
 
   os << "private:\n";
   os << "  inline ";
   printer.printBaseName(typeDecl);
-  os << "(void * _Nonnull ptr) noexcept : _opaquePointer(ptr) {}\n";
-  os << "\n  void * _Nonnull _opaquePointer;\n";
-  os << "  friend class " << cxx_synthesis::getCxxImplNamespaceName() << "::";
+  os << "(void * _Nonnull ptr) noexcept : " << baseClassName << "(ptr) {}\n";
+  os << "\n  friend class " << cxx_synthesis::getCxxImplNamespaceName() << "::";
   printCxxImplClassName(os, typeDecl);
   os << ";\n";
   os << "};\n\n";
@@ -73,9 +65,6 @@ void ClangClassTypePrinter::printClassTypeDecl(
         os << " makeRetained(void * _Nonnull ptr) noexcept { return ";
         printer.printBaseName(typeDecl);
         os << "(ptr); }\n";
-        os << "static inline void * _Nonnull getOpaquePointer(const ";
-        printer.printBaseName(typeDecl);
-        os << " &object) noexcept { return object._opaquePointer; }\n";
         os << "};\n";
       });
 }
@@ -96,12 +85,14 @@ void ClangClassTypePrinter::printClassTypeReturnScaffold(
 void ClangClassTypePrinter::printParameterCxxtoCUseScaffold(
     raw_ostream &os, const ClassDecl *type, const ModuleDecl *moduleContext,
     llvm::function_ref<void(void)> bodyPrinter, bool isInOut) {
-  // FIXME: Handle isInOut
-  ClangSyntaxPrinter(os).printModuleNamespaceQualifiersIfNeeded(
-      type->getModuleContext(), moduleContext);
-  os << cxx_synthesis::getCxxImplNamespaceName() << "::";
-  ClangValueTypePrinter::printCxxImplClassName(os, type);
-  os << "::getOpaquePointer(";
+  if (isInOut)
+    os << '&';
+  os << "::swift::" << cxx_synthesis::getCxxImplNamespaceName()
+     << "::_impl_RefCountedClass"
+     << "::getOpaquePointer";
+  if (isInOut)
+    os << "Ref";
+  os << '(';
   bodyPrinter();
   os << ')';
 }
diff --git a/lib/PrintAsClang/PrintClangFunction.cpp b/lib/PrintAsClang/PrintClangFunction.cpp
@@ -151,12 +151,13 @@ class CFunctionSignatureTypePrinter
   void visitClassType(ClassType *CT, Optional<OptionalTypeKind> optionalKind,
                       bool isInOutParam) {
     // FIXME: handle optionalKind.
-    // FIXME: handle isInOutParam.
     if (languageMode != OutputLanguageMode::Cxx) {
       os << "void * _Nonnull";
+      if (isInOutParam)
+        os << " * _Nonnull";
       return;
     }
-    if (typeUseKind == FunctionSignatureTypeUse::ParamType)
+    if (typeUseKind == FunctionSignatureTypeUse::ParamType && !isInOutParam)
       os << "const ";
     ClangSyntaxPrinter(os).printBaseName(CT->getDecl());
     if (typeUseKind == FunctionSignatureTypeUse::ParamType)
diff --git a/stdlib/public/SwiftShims/_SwiftCxxInteroperability.h b/stdlib/public/SwiftShims/_SwiftCxxInteroperability.h
@@ -53,6 +53,41 @@ inline void opaqueFree(void *_Nonnull p) noexcept {
 #endif
 }
 
+/// Base class for a Swift reference counted class value.
+class RefCountedClass {
+public:
+  inline ~RefCountedClass() { swift_release(_opaquePointer); }
+  inline RefCountedClass(const RefCountedClass &other) noexcept
+      : _opaquePointer(other._opaquePointer) {
+    swift_retain(_opaquePointer);
+  }
+  inline RefCountedClass &operator=(const RefCountedClass &other) noexcept {
+    swift_retain(other._opaquePointer);
+    swift_release(_opaquePointer);
+    _opaquePointer = other._opaquePointer;
+    return *this;
+  }
+  // FIXME: What to do in 'move'?
+  inline RefCountedClass(RefCountedClass &&) noexcept = default;
+
+protected:
+  inline RefCountedClass(void *_Nonnull ptr) noexcept : _opaquePointer(ptr) {}
+
+private:
+  void *_Nonnull _opaquePointer;
+  friend class _impl_RefCountedClass;
+};
+
+class _impl_RefCountedClass {
+public:
+  static inline void *_Nonnull getOpaquePointer(const RefCountedClass &object) {
+    return object._opaquePointer;
+  }
+  static inline void *_Nonnull &getOpaquePointerRef(RefCountedClass &object) {
+    return object._opaquePointer;
+  }
+};
+
 } // namespace _impl
 
 /// Swift's Int type.
diff --git a/test/Interop/SwiftToCxx/class/class-api-prohibited.cpp b/test/Interop/SwiftToCxx/class/class-api-prohibited.cpp
@@ -0,0 +1,12 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend %S/swift-class-in-cxx.swift -typecheck -module-name Class -clang-header-expose-public-decls -emit-clang-header-path %t/class.h
+
+// RUN: not %target-interop-build-clangxx -c %s -I %t -o %t/swift-class-execution.o
+
+#include "class.h"
+
+void test(void * _Nonnull p) {
+  // Prohibited to construct class reference directly from opaque pointer.
+  Class::ClassWithIntField x(p);
+}
diff --git a/test/Interop/SwiftToCxx/class/swift-class-execution.cpp b/test/Interop/SwiftToCxx/class/swift-class-execution.cpp
@@ -17,8 +17,8 @@
 extern "C" size_t swift_retainCount(void * _Nonnull obj);
 
 size_t getRetainCount(const Class::ClassWithIntField & swiftClass) {
-    void *p = Class::_impl::_impl_ClassWithIntField::getOpaquePointer(swiftClass);
-    return swift_retainCount(p);
+  void *p = swift::_impl::_impl_RefCountedClass::getOpaquePointer(swiftClass);
+  return swift_retainCount(p);
 }
 
 int main() {
@@ -50,6 +50,48 @@ int main() {
 // CHECK-NEXT: ClassWithIntField: 0;
 // CHECK-NEXT: ClassWithIntField: 42;
 // CHECK-NEXT: ClassWithIntField: 42;
+// CHECK-NEXT: destroy ClassWithIntField
+
+  {
+    auto x = returnClassWithIntField();
+    assert(getRetainCount(x) == 1);
+    takeClassWithIntFieldInout(x);
+    assert(getRetainCount(x) == 1);
+    takeClassWithIntField(x);
+  }
+// CHECK-NEXT: init ClassWithIntField
+// CHECK-NEXT: init ClassWithIntField
+// CHECK-NEXT: destroy ClassWithIntField
+// CHECK-NEXT: ClassWithIntField: -11;
+// CHECK-NEXT: destroy ClassWithIntField
+
+  {
+    auto x = returnClassWithIntField();
+    {
+      auto x2 = x;
+      assert(getRetainCount(x) == 2);
+    }
+    assert(getRetainCount(x) == 1);
+  }
+// CHECK-NEXT: init ClassWithIntField
+// CHECK-NEXT: destroy ClassWithIntField
+
+  {
+    auto x = returnClassWithIntField();
+    {
+      auto x2 = returnClassWithIntField();
+      assert(getRetainCount(x2) == 1);
+      assert(getRetainCount(x) == 1);
+      x = x2;
+      assert(getRetainCount(x) == 2);
+    }
+    takeClassWithIntField(x);
+    assert(getRetainCount(x) == 1);
+  }
+// CHECK-NEXT: init ClassWithIntField
+// CHECK-NEXT: init ClassWithIntField
+// CHECK-NEXT: destroy ClassWithIntField
+// CHECK-NEXT: ClassWithIntField: 0;
 // CHECK-NEXT: destroy ClassWithIntField
   return 0;
 }
diff --git a/test/Interop/SwiftToCxx/class/swift-class-in-cxx.swift b/test/Interop/SwiftToCxx/class/swift-class-in-cxx.swift
@@ -21,6 +21,7 @@ public final class ClassWithIntField {
 // CHECK: SWIFT_EXTERN void * _Nonnull $s5Class011passThroughA12WithIntFieldyAA0adeF0CADF(void * _Nonnull x) SWIFT_NOEXCEPT SWIFT_CALL; // passThroughClassWithIntField(_:)
 // CHECK-NEXT: SWIFT_EXTERN void * _Nonnull $s5Class06returnA12WithIntFieldAA0acdE0CyF(void) SWIFT_NOEXCEPT SWIFT_CALL; // returnClassWithIntField()
 // CHECK-NEXT: SWIFT_EXTERN void $s5Class04takeA12WithIntFieldyyAA0acdE0CF(void * _Nonnull x) SWIFT_NOEXCEPT SWIFT_CALL; // takeClassWithIntField(_:)
+// CHECK-NEXT: SWIFT_EXTERN void $s5Class04takeA17WithIntFieldInoutyyAA0acdE0CzF(void * _Nonnull * _Nonnull x) SWIFT_NOEXCEPT SWIFT_CALL; // takeClassWithIntFieldInout(_:)
 
 // CHECK: namespace Class {
 
@@ -30,14 +31,13 @@ public final class ClassWithIntField {
 // CHECK-EMPTY:
 // CHECK-NEXT: } // namespace _impl
 // CHECK-EMPTY:
-// CHECK-NEXT: class ClassWithIntField final {
+// CHECK-NEXT: class ClassWithIntField final : public swift::_impl::RefCountedClass {
 // CHECK-NEXT: public:
-// CHECK-NEXT:   inline ~ClassWithIntField() { swift::_impl::swift_release(_opaquePointer); }
-// CHECK-NEXT:   inline ClassWithIntField(ClassWithIntField&&) noexcept = default;
+// CHECK-NEXT:   using RefCountedClass::RefCountedClass;
+// CHECK-NEXT:   using RefCountedClass::operator=;
 // CHECK-NEXT: private:
-// CHECK-NEXT:   inline ClassWithIntField(void * _Nonnull ptr) noexcept : _opaquePointer(ptr) {}
+// CHECK-NEXT:   inline ClassWithIntField(void * _Nonnull ptr) noexcept : RefCountedClass(ptr) {}
 // CHECK-EMPTY:
-// CHECK-NEXT:   void * _Nonnull _opaquePointer;
 // CHECK-NEXT:   friend class _impl::_impl_ClassWithIntField;
 // CHECK-NEXT: };
 // CHECK-EMPTY:
@@ -46,18 +46,17 @@ public final class ClassWithIntField {
 // CHECK-NEXT:class _impl_ClassWithIntField {
 // CHECK-NEXT:public:
 // CHECK-NEXT:static inline ClassWithIntField makeRetained(void * _Nonnull ptr) noexcept { return ClassWithIntField(ptr); }
-// CHECK-NEXT:static inline void * _Nonnull getOpaquePointer(const ClassWithIntField &object) noexcept { return object._opaquePointer; }
 // CHECK-NEXT:};
 // CHECK-EMPTY:
 // CHECK-NEXT:} // namespace _impl
 
 // CHECK: inline ClassWithIntField passThroughClassWithIntField(const ClassWithIntField& x) noexcept SWIFT_WARN_UNUSED_RESULT {
-// CHECK-NEXT:  return _impl::_impl_ClassWithIntField::makeRetained(_impl::$s5Class011passThroughA12WithIntFieldyAA0adeF0CADF(_impl::_impl_ClassWithIntField::getOpaquePointer(x)));
+// CHECK-NEXT:  return _impl::_impl_ClassWithIntField::makeRetained(_impl::$s5Class011passThroughA12WithIntFieldyAA0adeF0CADF(::swift::_impl::_impl_RefCountedClass::getOpaquePointer(x)));
 // CHECK-NEXT: }
 
 public final class register { }
 
-// CHECK: class register_ final {
+// CHECK: class register_ final : public swift::_impl::RefCountedClass {
 
 public func returnClassWithIntField() -> ClassWithIntField {
   return ClassWithIntField()
@@ -73,10 +72,15 @@ public func takeClassWithIntField(_ x: ClassWithIntField) {
   print("ClassWithIntField: \(x.field);")
 }
 
+public func takeClassWithIntFieldInout(_ x: inout ClassWithIntField) {
+  x = ClassWithIntField()
+  x.field = -11
+}
+
 // CHECK: inline ClassWithIntField returnClassWithIntField() noexcept SWIFT_WARN_UNUSED_RESULT {
 // CHECK-NEXT:   return _impl::_impl_ClassWithIntField::makeRetained(_impl::$s5Class06returnA12WithIntFieldAA0acdE0CyF());
 // CHECK-NEXT: }
 
 // CHECK: inline void takeClassWithIntField(const ClassWithIntField& x) noexcept {
-// CHECK-NEXT:  return _impl::$s5Class04takeA12WithIntFieldyyAA0acdE0CF(_impl::_impl_ClassWithIntField::getOpaquePointer(x));
+// CHECK-NEXT:  return _impl::$s5Class04takeA12WithIntFieldyyAA0acdE0CF(::swift::_impl::_impl_RefCountedClass::getOpaquePointer(x));
 // CHECK-NEXT: }
