Skip to content

Commit 02696c9

Browse files
committed
[CSBindings] Downgrade key path mutability based on contextual information
If key path literal is converted to a read-only type, let's not use maximum mutability to avoid unnecessary conversions. This is also important because accessor references are availability checked and we need to avoid referencing something that is not actually going to be used. There are some edge-cases in this approach which would still produce a conversions, i.e.: ``` struct S { var a: Int let b: Int } func test<T>(_: T, _: T) {} test(\S.a, \S.b) ``` Here `\S.a` is going to be converted to `KeyPath<S, Int>`. Availability checker would still have to recognize situations like that and skip checking setters if they are not used.
1 parent cf3dab1 commit 02696c9

File tree

3 files changed

+30
-7
lines changed

3 files changed

+30
-7
lines changed

lib/Sema/CSBindings.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,7 @@ bool BindingSet::finalize(bool transitive) {
721721
if (isValid && !capability)
722722
return false;
723723

724+
bool isContextualTypeReadOnly = false;
724725
// If the key path is sufficiently resolved we can add inferred binding
725726
// to the set.
726727
SmallSetVector<PotentialBinding, 4> updatedBindings;
@@ -740,19 +741,30 @@ bool BindingSet::finalize(bool transitive) {
740741
}
741742

742743
updatedBindings.insert(binding.withType(fnType));
744+
isContextualTypeReadOnly = true;
745+
} else if (!(bindingTy->isWritableKeyPath() ||
746+
bindingTy->isReferenceWritableKeyPath())) {
747+
isContextualTypeReadOnly = true;
743748
}
744749
}
745750

746751
// Note that even though key path literal maybe be invalid it's
747752
// still the best course of action to use contextual function type
748753
// bindings because they allow to propagate type information from
749-
// the key path into the context, so key path bindings are addded
754+
// the key path into the context, so key path bindings are added
750755
// only if there is absolutely no other choice.
751756
if (updatedBindings.empty()) {
752757
auto rootTy = CS.getKeyPathRootType(keyPath);
753758

754759
// A valid key path literal.
755760
if (capability) {
761+
// Capability inference always results in a maximum mutability
762+
// but if context is read-only it can be downgraded to avoid
763+
// conversions.
764+
if (isContextualTypeReadOnly)
765+
capability =
766+
std::make_pair(KeyPathMutability::ReadOnly, capability->second);
767+
756768
// Note that the binding is formed using root & value
757769
// type variables produced during constraint generation
758770
// because at this point root is already known (otherwise

test/Concurrency/sendable_keypaths.swift

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ do {
6161

6262
let _: KeyPath<K, Bool> = \.[NonSendable()] // ok
6363
let _: KeyPath<K, Bool> & Sendable = \.[NonSendable()] // expected-warning {{type 'KeyPath<K, Bool>' does not conform to the 'Sendable' protocol}}
64-
let _: KeyPath<K, Int> & Sendable = \.[42, NonSendable(data: [-1, 0, 1])] // expected-warning {{type 'ReferenceWritableKeyPath<K, Int>' does not conform to the 'Sendable' protocol}}
64+
let _: KeyPath<K, Int> & Sendable = \.[42, NonSendable(data: [-1, 0, 1])] // expected-warning {{type 'KeyPath<K, Int>' does not conform to the 'Sendable' protocol}}
6565
let _: KeyPath<K, Int> & Sendable = \.[42, -1] // Ok
6666

6767
test(nonSendableKP) // expected-warning {{type 'KeyPath<K, Bool>' does not conform to the 'Sendable' protocol}}
@@ -106,22 +106,22 @@ do {
106106
// expected-warning@-1 {{converting non-Sendable function value to '@Sendable (V) -> Int' may introduce data races}}
107107

108108
let _: KeyPath<V, Int> & Sendable = \.[42, CondSendable(NonSendable(data: [1, 2, 3]))]
109-
// expected-warning@-1 {{type 'ReferenceWritableKeyPath<V, Int>' does not conform to the 'Sendable' protocol}}
109+
// expected-warning@-1 {{type 'KeyPath<V, Int>' does not conform to the 'Sendable' protocol}}
110110
let _: KeyPath<V, Int> & Sendable = \.[42, CondSendable(42)] // Ok
111111

112112
struct Root {
113113
let v: V
114114
}
115115

116116
testSendableKP(v: v, \.[42, CondSendable(NonSendable(data: [1, 2, 3]))])
117-
// expected-warning@-1 {{type 'ReferenceWritableKeyPath<V, Int>' does not conform to the 'Sendable' protocol}}
117+
// expected-warning@-1 {{type 'KeyPath<V, Int>' does not conform to the 'Sendable' protocol}}
118118
testSendableFn(v: v, \.[42, CondSendable(NonSendable(data: [1, 2, 3]))])
119119
// expected-warning@-1 {{converting non-Sendable function value to '@Sendable (V) -> Int' may introduce data races}}
120120
testSendableKP(v: v, \.[42, CondSendable(42)]) // Ok
121121

122122
let nonSendable = NonSendable()
123123
testSendableKP(v: v, \.[42, CondSendable(nonSendable)])
124-
// expected-warning@-1 {{type 'ReferenceWritableKeyPath<V, Int>' does not conform to the 'Sendable' protocol}}
124+
// expected-warning@-1 {{type 'KeyPath<V, Int>' does not conform to the 'Sendable' protocol}}
125125

126126
testSendableFn(v: v, \.[42, CondSendable(nonSendable)])
127127
// expected-warning@-1 {{converting non-Sendable function value to '@Sendable (V) -> Int' may introduce data races}}
@@ -277,3 +277,14 @@ do {
277277
// expected-warning@-1 {{type 'KeyPath<Foo, Int>' does not conform to the 'Sendable' protocol; this is an error in the Swift 6 language mode}}
278278
}
279279
}
280+
281+
public final class TestSetterRef {
282+
public internal(set) var v: Int = 0 // expected-note {{setter for property 'v' is not '@usableFromInline' or public}}
283+
284+
public func test1(_ kp: KeyPath<TestSetterRef, Int> = \.v) {} // Ok
285+
public func test2(_ kp: KeyPath<TestSetterRef, Int> = \TestSetterRef.v) {} // Ok
286+
public func test3(_ kp: KeyPath<TestSetterRef, Int> & Sendable = \.v) {} // Ok
287+
288+
public func test_err(_ kp: WritableKeyPath<TestSetterRef, Int> = \.v) {}
289+
// expected-warning@-1 {{setter for property 'v' is internal and should not be referenced from a default argument value}}
290+
}

test/SILGen/keypaths.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,11 +738,11 @@ class M {
738738
// CHECK-LABEL: // test_metatype_keypaths()
739739
// CHECK-LABEL: sil hidden [ossa] @{{.*}} : $@convention(thin) () -> () {
740740
func test_metatype_keypaths() {
741-
// CHECK: keypath $ReferenceWritableKeyPath<M.Type, Int>, (root $M.Type; settable_property $Int, id @$s8keypaths1MC10chanceRainSivgZ : $@convention(method) (@thick M.Type) -> Int, getter @$s8keypaths1MC10chanceRainSivpZACmTK : $@convention(keypath_accessor_getter) (@in_guaranteed @thick M.Type) -> @out Int, setter @$s8keypaths1MC10chanceRainSivpZACmTk : $@convention(keypath_accessor_setter) (@in_guaranteed Int, @in_guaranteed @thick M.Type) -> ())
741+
// CHECK: keypath $KeyPath<M.Type, Int>, (root $M.Type; settable_property $Int, id @$s8keypaths1MC10chanceRainSivgZ : $@convention(method) (@thick M.Type) -> Int, getter @$s8keypaths1MC10chanceRainSivpZACmTK : $@convention(keypath_accessor_getter) (@in_guaranteed @thick M.Type) -> @out Int, setter @$s8keypaths1MC10chanceRainSivpZACmTk : $@convention(keypath_accessor_setter) (@in_guaranteed Int, @in_guaranteed @thick M.Type) -> ())
742742
let _: KeyPath<M.Type, Int> = \M.Type.chanceRain
743743
// CHECK: keypath $KeyPath<M.Type, Bool>, (root $M.Type; gettable_property $Bool, id @$s8keypaths1MC7isSunnySbvgZ : $@convention(method) (@thick M.Type) -> Bool, getter @$s8keypaths1MC7isSunnySbvpZACmTK : $@convention(keypath_accessor_getter) (@in_guaranteed @thick M.Type) -> @out Bool)
744744
let _: KeyPath<M.Type, Bool> = \M.Type.isSunny
745-
// CHECK: keypath $ReferenceWritableKeyPath<M.Type, Bool>, (root $M.Type; settable_property $Bool, id @$s8keypaths1MC8isCloudySbvgZ : $@convention(method) (@thick M.Type) -> Bool, getter @$s8keypaths1MC8isCloudySbvpZACmTK : $@convention(keypath_accessor_getter) (@in_guaranteed @thick M.Type) -> @out Bool, setter @$s8keypaths1MC8isCloudySbvpZACmTk : $@convention(keypath_accessor_setter) (@in_guaranteed Bool, @in_guaranteed @thick M.Type) -> ())
745+
// CHECK: keypath $KeyPath<M.Type, Bool>, (root $M.Type; settable_property $Bool, id @$s8keypaths1MC8isCloudySbvgZ : $@convention(method) (@thick M.Type) -> Bool, getter @$s8keypaths1MC8isCloudySbvpZACmTK : $@convention(keypath_accessor_getter) (@in_guaranteed @thick M.Type) -> @out Bool, setter @$s8keypaths1MC8isCloudySbvpZACmTk : $@convention(keypath_accessor_setter) (@in_guaranteed Bool, @in_guaranteed @thick M.Type) -> ())
746746
let _: KeyPath<M.Type, Bool> = \M.Type.isCloudy
747747
// CHECK: keypath $KeyPath<M.Type, String>, (root $M.Type; gettable_property $String, id @$s8keypaths1MCySSSicigZ : $@convention(method) (Int, @thick M.Type) -> @owned String, getter @$s8keypaths1MCySSSicipZACmTK : $@convention(keypath_accessor_getter) (@in_guaranteed @thick M.Type, @in_guaranteed Int) -> @out String, indices [%$0 : $Int : $Int], indices_equals @$sSiTH : $@convention(keypath_accessor_equals) (@in_guaranteed Int, @in_guaranteed Int) -> Bool, indices_hash @$sSiTh : $@convention(keypath_accessor_hash) (@in_guaranteed Int) -> Int) (%{{.*}})
748748
let _: KeyPath<M.Type, String> = \M.Type.[2]

0 commit comments

Comments
 (0)