Skip to content

Commit

Permalink
PackageCMO: fix serializability check for keypath
Browse files Browse the repository at this point in the history
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
  • Loading branch information
elsh committed Jan 18, 2025
1 parent 4ae5685 commit 54b7b76
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 30 deletions.
3 changes: 2 additions & 1 deletion lib/SILOptimizer/IPO/CrossModuleOptimization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,8 @@ bool CrossModuleOptimization::canSerializeFieldsByInstructionKind(
[&](SILDeclRef method) {
if (method.isForeign)
canUse = false;
else if (isPackageCMOEnabled(method.getModuleContext())) {
else if (canUse &&
isPackageCMOEnabled(method.getModuleContext())) {
// If the referenced keypath is internal, do not
// serialize.
auto methodScope = method.getDecl()->getFormalAccessScope(
Expand Down
29 changes: 0 additions & 29 deletions test/SILOptimizer/package-cmo-cast-internal-dst

This file was deleted.

124 changes: 124 additions & 0 deletions test/SILOptimizer/package-cmo-check-cast-dst.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t

/// Verify a function with a cast instruction with an internal decl as dst
/// does not get serialized with PackageCMO in both scenarios below.

/// Scenario 1.
// RUN: %target-swift-frontend -emit-sil %t/Lib.swift -package-name pkg \
// RUN: -wmo -allow-non-resilient-access -package-cmo \
// RUN: -enable-library-evolution -swift-version 5 \
// RUN: -Xllvm -sil-print-function=topFunc -o %t/Lib.sil
// RUN: %FileCheck %s < %t/Lib.sil

/// Scenario 2.
// RUN: %target-build-swift %t/Mod.swift \
// RUN: -module-name=Mod -package-name pkg \
// RUN: -parse-as-library -emit-module -emit-module-path %t/Mod.swiftmodule -I%t \
// RUN: -Xfrontend -experimental-package-cmo -Xfrontend -experimental-allow-non-resilient-access \
// RUN: -O -wmo -enable-library-evolution
// RUN: %target-sil-opt -enable-sil-verify-all %t/Mod.swiftmodule -o %t/Mod.sil
// RUN: %FileCheck %s --check-prefix=CHECK-MOD < %t/Mod.sil


//--- Lib.swift
public class Pub {
public var pubVar: Int
public init(_ arg: Int) {
pubVar = arg
}
}

class InternalKlass: Pub {}

/// Verify that `InternalKlass` is visited and the instruction containing it is not serialized.
// CHECK: sil @$s3Lib7topFuncySiAA3PubCF : $@convention(thin) (@guaranteed Pub) -> Int {
// CHECK: checked_cast_br Pub in %0 to InternalKlass
public func topFunc(_ arg: Pub) -> Int {
let x = arg as? InternalKlass
return x != nil ? 1 : 0
}


//--- Mod.swift
struct SymmetricTextChildQuery<Provider: PubProto> {
var source: Text
init(_ arg: Text) {
source = arg
}
/// This function references isCollapsible(), which contains an internal decl.
/// If isCollapsible() were serialized, building a client of this module would fail
/// due to a linker error: undefined symbol `CollapsibleTextModifier`.
mutating func updateValue() {
let resolvedSource = ResolvedStyledText.styledText(canCollapse: source.isCollapsible())
}
}

@frozen
public struct Text: Equatable, Sendable {
public init() {}
@frozen
@usableFromInline
package enum Modifier: Equatable {
case font
case anyTextModifier(AnyTextModifier)

@usableFromInline
package static func ==(lhs: Modifier, rhs: Modifier) -> Bool {
return true
}
}

@usableFromInline
package var modifiers = [Modifier]()

}

extension Text {
/// Verify that function containing an internal decl CollapsibleTextModifier is
/// not serialized with Package CMO.
// CHECK-MOD-NOT: sil package [serialized_for_package] [canonical] @$s3Mod4TextV13isCollapsibleSbyF : $@convention(method) (@guaranteed Text) -> Bool {
// CHECK-MOD-NOT: checked_cast_br AnyTextModifier in {{.*}} : $AnyTextModifier to CollapsibleTextModifier
package func isCollapsible() -> Bool {
modifiers.contains { modifier in
guard case .anyTextModifier(let anyModifier) = modifier
else { return false }
return anyModifier is CollapsibleTextModifier
}
}
}

final class CollapsibleTextModifier: AnyTextModifier {
override func isEqual(to other: AnyTextModifier) -> Bool {
other is CollapsibleTextModifier
}
}

public protocol PubProto {
var pubVar: String { get set }
}

public struct PubStruct {
public static func makeView<P: PubProto>(_ type: P.Type, _ arg: Text) {
var child = SymmetricTextChildQuery<P>(arg)
child.updateValue()
}
}

public class ResolvedStyledText {
package var canCollapse: Bool
package init(_ arg: Bool) {
canCollapse = arg
}
}

extension ResolvedStyledText {
package static func styledText(canCollapse: Bool) -> ResolvedStyledText {
return ResolvedStyledText(canCollapse)
}
}

@usableFromInline
package class AnyTextModifier {
func isEqual(to other: AnyTextModifier) -> Bool { fatalError() }
}
41 changes: 41 additions & 0 deletions test/SILOptimizer/package-cmo-keypath.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t

// RUN: %target-build-swift %t/Lib.swift \
// RUN: -module-name=Lib -package-name Pkg \
// RUN: -parse-as-library -emit-module -emit-module-path %t/Lib.swiftmodule -I%t \
// RUN: -Xfrontend -experimental-package-cmo -Xfrontend -experimental-allow-non-resilient-access \
// RUN: -O -wmo -enable-library-evolution
// RUN: %target-sil-opt -enable-sil-verify-all %t/Lib.swiftmodule -o %t/Lib.sil

// RUN: %FileCheck %s < %t/Lib.sil

// REQUIRES: swift_in_compiler

//--- Lib.swift
package protocol PkgProto {
var pkgVar: String { get }
}

package struct Foo: PkgProto {
package var pkgVar: String {
return "Foo pkgVar"
}
}

// testKeyPath<A>(_:)
// CHECK-NOT: sil package [serialized_for_package] [canonical] @$s3Lib11testKeyPathys0cD0CyxSSGSayxGAA8PkgProtoRzlF : $@convention(thin) <T where T : PkgProto> (@guaranteed Array<T>) -> @owned KeyPath<T, String> {
// 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>
// CHECK-NOT: } // end sil function '$s3Lib11testKeyPathys0cD0CyxSSGSayxGAA8PkgProtoRzlF'
package func testKeyPath<T: PkgProto>(_ array: [T]) -> KeyPath<T, String> {
return \T.pkgVar
}

// key path getter for PkgProto.pkgVar : <A>A
// CHECK-NOT: sil [serialized_for_package] [thunk] [canonical] @$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK : $@convention(keypath_accessor_getter) <T where T : PkgProto> (@in_guaranteed T) -> @out String {
// 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
// CHECK-NOT: // end sil function '$s3Lib8PkgProtoP6pkgVarSSvpAaBRzlxTK'

// CHECK: sil_witness_table package [serialized_for_package] Foo: PkgProto module Lib {
// CHECK: method #PkgProto.pkgVar!getter: <Self where Self : PkgProto> (Self) -> () -> String : @$s3Lib3FooVAA8PkgProtoA2aDP6pkgVarSSvgTW

0 comments on commit 54b7b76

Please sign in to comment.