[cxx-interop] Itanium ABI C++ records should have address-only layout when they can't be passed in registers

lib/ClangImporter/ImportDecl.cpp

@@ -2427,15 +2427,39 @@ 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(
-              isNonTrivialForPurposeOfCalls(cxxRecordDecl));
+              isAddressOnlySwiftStruct(cxxRecordDecl));
 
         for (auto &getterAndSetter : Impl.GetterSetterMap[result]) {
           auto getter = getterAndSetter.second.first;
@@ -2686,6 +2710,17 @@ namespace {
       if (!result)
         return nullptr;
 
+      if (decl->hasAttr<clang::TrivialABIAttr>()) {
+        // We cannot yet represent trivial_abi C++ records in Swift.
+        // Clang tells us such type can be passed in registers, so
+        // we avoid using AddressOnly type-layout for such type, which means
+        // that it then does not use C++'s copy and destroy semantics from
+        // Swift.
+        Impl.markUnavailable(cast<ValueDecl>(result),
+                             "C++ classes with `trivial_abi` Clang attribute "
+                             "are not yet available in Swift");
+      }
+
       if (auto classDecl = dyn_cast<ClassDecl>(result)) {
         validateForeignReferenceType(decl, classDecl);
 

lib/SIL/IR/SILFunctionType.cpp

@@ -3137,6 +3137,12 @@ getIndirectCParameterConvention(const clang::ParmVarDecl *param) {
 ///
 /// Generally, whether the parameter is +1 is handled before this.
 static ParameterConvention getDirectCParameterConvention(clang::QualType type) {
+  if (auto *cxxRecord = type->getAsCXXRecordDecl()) {
+    // Directly passed non-trivially destroyed C++ record is consumed by the
+    // callee.
+    if (!cxxRecord->hasTrivialDestructor())
+      return ParameterConvention::Direct_Owned;
+  }
   return ParameterConvention::Direct_Unowned;
 }
 

test/Interop/Cxx/class/clang-trivial-abi.swift

@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %target-swift-ide-test -print-module -module-to-print=Test -I %t/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
+// RUN: %target-swift-frontend -typecheck -I %t/Inputs  %t/test.swift  -enable-experimental-cxx-interop -verify
+
+//--- Inputs/module.modulemap
+module Test {
+    header "test.h"
+    requires cplusplus
+}
+
+//--- Inputs/test.h
+
+class TrivialABIRecord {
+    int x = 0;
+public:
+    TrivialABIRecord() {}
+    ~TrivialABIRecord() {
+    }
+}
+__attribute__((trivial_abi));
+
+// CHECK: @available(*, unavailable, message: "C++ classes with `trivial_abi` Clang attribute are not yet available in Swift")
+// CHECK-NEXT: struct TrivialABIRecord {
+
+//--- test.swift
+
+import Test
+
+func test() {
+    let _ = TrivialABIRecord() // expected-error{{'TrivialABIRecord' is unavailable: C++ classes with `trivial_abi` Clang attribute are not yet available in Swift}}
+}

test/Interop/Cxx/objc-correctness/Inputs/cxx-class-with-arc-fields-ctor.h

@@ -5,10 +5,41 @@ struct S {
   NSString *_Nullable B;
   NSString *_Nullable C;
 
+#ifdef S_NONTRIVIAL_DESTRUCTOR
+  ~S() {}
+#endif
+
   void dump() const {
     printf("%s\n", [A UTF8String]);
     printf("%s\n", [B UTF8String]);
     printf("%s\n", [C UTF8String]);
   }
 };
 
+inline void takeSFunc(S s) {
+  s.dump();
+}
+
+struct NonTrivialLogDestructor {
+    int x = 0;
+
+    ~NonTrivialLogDestructor() {
+        printf("~NonTrivialLogDestructor %d\n", x);
+    }
+};
+
+@interface ClassWithNonTrivialDestructorIvar: NSObject
+
+- (ClassWithNonTrivialDestructorIvar * _Nonnull)init;
+
+- (void)takesS:(S)s;
+
+@end
+
+struct ReferenceStructToClassWithNonTrivialLogDestructorIvar {
+    ClassWithNonTrivialDestructorIvar *_Nonnull x;
+
+#ifdef S_NONTRIVIAL_DESTRUCTOR
+    ~ReferenceStructToClassWithNonTrivialLogDestructorIvar() {}
+#endif
+};

test/Interop/Cxx/objc-correctness/Inputs/objc-class-with-non-trivial-cxx-record.mm

@@ -0,0 +1,18 @@
+#import <Foundation/Foundation.h>
+#import "cxx-class-with-arc-fields-ctor.h"
+
+@implementation ClassWithNonTrivialDestructorIvar {
+    NonTrivialLogDestructor value;
+};
+
+- (ClassWithNonTrivialDestructorIvar *)init {
+    self->value.x = 21;
+    return self;
+}
+
+- (void)takesS:(S)s {
+    printf("takesS!\n");
+    s.dump();
+}
+
+@end

test/Interop/Cxx/objc-correctness/call-to-generated-init-with-nsstring.swift

@@ -1,6 +1,8 @@
 // RUN: %target-swift-ide-test -print-module -module-to-print=CxxClassWithNSStringInit -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop -enable-objc-interop | %FileCheck -check-prefix=CHECK-IDE-TEST %s
-// RUN: %target-swift-frontend -I %S/Inputs -enable-experimental-cxx-interop -emit-ir %s -Xcc -fignore-exceptions | %FileCheck %s
-
+// RUN: %target-swift-frontend -I %S/Inputs -enable-experimental-cxx-interop -emit-sil %s -Xcc -fignore-exceptions | %FileCheck --check-prefix=SIL-TRIVIAL %s
+// RUN: %target-swift-frontend -I %S/Inputs -enable-experimental-cxx-interop -emit-sil %s -Xcc -fignore-exceptions -Xcc -DS_NONTRIVIAL_DESTRUCTOR | %FileCheck --check-prefix=SIL-NONTRIVIAL %s
+// RUN: %target-swift-frontend -I %S/Inputs -enable-experimental-cxx-interop -emit-ir %s -Xcc -fignore-exceptions | %FileCheck --check-prefix=IR-TRIVIAL %s
+// RUN: %target-swift-frontend -I %S/Inputs -enable-experimental-cxx-interop -emit-ir %s -Xcc -fignore-exceptions -Xcc -DS_NONTRIVIAL_DESTRUCTOR | %FileCheck --check-prefix=IR-NONTRIVIAL %s
 
 // REQUIRES: objc_interop
 
@@ -15,11 +17,45 @@ import CxxClassWithNSStringInit
 // CHECK-IDE-TEST:   var C: NSString?
 // CHECK-IDE-TEST: }
 
-var foo: NSString? = "foo"
-var bar: NSString? = "bar"
-var baz: NSString? = "baz"
-var s = S(A: foo, B: bar, C: baz)
-s.dump()
+func testSdump() {
+  var foo: NSString? = "foo"
+  var bar: NSString? = "bar"
+  var baz: NSString? = "baz"
+  var s = S(A: foo, B: bar, C: baz)
+  s.dump()
+  ClassWithNonTrivialDestructorIvar().takesS(s)
+  takeSFunc(s)
+}
+
+testSdump()
+
+// SIL-TRIVIAL:   function_ref @_ZNK1S4dumpEv : $@convention(cxx_method) (@in_guaranteed S) -> ()
+// SIL-TRIVIAL-NEXT:   apply %{{.*}}(%{{.*}}) : $@convention(cxx_method) (@in_guaranteed S) -> ()
+// SIL-TRIVIAL:      $@convention(objc_method) (@owned S, ClassWithNonTrivialDestructorIvar) -> ()
+// SIL-TRIVIAL-NEXT: apply %{{.*}}(%{{.*}}) : $@convention(objc_method) (@owned S, ClassWithNonTrivialDestructorIvar) -> ()
+// SIL-TRIVIAL: function_ref @_Z9takeSFunc : $@convention(c) (@owned S) -> ()
+// SIL-TRIVIAL-NEXT: apply %{{.*}}(%{{.*}}) : $@convention(c) (@owned S) -> ()
+
+// SIL-NONTRIVIAL:   function_ref @_ZNK1S4dumpEv : $@convention(cxx_method) (@in_guaranteed S) -> ()
+// SIL-NONTRIVIAL-NEXT:   apply %{{.*}}(%{{.*}}) : $@convention(cxx_method) (@in_guaranteed S) -> ()
+// SIL-NONTRIVIAL:      $@convention(objc_method) (@in S, ClassWithNonTrivialDestructorIvar) -> ()
+// SIL-NONTRIVIAL-NEXT: apply %{{.*}}(%{{.*}}) : $@convention(objc_method) (@in S, ClassWithNonTrivialDestructorIvar) -> ()
+// SIL-NONTRIVIAL: function_ref @_Z9takeSFunc : $@convention(c) (@in S) -> ()
+// SIL-NONTRIVIAL-NEXT: apply %{{.*}}(%{{.*}}) : $@convention(c) (@in S) -> ()
+
+
+// IR-TRIVIAL-LABEL: define {{.*}} swiftcc void @"$s4main9testSdumpyyF"()
+// IR-TRIVIAL-NOT: @_ZN1SC1ERKS_
+// IR-TRIVIAL: call {{.*}} @_ZNK1S4dumpEv
+// IR-TRIVIAL: call {{.*}} @"$sSo1SVWOh"
+
+// IR-TRIVIAL-LABEL: define linkonce_odr {{.*}} @"$sSo1SVWOh"(
+// IR-TRIVIAL: @llvm.objc.release
+// IR-TRIVIAL: @llvm.objc.release
+// IR-TRIVIAL: @llvm.objc.release
+// IR-TRIVIAL: }
 
-// CHECK: call {{.*}} @_ZN1SC1ERKS_
-// CHECK: call {{.*}} @_ZNK1S4dumpEv
+// IR-NONTRIVIAL-LABEL: define {{.*}} swiftcc void @"$s4main9testSdumpyyF"()
+// IR-NONTRIVIAL: call {{.*}} @_ZN1SC1ERKS_
+// IR-NONTRIVIAL: call {{.*}} @_ZNK1S4dumpEv
+// IR-NONTRIVIAL: call {{.*}} @_ZN1SD1Ev

test/Interop/Cxx/objc-correctness/objcxx-arc-field-in-struct-type-layout-execution.swift

@@ -0,0 +1,64 @@
+// RUN: %empty-directory(%t2)
+
+// RUN: %target-interop-build-clangxx -c %S/Inputs/objc-class-with-non-trivial-cxx-record.mm -o %t2/objc-class-impl.o -fobjc-arc
+
+// RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -enable-experimental-cxx-interop -Xcc -fignore-exceptions -Xlinker %t2/objc-class-impl.o) | %FileCheck %s
+// RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -enable-experimental-cxx-interop -Xcc -fignore-exceptions -O -Xlinker %t2/objc-class-impl.o) | %FileCheck %s
+
+// RUN: %target-interop-build-clangxx -c %S/Inputs/objc-class-with-non-trivial-cxx-record.mm -o %t2/objc-class-impl-non-trivial.o -fobjc-arc -DS_NONTRIVIAL_DESTRUCTOR
+// RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -enable-experimental-cxx-interop -Xcc -fignore-exceptions -Xcc -DS_NONTRIVIAL_DESTRUCTOR -Xlinker %t2/objc-class-impl-non-trivial.o) | %FileCheck %s
+//
+// REQUIRES: executable_test
+// REQUIRES: objc_interop
+
+import Foundation
+import CxxClassWithNSStringInit
+
+func testS() {
+    let copy: S
+    do {
+        let foo: NSString? = "super long string actually allocated"
+        let bar: NSString? = "bar"
+        let baz: NSString? = "baz"
+        var s = S(A: foo, B: bar, C: baz)
+        s.dump()
+        copy = s
+    }
+    print("after scope")
+    copy.dump()
+    print("takeSFunc")
+    takeSFunc(copy)
+}
+
+@inline(never)
+func blackHole<T>(_ x: T) {
+
+}
+
+func testReferenceStructToClassWithNonTrivialLogDestructorIvar() {
+    print("testReferenceStructToClassWithNonTrivialLogDestructorIvar")
+    let m = ReferenceStructToClassWithNonTrivialLogDestructorIvar(x: ClassWithNonTrivialDestructorIvar())
+    m.x.takesS(S(A: "hello world two", B: "bar", C: "baz"))
+    blackHole(m)
+}
+
+testS()
+testReferenceStructToClassWithNonTrivialLogDestructorIvar()
+
+// CHECK: super long string actually allocated
+// CHECK-NEXT: bar
+// CHECK-NEXT: baz
+// CHECK-NEXT: after scope
+// CHECK-NEXT: super long string actually allocated
+// CHECK-NEXT: bar
+// CHECK-NEXT: baz
+// CHECK-NEXT: takeSFunc
+// CHECK-NEXT: super long string actually allocated
+// CHECK-NEXT: bar
+// CHECK-NEXT: baz
+// CHECK-NEXT: testReferenceStructToClassWithNonTrivialLogDestructorIvar
+// CHECK-NEXT: takesS!
+// CHECK-NEXT: hello world two
+// CHECK-NEXT: bar
+// CHECK-NEXT: baz
+// CHECK-NEXT: ~NonTrivialLogDestructor 21