Revert "[6.1][Package CMO] Prevent serializing types from SDK/system modules imported as @_implementationOnly."

This reverts commit 75d8bab.

Author: elsh

lib/SILOptimizer/IPO/CrossModuleOptimization.cpp

@@ -784,26 +784,11 @@ bool CrossModuleOptimization::checkImports(DeclContext *ctxt) const {
   ModuleDecl::ImportFilter filter;
 
   if (isPackageCMOEnabled(M.getSwiftModule())) {
-    // When Package CMO is enabled, types imported with `package import`
+    // If Package CMO is enabled, decls imported with `package import`
     // or `@_spiOnly import` into this module should be allowed to be
-    // serialized. These types may be used in APIs with `package` or
-    // higher access level, with or without `@_spi`, and such APIs should
-    // be serializable to allow direct access by another module if it's
-    // in the same package.
-    //
-    // However, types are from modules imported as `@_implementationOnly`
-    // should not be serialized, even if their defining modules are SDK
-    // or system modules. Since these types are intended to remain hidden
-    // from external clients, their metadata (e.g. field offsets) may be
-    // stripped, making it unavailable for look up at runtime. If serialized,
-    // the client will attempt to use the serialized accessor and fail
-    // because the metadata is missing, leading to a linker error.
-    //
-    // This issue applies to transitively imported types as well;
-    // `@_implementationOnly import Foundation` imports `ObjectiveC`
-    // indirectly, and metadata for types like `NSObject` from `ObjectiveC`
-    // can also be stripped, thus such types should not be allowed for
-    // serialization.
+    // serialized. They are used in decls with `package` or higher
+    // access level, with or without @_spi; a client of this module
+    // should be able to access them directly if in the same package.
     filter = { ModuleDecl::ImportFilterKind::ImplementationOnly };
   } else {
     // See if context is imported in a "regular" way, i.e. not with
@@ -819,8 +804,17 @@ bool CrossModuleOptimization::checkImports(DeclContext *ctxt) const {
 
   auto &imports = M.getSwiftModule()->getASTContext().getImportCache();
   for (auto &desc : results) {
-    if (imports.isImportedBy(moduleOfCtxt, desc.importedModule))
+    if (imports.isImportedBy(moduleOfCtxt, desc.importedModule)) {
+      // E.g. `@_implementationOnly import QuartzCore_Private.CALayerPrivate`
+      // imports `Foundation` as its transitive dependency module; use of a
+      // a `public` decl in `Foundation` such as `IndexSet` in a function
+      // signature should not block serialization in Package CMO given the
+      // function has `package` or higher access level.
+      if (isPackageCMOEnabled(M.getSwiftModule()) &&
+          moduleOfCtxt->isNonUserModule())
+          continue;
       return false;
+    }
   }
   return true;
 }

test/SILOptimizer/package-cmo-import-filter.swift

@@ -1,26 +1,6 @@
 // RUN: %empty-directory(%t)
 // RUN: split-file %s %t
 
-/// 1. Test `@_implementationOnly import`.
-// RUN: %target-build-swift-dylib(%t/%target-library-name(Utils)) \
-// RUN: %t/UtilsA.swift %t/UtilsB.swift \
-// RUN: -module-name Utils -emit-module -package-name Pkg \
-// RUN: -Xfrontend -experimental-package-cmo -Xfrontend -experimental-allow-non-resilient-access \
-// RUN: -enable-library-evolution -O -wmo
-// RUN: %target-sil-opt %t/Utils.swiftmodule -I %t -sil-verify-all -o %t/Utils.sil
-// RUN: %FileCheck %s --check-prefix=CHECK-UTILS < %t/Utils.sil
-
-/// Verify accessing PkgKlass.second from a client in a dynamic context does not cause a linker error.
-// RUN: %target-build-swift -I %t -L %t %t/Client.swift -package-name Pkg \
-// RUN: -O -wmo -enable-library-evolution \
-// RUN: %target-rpath(%t) -lUtils -o %t/a.out
-
-// RUN: %target-swift-frontend -emit-sil -I %t -L %t %t/Client.swift \
-// RUN: -package-name Pkg -O -wmo -enable-library-evolution \
-// RUN: -lUtils -o %t/Client.sil
-// RUN: %FileCheck %s --check-prefix=CHECK-CLIENT < %t/Client.sil
-
-/// 2. Test `package import` and `@_spiOnly import`.
 // RUN: %target-swift-frontend %t/CoreA.swift \
 // RUN: -module-name=CoreA -package-name Pkg \
 // RUN: -parse-as-library -emit-module \
@@ -33,73 +13,32 @@
 // RUN: -emit-module-path %t/CoreB.swiftmodule -I%t \
 // RUN: -O -wmo -enable-library-evolution
 
-// RUN: %target-swift-frontend %t/UI.swift \
-// RUN: -module-name=UI -package-name Pkg \
+// RUN: %target-swift-frontend %t/Lib.swift \
+// RUN: -module-name=Lib -package-name Pkg \
 // RUN: -parse-as-library -emit-module \
 // RUN: -experimental-spi-only-imports \
-// RUN: -emit-module-path %t/UI.swiftmodule -I %t \
+// RUN: -emit-module-path %t/Lib.swiftmodule -I %t \
 // RUN: -experimental-package-cmo -experimental-allow-non-resilient-access \
-// RUN: -O -wmo -enable-library-evolution -Rmodule-loading 2> %t/UI-result.txt
-// RUN: %target-sil-opt %t/UI.swiftmodule -I %t -sil-verify-all -o %t/UI.sil
-// RUN: %FileCheck %s < %t/UI.sil
+// RUN: -O -wmo -enable-library-evolution -Rmodule-loading 2> %t/Lib-result.txt
+// RUN: %target-sil-opt %t/Lib.swiftmodule -I %t -sil-verify-all -o %t/Lib.sil
+// RUN: %FileCheck %s < %t/Lib.sil
 
 // REQUIRES: swift_in_compiler
-// REQUIRES: OS=macosx || OS=ios || OS=tvos || OS=watchos || OS=maccatalyst
-
-//--- Client.swift
-package import Utils
-
-package func clientFunc<T: PkgKlass>(_ list: [T]) {
-  // closure #1 in clientFunc<A>(_:)
-  // CHECK-CLIENT: sil private @$s6Client10clientFuncyySayxG5Utils8PkgKlassCRbzlFSo8NSObjectCxXEfU_ : $@convention(thin) <T where T : PkgKlass> (@in_guaranteed T) -> (@out NSObject, @error_indirect Never) {
-  // CHECK-CLIENT: class_method {{.*}} #PkgKlass.second!getter : (PkgKlass) -> () -> NSObject, $@convention(method) (@guaranteed PkgKlass) -> @owned NSObject
-  // CHECK-CLIENT: } // end sil function '$s6Client10clientFuncyySayxG5Utils8PkgKlassCRbzlFSo8NSObjectCxXEfU_'
-  let result = list.map { $0.second }
-  print(result)
-}
-
-
-//--- UtilsA.swift
-public import Foundation // public import to allow `NSObject` in API.
 
-package class PkgKlass: NSObject {
-  /// Serialized since it does _not_ reference a type from module imported as @_implementationOnly.
-  // PkgKlass.first.getter
-  // CHECK-UTILS-DAG: sil package [serialized_for_package] [canonical] @$s5Utils8PkgKlassC5firstSSvg : $@convention(method) (@guaranteed PkgKlass) -> @owned String {
-  package var first: String
-
-  /// NOT serialized since it does reference a type from module imported as @_implementationOnly.
-  // PkgKlass.second.getter
-  // CHECK-UTILS-DAG: sil package_external [canonical] @$s5Utils8PkgKlassC6secondSo8NSObjectCvg : $@convention(method) (@guaranteed PkgKlass) -> @owned NSObject
-  @objc package var second: NSObject
-
-  init(first: String, second: NSObject) {
-      self.first = first
-      self.second = second
-  }
-}
-
-//--- UtilsB.swift
-@_implementationOnly import Foundation
-
-public func utilsFunc() {
-  let x: NSString = "utilsfunc"
-  print(x)
-}
 
-//--- UI.swift
+//--- Lib.swift
 package import CoreA
 @_spiOnly public import CoreB
 
 /// PkgStruct is imported with `package import` and should be serialized.
-// CHECK-DAG: sil package [serialized_for_package] [canonical] @$s2UI7libFuncyy5CoreA9PkgStructVF : $@convention(thin) (@in_guaranteed PkgStruct) -> () {
+// CHECK-DAG: sil package [serialized_for_package] [canonical] @$s3Lib7libFuncyy5CoreA9PkgStructVF : $@convention(thin) (@in_guaranteed PkgStruct) -> () {
 package func libFunc(_ arg: PkgStruct) {
   print(arg.pkgVar)
 }
 
 /// PubStruct is imported with `@_spiOnly public import` and should be serialized.
-// CHECK-DAG: sil [serialized_for_package] [canonical] @$s2UI7spiFuncyy5CoreB15PubStructForSPIVF : $@convention(thin) (@in_guaranteed PubStructForSPI) -> () {
-@_spi(GroupB)
+// CHECK-DAG: sil [serialized_for_package] [canonical] @$s3Lib7spiFuncyy5CoreB15PubStructForSPIVF : $@convention(thin) (@in_guaranteed PubStructForSPI) -> () {
+@_spi(InCoreB)
 public func spiFunc(_ arg: PubStructForSPI) {
   print(arg.pubVarForSPI)
 }