Merge pull request #73055 from apple/egorzhdan/ncg-unsafepointer

egorzhdan · web-flow · commit 70c3e432f6db · 2024-04-24T00:45:25.000+01:00
[cxx-interop] Import `NonCopyable*` as `OpaquePointer` if noncopyable generics are disabled
diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp
@@ -5728,6 +5728,8 @@ DeclAttributes cloneImportedAttributes(ValueDecl *decl, ASTContext &context) {
 
 static ValueDecl *
 cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
+  ASTContext &context = decl->getASTContext();
+
   if (auto fn = dyn_cast<FuncDecl>(decl)) {
     // TODO: function templates are specialized during type checking so to
     // support these we need to tell Swift to type check the synthesized bodies.
@@ -5745,7 +5747,6 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
         return nullptr;
     }
 
-    ASTContext &context = decl->getASTContext();
     auto out = FuncDecl::createImplicit(
         context, fn->getStaticSpelling(), fn->getName(),
         fn->getNameLoc(), fn->hasAsync(), fn->hasThrows(),
@@ -5781,6 +5782,20 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
   }
 
   if (auto var = dyn_cast<VarDecl>(decl)) {
+    auto oldContext = var->getDeclContext();
+    auto oldTypeDecl = oldContext->getSelfNominalTypeDecl();
+    // If the base type is non-copyable, and non-copyable generics are disabled,
+    // we cannot synthesize the accessor, because its implementation would use
+    // `UnsafePointer<BaseTy>`.
+    // We cannot use `ty->isNoncopyable()` here because that would create a
+    // cyclic dependency between ModuleQualifiedLookupRequest and
+    // LookupConformanceInModuleRequest, so we check for the presence of
+    // move-only attribute that is implicitly added to non-copyable C++ types by
+    // ClangImporter.
+    if (oldTypeDecl->getAttrs().hasAttribute<MoveOnlyAttr>() &&
+        !context.LangOpts.hasFeature(Feature::NoncopyableGenerics))
+      return nullptr;
+
     auto rawMemory = allocateMemoryForDecl<VarDecl>(var->getASTContext(),
                                                     sizeof(VarDecl), false);
     auto out =
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
@@ -509,6 +509,26 @@ namespace {
         return importFunctionPointerLikeType(*type, pointeeType);
       }
 
+      // If non-copyable generics are disabled, we cannot specify
+      // UnsafePointer<T> with a non-copyable type T.
+      // We cannot use `ty->isNoncopyable()` here because that would create a
+      // cyclic dependency between ModuleQualifiedLookupRequest and
+      // LookupConformanceInModuleRequest, so we check for the presence of
+      // move-only attribute that is implicitly added to non-copyable C++ types
+      // by ClangImporter.
+      if (pointeeType && pointeeType->getAnyNominal() &&
+          pointeeType->getAnyNominal()
+              ->getAttrs()
+              .hasAttribute<MoveOnlyAttr>() &&
+          !Impl.SwiftContext.LangOpts.hasFeature(
+              Feature::NoncopyableGenerics)) {
+        auto opaquePointerDecl = Impl.SwiftContext.getOpaquePointerDecl();
+        if (!opaquePointerDecl)
+          return Type();
+        return {opaquePointerDecl->getDeclaredInterfaceType(),
+                ImportHint::OtherPointer};
+      }
+
       PointerTypeKind pointerKind;
       if (quals.hasConst()) {
         pointerKind = PTK_UnsafePointer;
diff --git a/test/Interop/Cxx/class/move-only/Inputs/move-only-cxx-value-type.h b/test/Interop/Cxx/class/move-only/Inputs/move-only-cxx-value-type.h
@@ -53,4 +53,7 @@ struct NonCopyableHolderDerivedDerived: NonCopyableHolderDerived {
     }
 };
 
+inline NonCopyable *getNonCopyablePtr() { return nullptr; }
+inline NonCopyableDerived *getNonCopyableDerivedPtr() { return nullptr; }
+
 #endif // TEST_INTEROP_CXX_CLASS_MOVE_ONLY_VT_H
diff --git a/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift b/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift
@@ -1,5 +1,5 @@
-// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=swift-6 %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
-// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=upcoming-swift %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
+// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=swift-6 -enable-experimental-feature NoncopyableGenerics %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
+// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature NoncopyableGenerics %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s
 
 import MoveOnlyCxxValueType
 
diff --git a/test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift b/test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift
@@ -1,5 +1,5 @@
-// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=swift-6 %s -validate-tbd-against-ir=none | %FileCheck %s
-// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=upcoming-swift %s -validate-tbd-against-ir=none | %FileCheck %s
+// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=swift-6 -enable-experimental-feature NoncopyableGenerics %s -validate-tbd-against-ir=none | %FileCheck %s
+// RUN: %target-swift-emit-sil -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature NoncopyableGenerics %s -validate-tbd-against-ir=none | %FileCheck %s
 
 import MoveOnlyCxxValueType
 
diff --git a/test/Interop/Cxx/class/move-only/move-only-cxx-value-type-module-interface.swift b/test/Interop/Cxx/class/move-only/move-only-cxx-value-type-module-interface.swift
@@ -0,0 +1,8 @@
+// RUN: %target-swift-ide-test -print-module -module-to-print=MoveOnlyCxxValueType -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -source-filename=x | %FileCheck %s --check-prefix=CHECK-NO-NCG
+// RUN: %target-swift-ide-test -print-module -module-to-print=MoveOnlyCxxValueType -I %S/Inputs -cxx-interoperability-mode=upcoming-swift -source-filename=x -enable-experimental-feature NoncopyableGenerics | %FileCheck %s --check-prefix=CHECK-NCG
+
+// CHECK-NO-NCG: func getNonCopyablePtr() -> OpaquePointer
+// CHECK-NO-NCG: func getNonCopyableDerivedPtr() -> OpaquePointer
+
+// CHECK-NCG: func getNonCopyablePtr() -> UnsafeMutablePointer<NonCopyable>
+// CHECK-NCG: func getNonCopyableDerivedPtr() -> UnsafeMutablePointer<NonCopyableDerived>
diff --git a/test/Interop/Cxx/class/move-only/move-only-cxx-value-type.swift b/test/Interop/Cxx/class/move-only/move-only-cxx-value-type.swift
@@ -1,7 +1,8 @@
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift)
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature NoncopyableGenerics -D HAS_NONCOPYABLE_GENERICS)
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-6 -O)
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-6 -O -enable-experimental-feature NoncopyableGenerics -D HAS_NONCOPYABLE_GENERICS)
 
-//
 // REQUIRES: executable_test
 
 import MoveOnlyCxxValueType
@@ -27,10 +28,12 @@ MoveOnlyCxxValueType.test("Test derived move only type member access") {
   var k = c.method(-3)
   expectEqual(k, -6)
   expectEqual(c.method(1), 2)
+#if HAS_NONCOPYABLE_GENERICS
   k = c.x
   expectEqual(k, 2)
   c.x = 11
   expectEqual(c.x, 11)
+#endif
   k = c.mutMethod(-13)
   expectEqual(k, -13)
 }
@@ -56,6 +59,7 @@ MoveOnlyCxxValueType.test("Test move only field access in holder") {
   expectEqual(c.x.x, 5)
 }
 
+#if HAS_NONCOPYABLE_GENERICS
 MoveOnlyCxxValueType.test("Test move only field access in derived holder") {
   var c = NonCopyableHolderDerivedDerived(-11)
   var k = borrowNC(c.x)
@@ -69,5 +73,6 @@ MoveOnlyCxxValueType.test("Test move only field access in derived holder") {
   c.x.mutMethod(5)
   expectEqual(c.x.x, 5)
 }
+#endif
 
 runAllTests()
diff --git a/test/Interop/Cxx/operators/move-only/move-only-synthesized-properties.swift b/test/Interop/Cxx/operators/move-only/move-only-synthesized-properties.swift
@@ -1,7 +1,9 @@
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-6)
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift)
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature NoncopyableGenerics -D HAS_NONCOPYABLE_GENERICS)
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-6 -O)
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -O)
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -O -enable-experimental-feature NoncopyableGenerics -D HAS_NONCOPYABLE_GENERICS)
 //
 // REQUIRES: executable_test
 
@@ -88,6 +90,7 @@ MoveOnlyCxxOperators.test("testNonCopyableHolderValueMutDeref pointee value") {
   expectEqual(k.x, k2.x)
 }
 
+#if HAS_NONCOPYABLE_GENERICS
 MoveOnlyCxxOperators.test("NonCopyableHolderConstDerefDerivedDerived pointee borrow") {
   let holder = NonCopyableHolderConstDerefDerivedDerived(11)
   var k = borrowNC(holder.pointee)
@@ -155,5 +158,6 @@ MoveOnlyCxxOperators.test("testNonCopyableHolderValueMutDerefDerivedDerived poin
   var k2 = holder.pointee
   expectEqual(k.x, k2.x)
 }
+#endif
 
 runAllTests()

Original file line number	Diff line number	Diff line change
`@@ -53,4 +53,7 @@ struct NonCopyableHolderDerivedDerived: NonCopyableHolderDerived {`
`53`	`53`	`}`
`54`	`54`	`};`
`55`	`55`
	`56`	`+inline NonCopyable *getNonCopyablePtr() { return nullptr; }`
	`57`	`+inline NonCopyableDerived *getNonCopyableDerivedPtr() { return nullptr; }`
	`58`	`+`
`56`	`59`	`#endif // TEST_INTEROP_CXX_CLASS_MOVE_ONLY_VT_H`