Merge pull request #65749 from hyp/eng/move-semantics-fixups

hyp · web-flow · commit 1bcda2fb0c84 · 2023-05-07T22:41:16.000-07:00
[interop][SwiftToCxx] move semantics for Swift classes; use of C++ move for unimplemented Swift value moves fails at link time now
diff --git a/lib/PrintAsClang/PrintClangValueType.cpp b/lib/PrintAsClang/PrintClangValueType.cpp
@@ -308,11 +308,21 @@ void ClangValueTypePrinter::printValueTypeDecl(
 
   // FIXME: implement the move constructor.
   os << "  [[noreturn]] ";
-  printer.printInlineForThunk();
+  // NOTE: Do not apply attribute((used))
+  // here to ensure the linker error isn't
+  // forced, so just mark this an inline
+  // helper function instead.
+  printer.printInlineForHelperFunction();
   printer.printBaseName(typeDecl);
   os << "(";
   printer.printBaseName(typeDecl);
-  os << " &&) noexcept { abort(); }\n";
+  StringRef moveErrorMessage = "C++ does not support moving a Swift value yet";
+  os << " &&) noexcept {\n  "
+        "swift::_impl::_fatalError_Cxx_move_of_Swift_value_type_not_supported_"
+        "yet();\n  swift::_impl::_swift_stdlib_reportFatalError(\"swift\", 5, "
+        "\""
+     << moveErrorMessage << "\", " << moveErrorMessage.size()
+     << ", 0);\n  abort();\n  }\n";
 
   bodyPrinter();
   if (typeDecl->isStdlibDecl())
diff --git a/lib/PrintAsClang/_SwiftCxxInteroperability.h b/lib/PrintAsClang/_SwiftCxxInteroperability.h
@@ -93,6 +93,21 @@ extern "C" void *_Nonnull swift_retain(void *_Nonnull) noexcept;
 
 extern "C" void swift_release(void *_Nonnull) noexcept;
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreserved-identifier"
+
+extern "C" void _swift_stdlib_reportFatalError(const char *_Nonnull prefix,
+                                               int prefixLength,
+                                               const char *_Nonnull message,
+                                               int messageLength,
+                                               uint32_t flags) noexcept;
+
+// A dummy symbol that forces a linker error when
+// C++ tries to invoke a move of a Swift value type.
+extern "C" void _fatalError_Cxx_move_of_Swift_value_type_not_supported_yet();
+
+#pragma clang diagnostic pop
+
 SWIFT_INLINE_THUNK void *_Nonnull opaqueAlloc(size_t size,
                                               size_t align) noexcept {
 #if defined(_WIN32)
@@ -159,18 +174,28 @@ class RefCountedClass {
       : _opaquePointer(other._opaquePointer) {
     swift_retain(_opaquePointer);
   }
+  SWIFT_INLINE_THUNK RefCountedClass(RefCountedClass &&other) noexcept
+      : _opaquePointer(other._opaquePointer) {
+    // Moving a Swift class reference is a copy
+    // in C++. This allows C++ to avoid liveness
+    // checks to see if the pointer is `null` or not,
+    // as C++'s move is not consuming, unlike Swift's.
+    swift_retain(_opaquePointer);
+  }
   SWIFT_INLINE_THUNK RefCountedClass &
   operator=(const RefCountedClass &other) noexcept {
     swift_retain(other._opaquePointer);
     swift_release(_opaquePointer);
     _opaquePointer = other._opaquePointer;
     return *this;
   }
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wmissing-noreturn"
-  // FIXME: implement 'move'?
-  SWIFT_INLINE_THUNK RefCountedClass(RefCountedClass &&) noexcept { abort(); }
-#pragma clang diagnostic pop
+  SWIFT_INLINE_THUNK RefCountedClass &
+  operator=(RefCountedClass &&other) noexcept {
+    swift_retain(other._opaquePointer);
+    swift_release(_opaquePointer);
+    _opaquePointer = other._opaquePointer;
+    return *this;
+  }
 
 protected:
   SWIFT_INLINE_THUNK RefCountedClass(void *_Nonnull ptr) noexcept
diff --git a/test/Interop/SwiftToCxx/class/swift-class-execution.cpp b/test/Interop/SwiftToCxx/class/swift-class-execution.cpp
@@ -132,6 +132,33 @@ int main() {
 // CHECK-NEXT: init ClassWithIntField
 // CHECK-NEXT: destroy ClassWithIntField
 // CHECK-NEXT: ClassWithIntField: 0;
+// CHECK-NEXT: destroy ClassWithIntField
+
+  {
+    auto x = returnClassWithIntField();
+    assert(getRetainCount(x) == 1);
+    ClassWithIntField x2(std::move(x));
+    // Moving a Swift class in C++ is not consuming.
+    assert(getRetainCount(x) == 2);
+  }
+// CHECK-NEXT: init ClassWithIntField
+// CHECK-NEXT: destroy ClassWithIntField
+
+  {
+      auto x = returnClassWithIntField();
+      auto x2 = returnClassWithIntField();
+      assert(getRetainCount(x) == 1);
+      assert(getRetainCount(x2) == 1);
+      x2 = std::move(x);
+      // Moving a Swift class in C++ is not consuming.
+      assert(getRetainCount(x) == 2);
+      assert(getRetainCount(x2) == 2);
+      takeClassWithIntField(x2);
+  }
+// 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-inheritance-execution.cpp b/test/Interop/SwiftToCxx/class/swift-class-inheritance-execution.cpp
@@ -95,6 +95,17 @@ int main() {
 // CHECK-NEXT: useDerivedClass, type=Class.DerivedDerivedClass
 // CHECK-NEXT: destroy DerivedDerivedClass
 // CHECK-NEXT: destroy DerivedClass
+// CHECK-NEXT: destroy BaseClass
+
+  {
+    BaseClass x = returnDerivedClass();
+    assert(getRetainCount(x) == 1);
+    useBaseClass(x);
+  }
+// CHECK-NEXT: init BaseClass
+// CHECK-NEXT: init DerivedClass
+// CHECK-NEXT: useBaseClass, type=Class.DerivedClass
+// CHECK-NEXT: destroy DerivedClass
 // CHECK-NEXT: destroy BaseClass
   return 0;
 }
diff --git a/test/Interop/SwiftToCxx/expose-attr/expose-swift-decls-to-cxx.swift b/test/Interop/SwiftToCxx/expose-attr/expose-swift-decls-to-cxx.swift
@@ -83,6 +83,7 @@ public final class ExposedClass {
 // CHECK: class SWIFT_SYMBOL("{{.*}}") ExposedStruct final {
 // CHECK: class SWIFT_SYMBOL("{{.*}}") ExposedStruct2 final {
 // CHECK: ExposedStruct2(ExposedStruct2 &&)
+// CHECK: }
 // CHECK-NEXT: swift::Int getY() const SWIFT_SYMBOL("{{.*}}");
 // CHECK-NEXT: void setY(swift::Int value) SWIFT_SYMBOL("{{.*}}");
 // CHECK-NEXT: static SWIFT_INLINE_THUNK ExposedStruct2 init() SWIFT_SYMBOL("{{.*}}");
diff --git a/test/Interop/SwiftToCxx/initializers/init-in-cxx.swift b/test/Interop/SwiftToCxx/initializers/init-in-cxx.swift
@@ -39,7 +39,8 @@ public struct FirstSmallStruct {
 
 // CHECK: class SWIFT_SYMBOL("s:4Init16FirstSmallStructV") FirstSmallStruct final {
 // CHECK-NEXT: public:
-// CHECK:   SWIFT_INLINE_THUNK FirstSmallStruct(FirstSmallStruct &&)
+// CHECK:   SWIFT_INLINE_PRIVATE_HELPER FirstSmallStruct(FirstSmallStruct &&)
+// CHECK: }
 // CHECK-NEXT:   SWIFT_INLINE_THUNK uint32_t getX() const SWIFT_SYMBOL("s:4Init16FirstSmallStructV1xs6UInt32Vvp");
 // CHECK-NEXT:   static SWIFT_INLINE_THUNK FirstSmallStruct init() SWIFT_SYMBOL("s:4Init16FirstSmallStructVACycfc");
 // CHECK-NEXT:   static SWIFT_INLINE_THUNK FirstSmallStruct init(swift::Int x) SWIFT_SYMBOL("s:4Init16FirstSmallStructVyACSicfc");
diff --git a/test/Interop/SwiftToCxx/methods/method-in-cxx.swift b/test/Interop/SwiftToCxx/methods/method-in-cxx.swift
@@ -95,7 +95,8 @@ public final class PassStructInClassMethod {
 // CHECK-NEXT:   static SWIFT_INLINE_THUNK LargeStruct staticFinalClassMethod(swift::Int x) SWIFT_SYMBOL("s:7Methods09ClassWithA0C011staticFinalB6Method1xAA11LargeStructVSi_tFZ");
 
 // CHECK: class SWIFT_SYMBOL("s:7Methods11LargeStructV") LargeStruct final {
-// CHECK: SWIFT_INLINE_THUNK LargeStruct(LargeStruct &&)
+// CHECK: SWIFT_INLINE_PRIVATE_HELPER LargeStruct(LargeStruct &&)
+// CHECK: }
 // CHECK-NEXT: SWIFT_INLINE_THUNK LargeStruct doubled() const SWIFT_SYMBOL("s:7Methods11LargeStructV7doubledACyF");
 // CHECK-NEXT: SWIFT_INLINE_THUNK void dump() const SWIFT_SYMBOL("s:7Methods11LargeStructV4dumpyyF");
 // CHECK-NEXT: SWIFT_INLINE_THUNK LargeStruct scaled(swift::Int x, swift::Int y) const SWIFT_SYMBOL("s:7Methods11LargeStructV6scaledyACSi_SitF");
diff --git a/test/Interop/SwiftToCxx/methods/mutating-method-in-cxx.swift b/test/Interop/SwiftToCxx/methods/mutating-method-in-cxx.swift
@@ -57,14 +57,16 @@ public struct SmallStruct {
 // CHECK: SWIFT_EXTERN void $s7Methods11SmallStructV6invertyyF(SWIFT_CONTEXT void * _Nonnull _self) SWIFT_NOEXCEPT SWIFT_CALL; // invert()
 
 // CHECK: class SWIFT_SYMBOL("s:7Methods11LargeStructV") LargeStruct final {
-// CHECK: SWIFT_INLINE_THUNK LargeStruct(LargeStruct &&)
+// CHECK: SWIFT_INLINE_PRIVATE_HELPER LargeStruct(LargeStruct &&)
+// CHECK: }
 // CHECK-NEXT:   SWIFT_INLINE_THUNK void dump() const SWIFT_SYMBOL("s:7Methods11LargeStructV4dumpyyF");
 // CHECK-NEXT:   SWIFT_INLINE_THUNK void double_() SWIFT_SYMBOL("s:7Methods11LargeStructV6doubleyyF");
 // CHECK-NEXT:   SWIFT_INLINE_THUNK LargeStruct scale(swift::Int x, swift::Int y) SWIFT_SYMBOL("s:7Methods11LargeStructV5scaleyACSi_SitF");
 // CHECK-NEXT: private
 
 // CHECK: class SWIFT_SYMBOL("s:7Methods11SmallStructV") SmallStruct final {
-// CHECK:   SWIFT_INLINE_THUNK SmallStruct(SmallStruct &&)
+// CHECK:   SWIFT_INLINE_PRIVATE_HELPER SmallStruct(SmallStruct &&)
+// CHECK: }
 // CHECK-NEXT:   SWIFT_INLINE_THUNK void dump() const SWIFT_SYMBOL("s:7Methods11SmallStructV4dumpyyF");
 // CHECK-NEXT:   SWIFT_INLINE_THUNK SmallStruct scale(float y) SWIFT_SYMBOL("s:7Methods11SmallStructV5scaleyACSfF");
 // CHECK-NEXT:   SWIFT_INLINE_THUNK void invert() SWIFT_SYMBOL("s:7Methods11SmallStructV6invertyyF");
diff --git a/test/Interop/SwiftToCxx/properties/getter-in-cxx.swift b/test/Interop/SwiftToCxx/properties/getter-in-cxx.swift
@@ -10,7 +10,8 @@ public struct FirstSmallStruct {
 
 // CHECK: class SWIFT_SYMBOL({{.*}}) FirstSmallStruct final {
 // CHECK: public:
-// CHECK:   SWIFT_INLINE_THUNK FirstSmallStruct(FirstSmallStruct &&)
+// CHECK:   SWIFT_INLINE_PRIVATE_HELPER FirstSmallStruct(FirstSmallStruct &&)
+// CHECK: }
 // CHECK-NEXT:   SWIFT_INLINE_THUNK uint32_t getX() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT:   private:
 
@@ -36,7 +37,8 @@ public struct LargeStruct {
 
 // CHECK: class SWIFT_SYMBOL({{.*}}) LargeStruct final {
 // CHECK: public:
-// CHECK: SWIFT_INLINE_THUNK LargeStruct(LargeStruct &&)
+// CHECK: SWIFT_INLINE_PRIVATE_HELPER LargeStruct(LargeStruct &&)
+// CHECK: }
 // CHECK-NEXT: SWIFT_INLINE_THUNK swift::Int getX1() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT: SWIFT_INLINE_THUNK swift::Int getX2() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT: SWIFT_INLINE_THUNK swift::Int getX3() const SWIFT_SYMBOL({{.*}});
@@ -92,7 +94,8 @@ public struct SmallStructWithGetters {
 
 // CHECK: class SWIFT_SYMBOL({{.*}}) SmallStructWithGetters final {
 // CHECK: public:
-// CHECK:   SWIFT_INLINE_THUNK SmallStructWithGetters(SmallStructWithGetters &&)
+// CHECK:   SWIFT_INLINE_PRIVATE_HELPER SmallStructWithGetters(SmallStructWithGetters &&)
+// CHECK: }
 // CHECK-NEXT:  SWIFT_INLINE_THUNK uint32_t getStoredInt() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT:  SWIFT_INLINE_THUNK swift::Int getComputedInt() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT:  SWIFT_INLINE_THUNK LargeStruct getLargeStruct() const SWIFT_SYMBOL({{.*}});
diff --git a/test/Interop/SwiftToCxx/properties/setter-in-cxx.swift b/test/Interop/SwiftToCxx/properties/setter-in-cxx.swift
@@ -10,7 +10,8 @@ public struct FirstSmallStruct {
 
 // CHECK: class SWIFT_SYMBOL({{.*}}) FirstSmallStruct final {
 // CHECK: public:
-// CHECK:   SWIFT_INLINE_THUNK FirstSmallStruct(FirstSmallStruct &&)
+// CHECK:   SWIFT_INLINE_PRIVATE_HELPER FirstSmallStruct(FirstSmallStruct &&)
+// CHECK: }
 // CHECK-NEXT:   SWIFT_INLINE_THUNK uint32_t getX() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT:   SWIFT_INLINE_THUNK void setX(uint32_t value) SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT:   private:
@@ -31,7 +32,8 @@ public struct LargeStruct {
 
 // CHECK: class SWIFT_SYMBOL({{.*}}) LargeStruct final {
 // CHECK: public:
-// CHECK: SWIFT_INLINE_THUNK LargeStruct(LargeStruct &&)
+// CHECK: SWIFT_INLINE_PRIVATE_HELPER LargeStruct(LargeStruct &&)
+// CHECK: }
 // CHECK-NEXT: SWIFT_INLINE_THUNK swift::Int getX1() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT: SWIFT_INLINE_THUNK void setX1(swift::Int value) SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT: SWIFT_INLINE_THUNK swift::Int getX2() const SWIFT_SYMBOL({{.*}});
@@ -109,7 +111,8 @@ public struct SmallStructWithProps {
 
 // CHECK: class SWIFT_SYMBOL({{.*}}) SmallStructWithProps final {
 // CHECK: public:
-// CHECK:   SWIFT_INLINE_THUNK SmallStructWithProps(SmallStructWithProps &&)
+// CHECK:   SWIFT_INLINE_PRIVATE_HELPER SmallStructWithProps(SmallStructWithProps &&)
+// CHECK: }
 // CHECK-NEXT:    SWIFT_INLINE_THUNK uint32_t getStoredInt() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT:    SWIFT_INLINE_THUNK void setStoredInt(uint32_t value) SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT:    SWIFT_INLINE_THUNK swift::Int getComputedInt() const SWIFT_SYMBOL({{.*}});
diff --git a/test/Interop/SwiftToCxx/stdlib/swift-stdlib-in-cxx.swift b/test/Interop/SwiftToCxx/stdlib/swift-stdlib-in-cxx.swift
@@ -72,7 +72,8 @@
 // CHECK:  }
 // CHECK-NEXT:  SWIFT_INLINE_THUNK String(const String &other) noexcept {
 // CHECK:  }
-// CHECK-NEXT:  SWIFT_INLINE_THUNK String(String &&) noexcept { abort(); }
+// CHECK-NEXT:  SWIFT_INLINE_PRIVATE_HELPER String(String &&) noexcept {
+// CHECK: }
 // CHECK-NEXT:  static SWIFT_INLINE_THUNK String init() SWIFT_SYMBOL({{.*}});
 // CHECK:  SWIFT_INLINE_THUNK void append(const String& other)
 // CHECK:  SWIFT_INLINE_THUNK UTF8View getUtf8() const SWIFT_SYMBOL({{.*}});
@@ -107,7 +108,8 @@
 // CHECK-NEXT: private:
 
 // CHECK: class SWIFT_SYMBOL({{.*}}) UTF8View final {
-// CHECK:  SWIFT_INLINE_THUNK UTF8View(UTF8View &&) noexcept { abort(); }
+// CHECK: SWIFT_INLINE_PRIVATE_HELPER UTF8View(UTF8View &&) noexcept {
+// CHECK: }
 // CHECK-NEXT: SWIFT_INLINE_THUNK String_Index getStartIndex() const SWIFT_SYMBOL({{.*}});
 // CHECK-NEXT:   SWIFT_INLINE_THUNK String_Index getEndIndex() const SWIFT_SYMBOL({{.*}});
 // CHECK:   SWIFT_INLINE_THUNK swift::Optional<String_Index> indexOffsetByLimitedBy(const String_Index& i, swift::Int n, const String_Index& limit) const SWIFT_SYMBOL({{.*}});
diff --git a/test/Interop/SwiftToCxx/structs/struct-move-semantics-in-cxx.cpp b/test/Interop/SwiftToCxx/structs/struct-move-semantics-in-cxx.cpp
@@ -0,0 +1,41 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend %S/large-structs-pass-return-indirect-in-cxx.swift -typecheck -module-name Structs -clang-header-expose-decls=all-public -emit-clang-header-path %t/structs.h
+
+// Link should fail by default when a move is
+// performed in C++.
+// RUN: %target-interop-build-clangxx -c %s -I %t -o %t/swift-structs-execution.o
+// RUN: not %target-interop-build-swift %S/large-structs-pass-return-indirect-in-cxx.swift -o %t/swift-structs-execution -Xlinker %t/swift-structs-execution.o -module-name Structs -Xfrontend -entry-point-function-name -Xfrontend swiftMain 2>&1 | %FileCheck --check-prefix=LINK %s
+
+// LINK: fatalError_Cxx_move_of_Swift_value_type_not_supported_yet
+
+// Fallback to abort at runtime:
+
+// RUN: %target-interop-build-clangxx -c %s -I %t -o %t/swift-structs-execution.o -DLINKS
+// RUN: %target-interop-build-swift %S/large-structs-pass-return-indirect-in-cxx.swift -o %t/swift-structs-execution -Xlinker %t/swift-structs-execution.o -module-name Structs -Xfrontend -entry-point-function-name -Xfrontend swiftMain 2>&1
+
+// RUN: %target-codesign %t/swift-structs-execution
+// RUN: %target-run %t/swift-structs-execution 2>%t/output || true
+// RUN: cat %t/output  | %FileCheck %s
+
+// REQUIRES: executable_test
+
+#include <assert.h>
+#include "structs.h"
+#include <memory>
+
+#ifdef LINKS
+extern "C" void _fatalError_Cxx_move_of_Swift_value_type_not_supported_yet() {
+    
+}
+#endif
+
+int main() {
+  using namespace Structs;
+
+  auto x = returnNewStructSeveralI64(42);
+  StructSeveralI64 x2 = std::move(x);
+  return 0;
+}
+
+// CHECK: C++ does not support moving a Swift value yet
diff --git a/test/Interop/SwiftToCxx/structs/struct-with-refcounted-member.swift b/test/Interop/SwiftToCxx/structs/struct-with-refcounted-member.swift
@@ -47,5 +47,9 @@ public func printBreak(_ x: Int) {
 // CHECK-NEXT: #endif
 // CHECK-NEXT:     vwTable->initializeWithCopy(_getOpaquePointer(), const_cast<char *>(other._getOpaquePointer()), metadata._0);
 // CHECK-NEXT:   }
-// CHECK-NEXT:   SWIFT_INLINE_THUNK StructWithRefcountedMember(StructWithRefcountedMember &&) noexcept { abort(); }
+// CHECK-NEXT:   SWIFT_INLINE_PRIVATE_HELPER StructWithRefcountedMember(StructWithRefcountedMember &&) noexcept {
+// CHECK-NEXT:  swift::_impl::_fatalError_Cxx_move_of_Swift_value_type_not_supported_yet();
+// CHECK-NEXT:  swift::_impl::_swift_stdlib_reportFatalError("swift", 5, "C++ does not support moving a Swift value yet", 45, 0);
+// CHECK-NEXT:  abort();
+// CHECK-NEXT: }
 // CHECK-NEXT: private:
diff --git a/test/Interop/SwiftToCxx/structs/swift-struct-in-cxx.swift b/test/Interop/SwiftToCxx/structs/swift-struct-in-cxx.swift
@@ -38,7 +38,11 @@
 // CHECK:        }
 // CHECK-NEXT:   SWIFT_INLINE_THUNK StructWithIntField(const StructWithIntField &other) noexcept {
 // CHECK:        }
-// CHECK-NEXT:   noreturn]] SWIFT_INLINE_THUNK StructWithIntField(StructWithIntField &&) noexcept { abort(); }
+// CHECK-NEXT:   noreturn]] SWIFT_INLINE_PRIVATE_HELPER StructWithIntField(StructWithIntField &&) noexcept {
+// CHECK-NEXT:   swift::_impl::_fatalError_Cxx_move_of_Swift_value_type_not_supported_yet();
+// CHECK-NEXT:   swift::_impl::_swift_stdlib_reportFatalError("swift", 5, "C++ does not support moving a Swift value yet", 45, 0);
+// CHECK-NEXT:   abort();
+// CHECK-NEXT:   }
 // CHECK-NEXT: private:
 // CHECK-NEXT:   SWIFT_INLINE_THUNK StructWithIntField() noexcept {}
 // CHECK-NEXT:   static SWIFT_INLINE_THUNK StructWithIntField _make() noexcept { return StructWithIntField(); }
