swiftlang · hyp · Jul 22, 2023 · Jul 12, 2023 · Jul 17, 2023 · Jul 21, 2023
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -2472,39 +2472,33 @@ namespace {
       }
 
       if (cxxRecordDecl) {
-        // FIXME: Swift right now uses AddressOnly type layout
-        // in a way that conflates C++ types
-        // that need to be destroyed or copied explicitly with C++
-        // types that have to be passed indirectly, because
-        // only AddressOnly types can be copied or destroyed using C++
-        // semantics. However, in actuality these two concepts are
-        // separate and don't map to one notion of AddressOnly type
-        // layout cleanly. We should reserve the use of AddressOnly
-        // type layout when types have to use C++ copy/move/destroy
-        // operations, but allow AddressOnly types to be passed
-        // directly as well. This will help unify the MSVC and
-        // Itanium difference here, and will allow us to support
-        // trivial_abi C++ types as well.
-        auto isNonTrivialForPurposeOfCalls =
-            [](const clang::CXXRecordDecl *decl) -> bool {
-          return decl->hasNonTrivialCopyConstructor() ||
-                 decl->hasNonTrivialMoveConstructor() ||
-                 !decl->hasTrivialDestructor();
-        };
-        auto isAddressOnlySwiftStruct =
-            [&](const clang::CXXRecordDecl *decl) -> bool {
-          // MSVC ABI allows non-trivially destroyed C++ types
-          // to be passed in register. This is not supported, as such
-          // type wouldn't be destroyed in Swift correctly. Therefore,
-          // force AddressOnly type layout using the old heuristic.
-          // FIXME: Support can pass in registers for MSVC correctly.
-          if (Impl.SwiftContext.LangOpts.Target.isWindowsMSVCEnvironment())
-            return isNonTrivialForPurposeOfCalls(decl);
-          return !decl->canPassInRegisters();
-        };
-        if (auto structResult = dyn_cast<StructDecl>(result))
-          structResult->setIsCxxNonTrivial(
-              isAddressOnlySwiftStruct(cxxRecordDecl));
+        if (auto structResult = dyn_cast<StructDecl>(result)) {
+          // Address-only type is a type that can't be passed in registers.
+          // Address-only types are typically non-trivial, however some
+          // non-trivial types can be loadable as well (although such types
+          // are not yet available in Swift).
+          bool isAddressOnly = !cxxRecordDecl->canPassInRegisters();
+          // Check if the given type is non-trivial to ensure we can
+          // still perform the right copy/move/destroy even if it's
+          // not an address-only type.
+          auto isNonTrivial = [](const clang::CXXRecordDecl *decl) -> bool {
+            return decl->hasNonTrivialCopyConstructor() ||
+                   decl->hasNonTrivialMoveConstructor() ||
+                   !decl->hasTrivialDestructor();
+          };
+          if (!isAddressOnly &&
+              Impl.SwiftContext.LangOpts.Target.isWindowsMSVCEnvironment() &&
+              isNonTrivial(cxxRecordDecl)) {
+            // MSVC ABI allows non-trivially destroyed C++ types
+            // to be passed in register. This is not supported, as such
+            // type wouldn't be destroyed in Swift correctly. Therefore,
+            // mark this type as unavailable.
+            // FIXME: Support can pass in registers for MSVC correctly.
+            Impl.markUnavailable(result, "non-trivial C++ class with trivial "
+                                         "ABI is not yet available in Swift");
+          }
+          structResult->setIsCxxNonTrivial(isAddressOnly);
+        }
 
         for (auto &getterAndSetter : Impl.GetterSetterMap[result]) {
           auto getter = getterAndSetter.second.first;

diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp
@@ -2301,7 +2301,14 @@ class SyncCallEmission final : public CallEmission {
     SILFunctionConventions fnConv(origCalleeType, IGF.getSILModule());
 
     // Pass along the indirect result pointers.
-    original.transferInto(adjusted, fnConv.getNumIndirectSILResults());
+    auto passIndirectResults = [&]() {
+        original.transferInto(adjusted, fnConv.getNumIndirectSILResults());
+    };
+    // Indirect results for C++ methods can come
+    // after `this`.
+    if (getCallee().getRepresentation() !=
+        SILFunctionTypeRepresentation::CXXMethod)
+      passIndirectResults();
 
     // Pass along the coroutine buffer.
     switch (origCalleeType->getCoroutineKind()) {
@@ -2341,7 +2348,14 @@ class SyncCallEmission final : public CallEmission {
                                        IGF.IGM.getPointerAlignment());
         }
 
+        // Windows ABI places `this` before the
+        // returned indirect values.
+        bool isThisFirst = IGF.IGM.Triple.isWindowsMSVCEnvironment();
+        if (!isThisFirst)
+          passIndirectResults();
         adjusted.add(arg);
+        if (isThisFirst)
+          passIndirectResults();
       }
 
       LLVM_FALLTHROUGH;

diff --git a/test/Interop/Cxx/class/Inputs/destructors.h b/test/Interop/Cxx/class/Inputs/destructors.h
@@ -6,11 +6,21 @@ struct DummyStruct {};
 struct __attribute__((swift_attr("import_unsafe")))
 HasUserProvidedDestructorAndDummy {
   DummyStruct dummy;
+  HasUserProvidedDestructorAndDummy(DummyStruct dummy) : dummy(dummy) {}
+#if __is_target_os(windows)
+  // On windows, force this type to be address-only.
+  HasUserProvidedDestructorAndDummy(const HasUserProvidedDestructorAndDummy &) {}
+#endif
   ~HasUserProvidedDestructorAndDummy() {}
 };
 
 struct __attribute__((swift_attr("import_unsafe"))) HasUserProvidedDestructor {
   int *value;
+#if __is_target_os(windows)
+  // On windows, force this type to be address-only.
+  HasUserProvidedDestructor() {}
+  HasUserProvidedDestructor(const HasUserProvidedDestructor &other) {}
+#endif
   ~HasUserProvidedDestructor() { *value = 42; }
 };
 

diff --git a/test/Interop/Cxx/class/Inputs/nodiscard.h b/test/Interop/Cxx/class/Inputs/nodiscard.h
@@ -6,6 +6,7 @@
 class NoDiscardMultiply {
  public:
   NoDiscardMultiply() {}
+  NoDiscardMultiply(const NoDiscardMultiply &) { }
   ~NoDiscardMultiply() {}
 
   [[nodiscard]] int Multiply(int x, int y) { return x * y; }

diff --git a/test/Interop/Cxx/class/Inputs/protocol-conformance.h b/test/Interop/Cxx/class/Inputs/protocol-conformance.h
@@ -12,6 +12,7 @@ struct DoesNotConformToProtocol {
 struct DummyStruct {};
 
 struct __attribute__((swift_attr("import_unsafe"))) NonTrivial {
+  NonTrivial(const NonTrivial &other) {}
   ~NonTrivial() {}
   NonTrivial(DummyStruct) {}
   NonTrivial() {}
@@ -67,4 +68,4 @@ struct HasOperatorPlusEqual {
 
 using HasOperatorPlusEqualInt = HasOperatorPlusEqual<int>;
 
-#endif // TEST_INTEROP_CXX_CLASS_INPUTS_PROTOCOL_CONFORMANCE_H
+#endif // TEST_INTEROP_CXX_CLASS_INPUTS_PROTOCOL_CONFORMANCE_H
diff --git a/test/Interop/Cxx/class/Inputs/type-classification.h b/test/Interop/Cxx/class/Inputs/type-classification.h
@@ -80,6 +80,12 @@ struct StructWithSubobjectMoveAssignment {
 };
 
 struct __attribute__((swift_attr("import_unsafe"))) StructWithDestructor {
+#if __is_target_os(windows)
+  // On windows, force this type to be address-only.
+  StructWithDestructor() {}
+  StructWithDestructor(const StructWithDestructor &other) {}
+#endif
+
   ~StructWithDestructor() {}
 };
 

diff --git a/test/Interop/Cxx/class/delete-operator-destructor-ref-irgen.swift b/test/Interop/Cxx/class/delete-operator-destructor-ref-irgen.swift
@@ -24,6 +24,7 @@ public:
 class Container {
 public:
     Container() : pointer(new BaseClass) {}
+    Container(const Container &other) : pointer(new BaseClass) {}
     ~Container() { delete pointer; }
 
     inline int method() const {

diff --git a/test/Interop/Cxx/class/destructors-correct-abi-irgen.swift b/test/Interop/Cxx/class/destructors-correct-abi-irgen.swift
@@ -4,10 +4,12 @@ import Destructors
 
 // CHECK-LABEL: define {{.*}}void @"$s4main4testyyF"
 // CHECK: [[H:%.*]] = alloca %TSo33HasUserProvidedDestructorAndDummyV
+// CHECK: [[CXX_THIS_PRE:%.*]] = bitcast %TSo33HasUserProvidedDestructorAndDummyV* [[H]] to %struct.HasUserProvidedDestructorAndDummy*
 // CHECK: [[CXX_THIS:%.*]] = bitcast %TSo33HasUserProvidedDestructorAndDummyV* [[H]] to %struct.HasUserProvidedDestructorAndDummy*
 // CHECK: call {{.*}}@{{_ZN33HasUserProvidedDestructorAndDummyD(1|2)Ev|"\?\?1HasUserProvidedDestructorAndDummy@@QEAA@XZ"}}(%struct.HasUserProvidedDestructorAndDummy* [[CXX_THIS]])
+
 // CHECK: ret void
 public func test() {
   let d = DummyStruct()
-  let h = HasUserProvidedDestructorAndDummy(dummy: d)
+  let h = HasUserProvidedDestructorAndDummy(d)
 }
diff --git a/test/Interop/Cxx/class/inheritance/Inputs/fields.h b/test/Interop/Cxx/class/inheritance/Inputs/fields.h
@@ -30,6 +30,7 @@ struct DerivedFromOneField : OneField {};
 
 struct __attribute__((swift_attr("import_unsafe"))) NonTrivial {
   NonTrivial() {}
+  NonTrivial(const NonTrivial &) {}
   ~NonTrivial() {}
 };
 
@@ -45,6 +46,7 @@ struct NonTrivialDerivedWithOneField : NonTrivialHasThreeFields {
 
 struct __attribute__((swift_attr("import_unsafe"))) NonTrivialHasOneField {
   NonTrivialHasOneField() {}
+  NonTrivialHasOneField(const NonTrivialHasOneField &other) : e(other.e) {}
   ~NonTrivialHasOneField() {}
 
   int e = 5;

diff --git a/test/Interop/Cxx/class/inheritance/Inputs/functions.h b/test/Interop/Cxx/class/inheritance/Inputs/functions.h
@@ -1,5 +1,6 @@
 struct __attribute__((swift_attr("import_unsafe"))) NonTrivial {
   NonTrivial() {}
+  NonTrivial(const NonTrivial &) {}
   ~NonTrivial() {}
 
   inline const char *inNonTrivial() const

diff --git a/test/Interop/Cxx/class/method/Inputs/methods.h b/test/Interop/Cxx/class/method/Inputs/methods.h
@@ -4,6 +4,11 @@
 struct __attribute__((swift_attr("import_unsafe"))) NonTrivialInWrapper {
   int value;
 
+  NonTrivialInWrapper(int value) : value(value) {}
+
+  // explicit copy constructor is needed, as on Windows a destructor
+  // still makes this a type that's passed in registers.
+  NonTrivialInWrapper(const NonTrivialInWrapper &other) : value(other.value) {}
   ~NonTrivialInWrapper() { }
 };
 

diff --git a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift
@@ -0,0 +1,19 @@
+// RUN: %target-swift-emit-irgen -I %S/Inputs -enable-experimental-cxx-interop %s -Xcc -fignore-exceptions | %FileCheck %s
+
+// UNSUPPORTED: OS=windows-msvc
+
+import Methods
+
+public func use() -> CInt {
+    var instance = HasMethods()
+    let result = instance.nonConstPassThroughAsWrapper(42)
+    return result.value
+}
+
+// CHECK: %[[instance:.*]] = alloca %TSo10HasMethodsV
+// CHECK: %[[result:.*]] = alloca %TSo19NonTrivialInWrapperV
+// CHECK: %[[CXX_THIS_PRE:.*]] = bitcast %TSo10HasMethodsV* %[[instance]] to %struct.HasMethods*
+// CHECK: %[[CXX_THIS:.*]] = bitcast %TSo10HasMethodsV* %[[instance]] to %struct.HasMethods*
+// CHECK: call void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(%struct.NonTrivialInWrapper* sret(%struct.NonTrivialInWrapper) %{{.*}}, %struct.HasMethods* %[[CXX_THIS]], i32 42)
+
+// CHECK: define {{.*}} void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(%struct.NonTrivialInWrapper* noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, %struct.HasMethods* {{.*}} %{{.*}}, i32 noundef %{{.*}})
diff --git a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift
@@ -0,0 +1,19 @@
+// RUN: %target-swift-emit-irgen -I %S/Inputs -enable-experimental-cxx-interop %s | %FileCheck %s
+
+// REQUIRES: OS=windows-msvc
+
+import Methods
+
+public func use() -> CInt {
+    var instance = HasMethods()
+    let result = instance.nonConstPassThroughAsWrapper(42)
+    return result.value
+}
+
+// CHECK: %[[instance:.*]] = alloca %TSo10HasMethodsV
+// CHECK: %[[result:.*]] = alloca %TSo19NonTrivialInWrapperV
+// CHECK: %[[CXX_THIS_PRE:.*]] = bitcast %TSo10HasMethodsV* %[[instance]] to %struct.HasMethods*
+// CHECK: %[[CXX_THIS:.*]] = bitcast %TSo10HasMethodsV* %[[instance]] to %struct.HasMethods*
+// CHECK: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(%struct.HasMethods* %[[CXX_THIS]], %struct.NonTrivialInWrapper* sret(%struct.NonTrivialInWrapper) %{{.*}}, i32 42)
+
+// CHECK: define {{.*}} void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(%struct.HasMethods* {{.*}} %{{.*}}, %struct.NonTrivialInWrapper* noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, i32 noundef %{{.*}})
diff --git a/test/Interop/Cxx/class/method/methods.swift b/test/Interop/Cxx/class/method/methods.swift
@@ -1,9 +1,6 @@
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop)
 //
 // REQUIRES: executable_test
-//
-// Crash when running on windows: rdar://88391102
-// XFAIL: OS=windows-msvc
 
 import StdlibUnittest
 import Methods
@@ -34,15 +31,15 @@ CxxMethodTestSuite.test("(Int, Int) -> Int") {
 CxxMethodTestSuite.test("(NonTrivialInWrapper, NonTrivialInWrapper) -> Int") {
   var instance = HasMethods()
 
-  expectEqual(42, instance.nonConstSum(NonTrivialInWrapper(value: 40), NonTrivialInWrapper(value: 2)))
-  expectEqual(42, instance.constSum(NonTrivialInWrapper(value: 40), NonTrivialInWrapper(value: 2)))
+  expectEqual(42, instance.nonConstSum(NonTrivialInWrapper(40), NonTrivialInWrapper(2)))
+  expectEqual(42, instance.constSum(NonTrivialInWrapper(40), NonTrivialInWrapper(2)))
 }
 
 CxxMethodTestSuite.test("(NonTrivialInWrapper, NonTrivialInWrapper) -> NonTrivialInWrapper") {
   var instance = HasMethods()
 
-  expectEqual(42, instance.nonConstSumAsWrapper(NonTrivialInWrapper(value: 40), NonTrivialInWrapper(value: 2)).value)
-  expectEqual(42, instance.constSumAsWrapper(NonTrivialInWrapper(value: 40), NonTrivialInWrapper(value: 2)).value)
+  expectEqual(42, instance.nonConstSumAsWrapper(NonTrivialInWrapper(40), NonTrivialInWrapper(2)).value)
+  expectEqual(42, instance.constSumAsWrapper(NonTrivialInWrapper(40), NonTrivialInWrapper(2)).value)
 }
 
 CxxMethodTestSuite.test("(Int) -> NonTrivialInWrapper") {

diff --git a/test/Interop/Cxx/exceptions/trap-on-exception-irgen-itanium.swift b/test/Interop/Cxx/exceptions/trap-on-exception-irgen-itanium.swift
@@ -92,6 +92,11 @@ public:
 class ClassWithDestructor {
   int m = 0;
 public:
+#if __is_target_os(windows)
+  // On windows, force this type to be address-only.
+  inline ClassWithDestructor() noexcept {}
+  inline ClassWithDestructor(const ClassWithDestructor &other) noexcept : m(other.m)  {}
+#endif
   inline ~ClassWithDestructor() {
     (void)freeFunctionNoThrow(0);
   }
@@ -100,6 +105,11 @@ public:
 class ClassWithThrowingDestructor {
   int m = 0;
 public:
+#if __is_target_os(windows)
+  // On windows, force this type to be address-only.
+  inline ClassWithThrowingDestructor() noexcept {}
+  inline ClassWithThrowingDestructor(const ClassWithThrowingDestructor &other) noexcept : m(other.m) {}
+#endif
   inline ~ClassWithThrowingDestructor() noexcept(false) {
     throw 2;
   }
@@ -139,6 +149,8 @@ struct StructWithDefaultConstructor {
 
 
 struct NonTrivial {
+  NonTrivial() noexcept;
+  NonTrivial(const NonTrivial &other) noexcept;
   ~NonTrivial() {}
 };