Skip to content

Commit 827280a

Browse files
committed
[PackageCMO] Don't allow modifying AST.
Currently not all types are visited in canSerialize* calls, sometimes resulting in an internal type getting @usableFromInline, which is incorrect. For example, for `let q = P() as? Q`, where Q is an internal class inherting a public class P, Q is not visited in the canSerialize* checks, thus resulting in `@usableFromInline class Q`; this is not the intended behavior in the conservative mode used by PackageCMO as it modifies AST. To properly fix, instruction visitor needs to be refactored to do both the "canSerialize" check (that visits all types) and serialize or update visibility (modify AST in non-conservative modes). This PR provides a short-term fix that prevents modifying AST, and also ensures that the generated interfaces with PackageCMO flags are not affected by the optimization or contain modified AST. rdar://130292190
1 parent 5172b72 commit 827280a

File tree

2 files changed

+81
-0
lines changed

2 files changed

+81
-0
lines changed

lib/SILOptimizer/IPO/CrossModuleOptimization.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,21 @@ void CrossModuleOptimization::makeDeclUsableFromInline(ValueDecl *decl) {
818818
if (decl->getEffectiveAccess() >= AccessLevel::Package)
819819
return;
820820

821+
// FIXME: rdar://130456707
822+
// Currently not all types are visited in canSerialize* calls, sometimes
823+
// resulting in an internal type getting @usableFromInline, which is
824+
// incorrect.
825+
// For example, for `let q = P() as? Q`, where Q is an internal class
826+
// inherting a public class P, Q is not visited in the canSerialize*
827+
// checks, thus resulting in `@usableFromInline class Q`; this is not
828+
// the intended behavior in the conservative mode as it modifies AST.
829+
//
830+
// To properly fix, instruction visitor needs to be refactored to do
831+
// both the "canSerialize" check (that visits all types) and serialize
832+
// or update visibility (modify AST in non-conservative modes).
833+
if (isPackageCMOEnabled(M.getSwiftModule()))
834+
return;
835+
821836
// We must not modify decls which are defined in other modules.
822837
if (M.getSwiftModule() != decl->getDeclContext()->getParentModule())
823838
return;
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// RUN: %empty-directory(%t)
2+
3+
/// First Test: Check `@_usableFromInline` is not added to types in PackageCMO mode.
4+
// RUN: %target-swift-frontend -parse-as-library %s -O -wmo -enable-library-evolution -experimental-allow-non-resilient-access -experimental-package-cmo -module-name=Lib -package-name pkg -emit-module -o %t/Lib-package-cmo.swiftmodule
5+
// RUN: %target-sil-opt -module-name Lib -enable-sil-verify-all %t/Lib-package-cmo.swiftmodule -o %t/Lib-package-cmo.sil
6+
// RUN: %FileCheck %s < %t/Lib-package-cmo.sil
7+
8+
/// Second Test: Check .swiftinterface files with and without PackageCMO have the same decl signatures without `@_usableFromInline`.
9+
// RUN: %target-swift-frontend -emit-module %s -I %t \
10+
// RUN: -module-name Lib -package-name pkg \
11+
// RUN: -enable-library-evolution -swift-version 6 \
12+
// RUN: -emit-module-path %t/Lib.swiftmodule \
13+
// RUN: -emit-module-interface-path %t/Lib.swiftinterface \
14+
// RUN: -emit-private-module-interface-path %t/Lib.private.swiftinterface \
15+
// RUN: -emit-package-module-interface-path %t/Lib.package.swiftinterface
16+
// RUN: %FileCheck %s --check-prefixes=CHECK-PKG-INTERFACE,CHECK-INTERFACE < %t/Lib.package.swiftinterface
17+
// RUN: %FileCheck %s --check-prefix=CHECK-INTERFACE < %t/Lib.swiftinterface
18+
19+
// RUN: rm -rf %t/Lib.swiftmodule
20+
// RUN: rm -rf %t/Lib.swiftinterface
21+
// RUN: rm -rf %t/Lib.private.swiftinterface
22+
// RUN: rm -rf %t/Lib.package.swiftinterface
23+
24+
// RUN: %target-swift-frontend -emit-module %s -I %t \
25+
// RUN: -module-name Lib -package-name pkg \
26+
// RUN: -enable-library-evolution -swift-version 6 \
27+
// RUN: -O -wmo \
28+
// RUN: -experimental-allow-non-resilient-access -experimental-package-cmo \
29+
// RUN: -emit-module-path %t/Lib.swiftmodule \
30+
// RUN: -emit-module-interface-path %t/Lib.swiftinterface \
31+
// RUN: -emit-private-module-interface-path %t/Lib.private.swiftinterface \
32+
// RUN: -emit-package-module-interface-path %t/Lib.package.swiftinterface
33+
// RUN: %FileCheck %s --check-prefixes=CHECK-PKG-INTERFACE,CHECK-INTERFACE < %t/Lib.package.swiftinterface
34+
// RUN: %FileCheck %s --check-prefix=CHECK-INTERFACE < %t/Lib.swiftinterface
35+
36+
// REQUIRES: swift_in_compiler
37+
38+
// CHECK-NOT: @usableFromInline
39+
final class InternalKlass: PkgKlass {
40+
@inline(never)
41+
override func bar() -> Int { return 13 }
42+
}
43+
44+
package class PkgKlass {
45+
@inline(never)
46+
package func bar() -> Int { return 11 }
47+
}
48+
49+
// CHECK-NOT: sil package [serialized_for_package] [noinline] [canonical] @$s3Lib3fooySiSgAA8PkgKlassCF : $@convention(thin) (@guaranteed PkgKlass) -> Optional<Int> {
50+
// CHECK-NOT: checked_cast_br PkgKlass in {{.*}} : $PkgKlass to InternalKlass
51+
@inline(never)
52+
package func foo(_ arg: PkgKlass) -> Int? {
53+
let x = arg as? InternalKlass
54+
return x?.bar()
55+
}
56+
57+
public func run() -> Int {
58+
return PkgKlass().bar()
59+
}
60+
61+
// CHECK-PKG-INTERFACE-NOT: @usableFromInline
62+
// CHECK-INTERFACE-NOT: @usableFromInline
63+
// CHECK-PKG-INTERFACE: package class PkgKlass {
64+
// CHECK-PKG-INTERFACE: @inline(never) package func bar() -> Swift.Int
65+
// CHECK-PKG-INTERFACE: @inline(never) package func foo(_ arg: Lib.PkgKlass) -> Swift.Int?
66+
// CHECK-INTERFACE: public func run() -> Swift.Int

0 commit comments

Comments
 (0)