[cxx-interop] Windows: unify address-only logic and mark non-trivial loadable C++ types as unavailable

Windows logic for determining address-only type layout for a C++ type is now unified with other platforms.
However, this means that on Windows, a C++ type with a custom destructor, but a default copy constructor
is now loadable, even though it's non-trivial. Since Swift does not support such type operations at the
moment (it can't be yet destroyed), mark such type as unavailable in Swift instead, when building for
the Windows target.

This fixes the Windows miscompilation related to such types when they were passed indirectly to C++
functions even though they're actually passed directly.

Author: hyp

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;

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; }
 };
 

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; }

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

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() {}
 };
 

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 {

test/Interop/Cxx/class/destructors-correct-abi-irgen.swift

@@ -6,8 +6,9 @@ import Destructors
 // CHECK: [[H:%.*]] = alloca %TSo33HasUserProvidedDestructorAndDummyV
 // 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)
 }

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;

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

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() {}
 };
 

test/Interop/Cxx/value-witness-table/Inputs/custom-destructors.h

@@ -3,6 +3,12 @@
 
 struct __attribute__((swift_attr("import_unsafe"))) HasUserProvidedDestructor {
   int *value;
+  HasUserProvidedDestructor() {}
+  HasUserProvidedDestructor(int *value) : value(value) {}
+#if __is_target_os(windows) && !defined(WIN_TRIVIAL)
+  // On windows, force this type to be address-only.
+  HasUserProvidedDestructor(const HasUserProvidedDestructor &other) : value(other.value) {}
+#endif
   ~HasUserProvidedDestructor() { *value = 42; }
 };
 
@@ -71,6 +77,11 @@ struct DummyStruct {};
 struct __attribute__((swift_attr("import_unsafe")))
 HasUserProvidedDestructorAndDummy {
   DummyStruct dummy;
+  HasUserProvidedDestructorAndDummy(DummyStruct dummy) : dummy(dummy) {}
+#if __is_target_os(windows) && !defined(WIN_TRIVIAL)
+  // On windows, force this type to be address-only.
+  HasUserProvidedDestructorAndDummy(const HasUserProvidedDestructorAndDummy &other) : dummy(other.dummy) {}
+#endif
   ~HasUserProvidedDestructorAndDummy() {}
 };
 

test/Interop/Cxx/value-witness-table/custom-destructors-non-virtual-irgen.swift

@@ -6,7 +6,7 @@
 import CustomDestructor
 
 protocol InitWithDummy {
-  init(dummy: DummyStruct)
+  init(_: DummyStruct)
 }
 
 extension HasUserProvidedDestructorAndDummy : InitWithDummy { }
@@ -25,7 +25,7 @@ extension HasUserProvidedDestructorAndDummy : InitWithDummy { }
 // CHECK-LABEL: define {{.*}}@{{_ZN33HasUserProvidedDestructorAndDummyD(1|2)Ev|"\?\?1HasUserProvidedDestructorAndDummy@@QEAA@XZ"}}
 // CHECK: ret
 public func testHasUserProvidedDestructorAndDummy() {
-  _ = HasUserProvidedDestructorAndDummy(dummy: DummyStruct())
+  _ = HasUserProvidedDestructorAndDummy(DummyStruct())
 }
 
 // CHECK-LABEL: define {{.*}}void @"$s4main26testHasDefaultedDestructoryyF"

test/Interop/Cxx/value-witness-table/custom-destructors-non-virtual.swift

@@ -9,7 +9,7 @@ import StdlibUnittest
 var CXXDestructorTestSuite = TestSuite("CXXDestructor")
 
 protocol InitWithPtr {
-  init(value: UnsafeMutablePointer<Int32>!)
+  init(_: UnsafeMutablePointer<Int32>!)
 }
 
 extension HasUserProvidedDestructor : InitWithPtr { }
@@ -26,30 +26,30 @@ extension HasEmptyDestructorAndMemberWithUserDefinedConstructor
 func withCxxDestructorSideEffects<T>(_ _: inout T) { }
 
 func createTypeWithUserProvidedDestructor(_ ptr: UnsafeMutablePointer<Int32>) {
-  var obj = HasUserProvidedDestructor(value: ptr)
+  var obj = HasUserProvidedDestructor(ptr)
   withCxxDestructorSideEffects(&obj)
 }
 
 func createTypeWithEmptyDestructorAndMemberWithUserDefinedConstructor(
   _ ptr: UnsafeMutablePointer<Int32>
 ) {
-  let member = HasUserProvidedDestructor(value: ptr)
+  let member = HasUserProvidedDestructor(ptr)
   var obj = HasEmptyDestructorAndMemberWithUserDefinedConstructor(member: member)
   withCxxDestructorSideEffects(&obj)
 }
 
 func createTypeWithNonTrivialImplicitDestructor(
   _ ptr: UnsafeMutablePointer<Int32>
 ) {
-  let member = HasUserProvidedDestructor(value: ptr)
+  let member = HasUserProvidedDestructor(ptr)
   var obj = HasNonTrivialImplicitDestructor(member: member)
   withCxxDestructorSideEffects(&obj)
 }
 
 func createTypeWithNonTrivialDefaultDestructor(
   _ ptr: UnsafeMutablePointer<Int32>
 ) {
-  let member = HasUserProvidedDestructor(value: ptr)
+  let member = HasUserProvidedDestructor(ptr)
   var obj = HasNonTrivialDefaultedDestructor(member: member)
   withCxxDestructorSideEffects(&obj)
 }
@@ -58,15 +58,15 @@ func createTypeWithGeneric<T : InitWithPtr>(
   _ ptr: UnsafeMutablePointer<Int32>,
   type: T.Type
 ) {
-  var obj = T(value: ptr)
+  var obj = T(ptr)
   withCxxDestructorSideEffects(&obj)
 }
 
 func createTypeWithProtocol(
   _ ptr: UnsafeMutablePointer<Int32>,
   type: InitWithPtr.Type
 ) {
-  var obj = type.self.init(value: ptr)
+  var obj = type.self.init(ptr)
   withCxxDestructorSideEffects(&obj)
 }
 
@@ -75,7 +75,7 @@ func createTypeWithProtocol(
   type: InitWithPtr.Type,
   holder: InitWithMember.Type
 ) {
-  let member = type.self.init(value: ptr)
+  let member = type.self.init(ptr)
   var obj = holder.self.init(member: member as! HasUserProvidedDestructor)
   withCxxDestructorSideEffects(&obj)
 }

test/Interop/Cxx/value-witness-table/custom-destructors-typechecker-trivial-windows-abi.swift

@@ -0,0 +1,10 @@
+// RUN: %target-typecheck-verify-swift -I %S/Inputs -enable-experimental-cxx-interop -Xcc -DWIN_TRIVIAL
+
+// REQUIRES: OS=windows-msvc
+
+import CustomDestructor
+
+_ = HasUserProvidedDestructor() // expected-error {{non-trivial C++ class with trivial ABI is not yet available in Swift}}
+_ = HasEmptyDestructorAndMemberWithUserDefinedConstructor() // expected-error {{non-trivial C++ class with trivial ABI is not yet available in Swift}}
+_ = HasNonTrivialImplicitDestructor() // expected-error {{non-trivial C++ class with trivial ABI is not yet available in Swift}}
+_ = HasNonTrivialDefaultedDestructor() // expected-error {{non-trivial C++ class with trivial ABI is not yet available in Swift}}

test/Interop/CxxToSwiftToCxx/bridge-cxx-struct-back-to-cxx.swift

@@ -37,7 +37,7 @@ namespace ns {
         T x;
 
         NonTrivialTemplate();
-        NonTrivialTemplate(const NonTrivialTemplate<T> &) = default;
+        NonTrivialTemplate(const NonTrivialTemplate<T> &other) : x(other.x) {}
         NonTrivialTemplate(NonTrivialTemplate<T> &&) = default;
         ~NonTrivialTemplate() {}
     };
@@ -46,6 +46,8 @@ namespace ns {
 
     struct NonTrivialImplicitMove {
         NonTrivialTemplate<int> member;
+
+        NonTrivialImplicitMove(const NonTrivialImplicitMove &other) : member(other.member) {}
     };
 
     #define IMMORTAL_REF                                \

test/SILOptimizer/Inputs/CXXTypesWithUserProvidedDestructor.h

@@ -2,16 +2,20 @@
 #define TEST_SIL_OPTIMIZER_CXX_WITH_CUSTOM_DESTRUCTOR_H
 
 struct HasUserProvidedDestructor {
-  int x;
+  int x = 0;
+
+  HasUserProvidedDestructor(const HasUserProvidedDestructor &other) : x(other.x) {}
   ~HasUserProvidedDestructor() {}
 };
 
 struct Loadable {
-  int x;
+  int x = 0;
 };
 
 struct HasMemberWithUserProvidedDestructor {
   Loadable y;
+
+  HasMemberWithUserProvidedDestructor(const HasMemberWithUserProvidedDestructor &other) : y(other.y) {}
   ~HasMemberWithUserProvidedDestructor() {}
 };