Merge pull request #73640 from kavon/6.0-use-newer-cxx-moveonly-import-by-default

[6.0🍒] CxxInterop: Use newer cxx moveonly import by default

Author: kavon

SwiftCompilerSources/CMakeLists.txt

@@ -120,6 +120,11 @@ function(add_swift_compiler_modules_library name)
         "-Xfrontend" "${SWIFT_MIN_RUNTIME_VERSION}")
     endif()
     list(APPEND swift_compile_options "-Xfrontend" "-disable-implicit-string-processing-module-import")
+
+    # We cannot use Unsafe*Pointer when importing C++ move-only types until the
+    # host libraries are updated to Swift 6.0, because that importing strategy
+    # requires _Pointer have its Pointee: ~Copyable. (rdar://128013193)
+    list(APPEND swift_compile_options "-Xfrontend" "-cxx-interop-use-opaque-pointer-for-moveonly")
   endif()
 
   if(CMAKE_BUILD_TYPE STREQUAL "Debug")

include/swift/Basic/LangOptions.h

@@ -328,6 +328,9 @@ namespace swift {
     /// when importing Swift modules that enable C++ interoperability.
     bool RequireCxxInteropToImportCxxInteropModule = true;
 
+    // Workaround for bug when building SwiftCompilerSources (rdar://128013193)
+    bool CxxInteropUseOpaquePointerForMoveOnly = false;
+
     /// On Darwin platforms, use the pre-stable ABI's mark bit for Swift
     /// classes instead of the stable ABI's bit. This is needed when
     /// targeting OSes prior to macOS 10.14.4 and iOS 12.2, where

include/swift/Option/FrontendOptions.td

@@ -935,6 +935,11 @@ def cxx_interop_disable_requirement_at_import :
   HelpText<"Do not require C++ interoperability to be enabled when importing a Swift module that enables C++ interoperability">,
   Flags<[FrontendOption, HelpHidden]>;
 
+def cxx_interop_use_opaque_pointer_for_moveonly :
+  Flag<["-"], "cxx-interop-use-opaque-pointer-for-moveonly">,
+  HelpText<"Testing flag that will be eliminated soon. Do not use.">,
+  Flags<[FrontendOption, HelpHidden]>;
+
 def use_malloc : Flag<["-"], "use-malloc">,
   HelpText<"Allocate internal data structures using malloc "
            "(for memory debugging)">;

lib/ClangImporter/ClangImporter.cpp

@@ -5778,16 +5778,9 @@ 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, we cannot synthesize the accessor,
-    // because its implementation would use `UnsafePointer<BaseTy>`, and that
-    // triggers a bug when building SwiftCompilerSources. (rdar://128013193)
-    //
-    // 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>())
+    // FIXME: this is a workaround for rdar://128013193
+    if (oldTypeDecl->getAttrs().hasAttribute<MoveOnlyAttr>() &&
+        context.LangOpts.CxxInteropUseOpaquePointerForMoveOnly)
       return nullptr;
 
     auto rawMemory = allocateMemoryForDecl<VarDecl>(var->getASTContext(),

lib/ClangImporter/ImportType.cpp

@@ -509,19 +509,12 @@ namespace {
         return importFunctionPointerLikeType(*type, pointeeType);
       }
 
-      // We cannot specify UnsafePointer<T> with a non-copyable type T just yet,
-      // because it triggers a bug when building SwiftCompilerSources.
-      // (rdar://128013193)
-      //
-      // 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.
+      // FIXME: this is a workaround for rdar://128013193
       if (pointeeType && pointeeType->getAnyNominal() &&
           pointeeType->getAnyNominal()
               ->getAttrs()
-              .hasAttribute<MoveOnlyAttr>()) {
+              .hasAttribute<MoveOnlyAttr>() &&
+          Impl.SwiftContext.LangOpts.CxxInteropUseOpaquePointerForMoveOnly) {
         auto opaquePointerDecl = Impl.SwiftContext.getOpaquePointerDecl();
         if (!opaquePointerDecl)
           return Type();

lib/Frontend/CompilerInvocation.cpp

@@ -1267,6 +1267,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   Opts.CxxInteropGettersSettersAsProperties = Args.hasArg(OPT_cxx_interop_getters_setters_as_properties);
   Opts.RequireCxxInteropToImportCxxInteropModule =
       !Args.hasArg(OPT_cxx_interop_disable_requirement_at_import);
+  Opts.CxxInteropUseOpaquePointerForMoveOnly =
+      Args.hasArg(OPT_cxx_interop_use_opaque_pointer_for_moveonly);
 
   Opts.VerifyAllSubstitutionMaps |= Args.hasArg(OPT_verify_all_substitution_maps);
 

test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift

@@ -3,8 +3,6 @@
 
 // REQUIRES: rdar128013193
 
-// REQUIRES: rdar128013193
-
 import MoveOnlyCxxValueType
 
 func testGetX() -> CInt {

test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift

@@ -3,8 +3,6 @@
 
 // REQUIRES: rdar128013193
 
-// REQUIRES: rdar128013193
-
 import MoveOnlyCxxValueType
 
 func testGetX() -> CInt {

test/Interop/Cxx/class/move-only/move-only-cxx-value-type-generics.swift

@@ -2,7 +2,6 @@
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -O -Xfrontend -sil-verify-none)
 //
 // REQUIRES: executable_test
-// REQUIRES: GH_ISSUE_70246
 
 
 

test/Interop/Cxx/class/move-only/move-only-cxx-value-type-module-interface.swift

@@ -1,8 +1,4 @@
-// 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
+// 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: func getNonCopyablePtr() -> OpaquePointer
-// CHECK: func getNonCopyableDerivedPtr() -> OpaquePointer
-
-// FIXME: would prefer to have this (rdar://128013193)
-// func getNonCopyablePtr() -> UnsafeMutablePointer<NonCopyable>
-// func getNonCopyableDerivedPtr() -> UnsafeMutablePointer<NonCopyableDerived>
+// CHECK: func getNonCopyablePtr() -> UnsafeMutablePointer<NonCopyable>
+// CHECK: func getNonCopyableDerivedPtr() -> UnsafeMutablePointer<NonCopyableDerived>

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)
-// -- FIXME: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -D HAS_RDAR_128013193)
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -D HAS_NONCOPYABLE_GENERICS)
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-5.9 -O)
 // 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 -D HAS_NONCOPYABLE_GENERICS)
 
 // REQUIRES: executable_test
 
@@ -28,7 +29,7 @@ MoveOnlyCxxValueType.test("Test derived move only type member access") {
   var k = c.method(-3)
   expectEqual(k, -6)
   expectEqual(c.method(1), 2)
-#if HAS_RDAR_128013193
+#if HAS_NONCOPYABLE_GENERICS
   k = c.x
   expectEqual(k, 2)
   c.x = 11
@@ -59,7 +60,7 @@ MoveOnlyCxxValueType.test("Test move only field access in holder") {
   expectEqual(c.x.x, 5)
 }
 
-#if HAS_RDAR_128013193
+#if HAS_NONCOPYABLE_GENERICS
 MoveOnlyCxxValueType.test("Test move only field access in derived holder") {
   var c = NonCopyableHolderDerivedDerived(-11)
   var k = borrowNC(c.x)

test/Interop/Cxx/operators/move-only/move-only-synthesized-properties.swift

@@ -1,11 +1,11 @@
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-5.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)
-// -- FIXME: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -D HAS_RDAR_128013193)
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -D HAS_NONCOPYABLE_GENERICS)
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=swift-5.9 -O)
 // 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)
-// -- FIXME: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -O -D HAS_RDAR_128013193)
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -cxx-interoperability-mode=upcoming-swift -O -D HAS_NONCOPYABLE_GENERICS)
 //
 // REQUIRES: executable_test
 
@@ -92,7 +92,7 @@ MoveOnlyCxxOperators.test("testNonCopyableHolderValueMutDeref pointee value") {
   expectEqual(k.x, k2.x)
 }
 
-#if HAS_RDAR_128013193
+#if HAS_NONCOPYABLE_GENERICS
 MoveOnlyCxxOperators.test("NonCopyableHolderConstDerefDerivedDerived pointee borrow") {
   let holder = NonCopyableHolderConstDerefDerivedDerived(11)
   var k = borrowNC(holder.pointee)

test/SILOptimizer/moveonly_optional_force_unwrap.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -enable-experimental-feature NoncopyableGenerics -emit-sil -verify %s
+// RUN: %target-swift-frontend -emit-sil -verify %s
 
 struct NC: ~Copyable {
     borrowing func borrow() {}