Skip to content

Commit 7eeebea

Browse files
elshelsh
authored
Merge pull request #78732 from swiftlang/elsh/pcmo-keypath-fix
PackageCMO: fix serializability check for keypath
2 parents 52d4caa + 83f1ff6 commit 7eeebea

File tree

4 files changed

+187
-29
lines changed

4 files changed

+187
-29
lines changed

‎lib/SILOptimizer/IPO/CrossModuleOptimization.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,8 @@ bool CrossModuleOptimization::canSerializeFieldsByInstructionKind(
623623
canUse = false;
624624
},
625625
[&](SILDeclRef method) {
626+
if (!canUse) // If already set to false in the above lambda, return
627+
return;
626628
if (method.isForeign)
627629
canUse = false;
628630
else if (isPackageCMOEnabled(method.getModuleContext())) {

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

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎test/SILOptimizer/package-cmo-check-cast-dst.swift

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+
}

‎test/SILOptimizer/package-cmo-keypath.swift

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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+
/// Key path getter for a protocol property has a shared linkage as below, and a function referencing
18+
/// this getter (as well as this getter) should not be serialized in Package CMO, even if the `witness_method`
19+
/// in this getter has package or public access level.
20+
// key path getter for PkgProto.pkgVar : <A>A
21+
// sil shared [thunk] @$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK : $@convention(keypath_accessor_getter) <T where T : PkgProto> (@in_guaranteed T) -> @out String {
22+
// [%0: noescape **, write v**]
23+
// [%1: read v**, write v**, copy v**, destroy v**]
24+
// [global: read,write,copy,destroy,allocate,deinit_barrier]
25+
// // %0 // user: %4
26+
// // %1 // user: %3
27+
// bb0(%0 : $*String, %1 : $*T):
28+
// %2 = 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
29+
// %3 = apply %2<T>(%1) : $@convention(witness_method: PkgProto) <τ_0_0 where τ_0_0 : PkgProto> (@in_guaranteed τ_0_0) -> @owned String // user: %4
30+
// store %3 to %0 : $*String // id: %4
31+
// %5 = tuple () // user: %6
32+
// return %5 : $() // id: %6
33+
// } // end sil function '$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK'
34+
var pkgVar: String { get }
35+
36+
// key path getter for PkgProto.pkgVar : <A>A
37+
// CHECK-NOT: sil [serialized_for_package] [thunk] [canonical] @$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK : $@convention(keypath_accessor_getter) <T where T : PkgProto> (@in_guaranteed T) -> @out String {
38+
// 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
39+
// CHECK-NOT: // end sil function '$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK'
40+
}
41+
42+
package struct Foo: PkgProto {
43+
package var pkgVar: String {
44+
return "Foo pkgVar"
45+
}
46+
}
47+
48+
/// testKeyPath dynamically accesses pkgVar property on a generic type using keypath,
49+
/// referencing the getter above, s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK,
50+
/// and should not be serialized.
51+
// testKeyPath<A>(_:)
52+
// CHECK-NOT: sil package [serialized_for_package] [canonical] @$s3Lib11testKeyPathys0cD0CyxSSGSayxGAA8PkgProtoRzlF : $@convention(thin) <T where T : PkgProto> (@guaranteed Array<T>) -> @owned KeyPath<T, String> {
53+
// 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>
54+
// CHECK-NOT: } // end sil function '$s3Lib11testKeyPathys0cD0CyxSSGSayxGAA8PkgProtoRzlF'
55+
package func testKeyPath<T: PkgProto>(_ array: [T]) -> KeyPath<T, String> {
56+
return \T.pkgVar
57+
}
58+
59+
// CHECK: sil_witness_table package [serialized_for_package] Foo: PkgProto module Lib {
60+
// CHECK: method #PkgProto.pkgVar!getter: <Self where Self : PkgProto> (Self) -> () -> String : @$s3Lib3FooVAA8PkgProtoA2aDP6pkgVarSSvgTW
61+

0 commit comments

Comments
 (0)