Skip to content

Commit 54b7b76

Browse files
committed
PackageCMO: fix serializability check for keypath
When keypath was checked for serializability, its referenced method was sometimes incorrectly deemed serializable even if its referenced function was not. This inconsistency could result in serializing a function that contained such keypath. This PR fixes the issue by ensuring the serializability of the referenced function is properly tracked. Resolves rdar://142950306
1 parent 4ae5685 commit 54b7b76

File tree

4 files changed

+167
-30
lines changed

4 files changed

+167
-30
lines changed

lib/SILOptimizer/IPO/CrossModuleOptimization.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,8 @@ bool CrossModuleOptimization::canSerializeFieldsByInstructionKind(
625625
[&](SILDeclRef method) {
626626
if (method.isForeign)
627627
canUse = false;
628-
else if (isPackageCMOEnabled(method.getModuleContext())) {
628+
else if (canUse &&
629+
isPackageCMOEnabled(method.getModuleContext())) {
629630
// If the referenced keypath is internal, do not
630631
// serialize.
631632
auto methodScope = method.getDecl()->getFormalAccessScope(

test/SILOptimizer/package-cmo-cast-internal-dst

Lines changed: 0 additions & 29 deletions
This file was deleted.
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
/// Verify a function with a cast instruction with an internal decl as dst
5+
/// does not get serialized with PackageCMO in both scenarios below.
6+
7+
/// Scenario 1.
8+
// RUN: %target-swift-frontend -emit-sil %t/Lib.swift -package-name pkg \
9+
// RUN: -wmo -allow-non-resilient-access -package-cmo \
10+
// RUN: -enable-library-evolution -swift-version 5 \
11+
// RUN: -Xllvm -sil-print-function=topFunc -o %t/Lib.sil
12+
// RUN: %FileCheck %s < %t/Lib.sil
13+
14+
/// Scenario 2.
15+
// RUN: %target-build-swift %t/Mod.swift \
16+
// RUN: -module-name=Mod -package-name pkg \
17+
// RUN: -parse-as-library -emit-module -emit-module-path %t/Mod.swiftmodule -I%t \
18+
// RUN: -Xfrontend -experimental-package-cmo -Xfrontend -experimental-allow-non-resilient-access \
19+
// RUN: -O -wmo -enable-library-evolution
20+
// RUN: %target-sil-opt -enable-sil-verify-all %t/Mod.swiftmodule -o %t/Mod.sil
21+
// RUN: %FileCheck %s --check-prefix=CHECK-MOD < %t/Mod.sil
22+
23+
24+
//--- Lib.swift
25+
public class Pub {
26+
public var pubVar: Int
27+
public init(_ arg: Int) {
28+
pubVar = arg
29+
}
30+
}
31+
32+
class InternalKlass: Pub {}
33+
34+
/// Verify that `InternalKlass` is visited and the instruction containing it is not serialized.
35+
// CHECK: sil @$s3Lib7topFuncySiAA3PubCF : $@convention(thin) (@guaranteed Pub) -> Int {
36+
// CHECK: checked_cast_br Pub in %0 to InternalKlass
37+
public func topFunc(_ arg: Pub) -> Int {
38+
let x = arg as? InternalKlass
39+
return x != nil ? 1 : 0
40+
}
41+
42+
43+
//--- Mod.swift
44+
struct SymmetricTextChildQuery<Provider: PubProto> {
45+
var source: Text
46+
init(_ arg: Text) {
47+
source = arg
48+
}
49+
/// This function references isCollapsible(), which contains an internal decl.
50+
/// If isCollapsible() were serialized, building a client of this module would fail
51+
/// due to a linker error: undefined symbol `CollapsibleTextModifier`.
52+
mutating func updateValue() {
53+
let resolvedSource = ResolvedStyledText.styledText(canCollapse: source.isCollapsible())
54+
}
55+
}
56+
57+
@frozen
58+
public struct Text: Equatable, Sendable {
59+
public init() {}
60+
@frozen
61+
@usableFromInline
62+
package enum Modifier: Equatable {
63+
case font
64+
case anyTextModifier(AnyTextModifier)
65+
66+
@usableFromInline
67+
package static func ==(lhs: Modifier, rhs: Modifier) -> Bool {
68+
return true
69+
}
70+
}
71+
72+
@usableFromInline
73+
package var modifiers = [Modifier]()
74+
75+
}
76+
77+
extension Text {
78+
/// Verify that function containing an internal decl CollapsibleTextModifier is
79+
/// not serialized with Package CMO.
80+
// CHECK-MOD-NOT: sil package [serialized_for_package] [canonical] @$s3Mod4TextV13isCollapsibleSbyF : $@convention(method) (@guaranteed Text) -> Bool {
81+
// CHECK-MOD-NOT: checked_cast_br AnyTextModifier in {{.*}} : $AnyTextModifier to CollapsibleTextModifier
82+
package func isCollapsible() -> Bool {
83+
modifiers.contains { modifier in
84+
guard case .anyTextModifier(let anyModifier) = modifier
85+
else { return false }
86+
return anyModifier is CollapsibleTextModifier
87+
}
88+
}
89+
}
90+
91+
final class CollapsibleTextModifier: AnyTextModifier {
92+
override func isEqual(to other: AnyTextModifier) -> Bool {
93+
other is CollapsibleTextModifier
94+
}
95+
}
96+
97+
public protocol PubProto {
98+
var pubVar: String { get set }
99+
}
100+
101+
public struct PubStruct {
102+
public static func makeView<P: PubProto>(_ type: P.Type, _ arg: Text) {
103+
var child = SymmetricTextChildQuery<P>(arg)
104+
child.updateValue()
105+
}
106+
}
107+
108+
public class ResolvedStyledText {
109+
package var canCollapse: Bool
110+
package init(_ arg: Bool) {
111+
canCollapse = arg
112+
}
113+
}
114+
115+
extension ResolvedStyledText {
116+
package static func styledText(canCollapse: Bool) -> ResolvedStyledText {
117+
return ResolvedStyledText(canCollapse)
118+
}
119+
}
120+
121+
@usableFromInline
122+
package class AnyTextModifier {
123+
func isEqual(to other: AnyTextModifier) -> Bool { fatalError() }
124+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
// RUN: %target-build-swift %t/Lib.swift \
5+
// RUN: -module-name=Lib -package-name Pkg \
6+
// RUN: -parse-as-library -emit-module -emit-module-path %t/Lib.swiftmodule -I%t \
7+
// RUN: -Xfrontend -experimental-package-cmo -Xfrontend -experimental-allow-non-resilient-access \
8+
// RUN: -O -wmo -enable-library-evolution
9+
// RUN: %target-sil-opt -enable-sil-verify-all %t/Lib.swiftmodule -o %t/Lib.sil
10+
11+
// RUN: %FileCheck %s < %t/Lib.sil
12+
13+
// REQUIRES: swift_in_compiler
14+
15+
//--- Lib.swift
16+
package protocol PkgProto {
17+
var pkgVar: String { get }
18+
}
19+
20+
package struct Foo: PkgProto {
21+
package var pkgVar: String {
22+
return "Foo pkgVar"
23+
}
24+
}
25+
26+
// testKeyPath<A>(_:)
27+
// CHECK-NOT: sil package [serialized_for_package] [canonical] @$s3Lib11testKeyPathys0cD0CyxSSGSayxGAA8PkgProtoRzlF : $@convention(thin) <T where T : PkgProto> (@guaranteed Array<T>) -> @owned KeyPath<T, String> {
28+
// CHECK-NOT: keypath $KeyPath<T, String>, <τ_0_0 where τ_0_0 : PkgProto> (root $τ_0_0; gettable_property $String, id #PkgProto.pkgVar!getter : <Self where Self : PkgProto> (Self) -> () -> String, getter @$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK : $@convention(keypath_accessor_getter) <τ_0_0 where τ_0_0 : PkgProto> (@in_guaranteed τ_0_0) -> @out String) <T>
29+
// CHECK-NOT: } // end sil function '$s3Lib11testKeyPathys0cD0CyxSSGSayxGAA8PkgProtoRzlF'
30+
package func testKeyPath<T: PkgProto>(_ array: [T]) -> KeyPath<T, String> {
31+
return \T.pkgVar
32+
}
33+
34+
// key path getter for PkgProto.pkgVar : <A>A
35+
// CHECK-NOT: sil [serialized_for_package] [thunk] [canonical] @$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK : $@convention(keypath_accessor_getter) <T where T : PkgProto> (@in_guaranteed T) -> @out String {
36+
// CHECK-NOT: witness_method $T, #PkgProto.pkgVar!getter : <Self where Self : PkgProto> (Self) -> () -> String : $@convention(witness_method: PkgProto) <τ_0_0 where τ_0_0 : PkgProto> (@in_guaranteed τ_0_0) -> @owned String // user: %3
37+
// CHECK-NOT: // end sil function '$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK'
38+
39+
// CHECK: sil_witness_table package [serialized_for_package] Foo: PkgProto module Lib {
40+
// CHECK: method #PkgProto.pkgVar!getter: <Self where Self : PkgProto> (Self) -> () -> String : @$s3Lib3FooVAA8PkgProtoA2aDP6pkgVarSSvgTW
41+

0 commit comments

Comments
 (0)