Skip to content

Commit 1e5266e

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 f6cfa08 commit 1e5266e

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
@@ -829,6 +829,21 @@ void CrossModuleOptimization::makeDeclUsableFromInline(ValueDecl *decl) {
829829
if (decl->getEffectiveAccess() >= AccessLevel::Package)
830830
return;
831831

832+
// FIXME: rdar://130456707
833+
// Currently not all types are visited in canSerialize* calls, sometimes
834+
// resulting in an internal type getting @usableFromInline, which is
835+
// incorrect.
836+
// For example, for `let q = P() as? Q`, where Q is an internal class
837+
// inherting a public class P, Q is not visited in the canSerialize*
838+
// checks, thus resulting in `@usableFromInline class Q`; this is not
839+
// the intended behavior in the conservative mode as it modifies AST.
840+
//
841+
// To properly fix, instruction visitor needs to be refactored to do
842+
// both the "canSerialize" check (that visits all types) and serialize
843+
// or update visibility (modify AST in non-conservative modes).
844+
if (isPackageCMOEnabled(M.getSwiftModule()))
845+
return;
846+
832847
// We must not modify decls which are defined in other modules.
833848
if (M.getSwiftModule() != decl->getDeclContext()->getParentModule())
834849
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)