Skip to content

Commit 47d6909

Browse files
AnthonyLatsisAnthonyLatsis
authored
Merge pull request #42363 from AnthonyLatsis/swift-keypath-objc-optional-component
CS: `optional` storage key path components are read-only
2 parents 9de1a79 + 4a0f6ce commit 47d6909

File tree

4 files changed

+81
-16
lines changed

4 files changed

+81
-16
lines changed

‎lib/IRGen/GenObjC.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,7 +1279,10 @@ irgen::emitObjCGetterDescriptorParts(IRGenModule &IGM,
12791279
ObjCMethodDescriptor
12801280
irgen::emitObjCSetterDescriptorParts(IRGenModule &IGM,
12811281
VarDecl *property) {
1282-
assert(property->isSettable(property->getDeclContext()) &&
1282+
// Optional properties support mutation on the Objective-C side, but not the
1283+
// Swift side.
1284+
assert((property->getAttrs().hasAttribute<OptionalAttr>() ||
1285+
property->isSettable(property->getDeclContext())) &&
12831286
"not a settable property?!");
12841287

12851288
Selector setterSel(property, Selector::ForSetter);
@@ -1320,7 +1323,11 @@ irgen::emitObjCSetterDescriptorParts(IRGenModule &IGM,
13201323
ObjCMethodDescriptor
13211324
irgen::emitObjCSetterDescriptorParts(IRGenModule &IGM,
13221325
SubscriptDecl *subscript) {
1323-
assert(subscript->supportsMutation() && "not a settable subscript?!");
1326+
// Optional subscripts support mutation on the Objective-C side, but not the
1327+
// Swift side.
1328+
assert((subscript->getAttrs().hasAttribute<OptionalAttr>() ||
1329+
subscript->supportsMutation()) &&
1330+
"not a settable subscript?!");
13241331

13251332
Selector setterSel(subscript, Selector::ForSetter);
13261333
ObjCMethodDescriptor descriptor{};

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3309,7 +3309,9 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
33093309
WriteImplKind writeImpl = WriteImplKind::Immutable;
33103310
ReadWriteImplKind readWriteImpl = ReadWriteImplKind::Immutable;
33113311

3312-
if (storage->getParsedAccessor(AccessorKind::Set)) {
3312+
// TODO: Writing to optional storage requirements is not supported.
3313+
if (!storage->getAttrs().hasAttribute<OptionalAttr>() &&
3314+
storage->getParsedAccessor(AccessorKind::Set)) {
33133315
readImpl = ReadImplKind::Get;
33143316
writeImpl = WriteImplKind::Set;
33153317
readWriteImpl = ReadWriteImplKind::MaterializeToTemporary;

‎test/SILGen/keypaths_objc.swift

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -158,32 +158,43 @@ func dynamicMemberLookupMixedKeypaths(foo: DynamicClass<Foo>) {
158158
_ = foo.bar.foo.nonobjc.y
159159
}
160160

161+
@objc class Object: NSObject {
162+
var name: String
163+
init(name: String) {
164+
self.name = name
165+
}
166+
}
161167
@objc protocol ObjCProtoOptional {
162-
@objc optional var optionalProperty: Bool { get }
168+
@objc optional var object: Object { get set }
163169

164-
@objc optional subscript(_: Int) -> Bool { get }
170+
@objc optional subscript(_: Bool) -> Object { get set }
165171
}
166172

167173
// CHECK-LABEL: sil hidden [ossa] @{{.*}}0B28ProtocolOptionalRequirementsyyF
168-
// CHECK: keypath $KeyPath<ObjCProtoOptional, Optional<Bool>>, (objc "optionalProperty"; root $ObjCProtoOptional; gettable_property $Optional<Bool>, id #ObjCProtoOptional.optionalProperty!getter.foreign : <Self where Self : ObjCProtoOptional> (Self) -> () -> Bool, getter @$[[PROP_GETTER:[_a-zA-Z0-9]+]]
169-
// CHECK: keypath $KeyPath<ObjCProtoOptional, Optional<Bool>>, (root $ObjCProtoOptional; gettable_property $Optional<Bool>, id #ObjCProtoOptional.subscript!getter.foreign : <Self where Self : ObjCProtoOptional> (Self) -> (Int) -> Bool, getter @$[[SUBSCR_GETTER:[_a-zA-Z0-9]+]]
174+
// CHECK: keypath $KeyPath<ObjCProtoOptional, Optional<Object>>, (objc "object"; root $ObjCProtoOptional; gettable_property $Optional<Object>, id #ObjCProtoOptional.object!getter.foreign : <Self where Self : ObjCProtoOptional> (Self) -> () -> Object, getter @$[[PROP_GETTER:[_a-zA-Z0-9]+]]
175+
// CHECK: keypath $KeyPath<ObjCProtoOptional, Optional<Object>>, (root $ObjCProtoOptional; gettable_property $Optional<Object>, id #ObjCProtoOptional.subscript!getter.foreign : <Self where Self : ObjCProtoOptional> (Self) -> (Bool) -> Object, getter @$[[SUBSCR_GETTER:[_a-zA-Z0-9]+]]
176+
// CHECK: keypath $ReferenceWritableKeyPath<ObjCProtoOptional, String>, (root $ObjCProtoOptional; gettable_property $Optional<Object>, id #ObjCProtoOptional.object!getter.foreign : <Self where Self : ObjCProtoOptional> (Self) -> () -> Object, getter @$[[PROP_GETTER]] : {{.*}}; optional_force : $Object; settable_property $String,
177+
// CHECK: keypath $ReferenceWritableKeyPath<ObjCProtoOptional, String>, (root $ObjCProtoOptional; gettable_property $Optional<Object>, id #ObjCProtoOptional.subscript!getter.foreign : <Self where Self : ObjCProtoOptional> (Self) -> (Bool) -> Object, getter @$[[SUBSCR_GETTER]] : {{.*}}; optional_force : $Object; settable_property $String,
170178
// CHECK: } // end sil function '${{.*}}0B28ProtocolOptionalRequirementsyyF'
171179
//
172-
// CHECK: sil shared [thunk] [ossa] @$[[PROP_GETTER]] : $@convention(thin) (@in_guaranteed ObjCProtoOptional) -> @out Optional<Bool> {
180+
// CHECK: sil shared [thunk] [ossa] @$[[PROP_GETTER]] : $@convention(thin) (@in_guaranteed ObjCProtoOptional) -> @out Optional<Object> {
173181
// CHECK: [[BASE:%[0-9]+]] = open_existential_ref {{%[0-9]+}} : $ObjCProtoOptional to $[[OPENED_TY:@opened\("[-A-F0-9]+"\) ObjCProtoOptional]]
174-
// CHECK: dynamic_method_br [[BASE]] : $[[OPENED_TY]], #ObjCProtoOptional.optionalProperty!getter.foreign, bb1
175-
// CHECK: bb1({{%[0-9]+}} : $@convention(objc_method) ([[OPENED_TY]]) -> ObjCBool)
182+
// CHECK: dynamic_method_br [[BASE]] : $[[OPENED_TY]], #ObjCProtoOptional.object!getter.foreign, bb1
183+
// CHECK: bb1({{%[0-9]+}} : $@convention(objc_method) ([[OPENED_TY]]) -> @autoreleased Object)
176184
// CHECK: } // end sil function '$[[PROP_GETTER]]'
177185
//
178-
// CHECK: sil shared [thunk] [ossa] @$[[SUBSCR_GETTER]] : $@convention(thin) (@in_guaranteed ObjCProtoOptional, UnsafeRawPointer) -> @out Optional<Bool> {
186+
// CHECK: sil shared [thunk] [ossa] @$[[SUBSCR_GETTER]] : $@convention(thin) (@in_guaranteed ObjCProtoOptional, UnsafeRawPointer) -> @out Optional<Object> {
179187
// CHECK: [[BASE:%[0-9]+]] = open_existential_ref {{%[0-9]+}} : $ObjCProtoOptional to $[[OPENED_TY:@opened\("[-A-F0-9]+"\) ObjCProtoOptional]]
180-
// CHECK: [[INDEX:%[0-9]+]] = load [trivial] {{%[0-9]+}} : $*Int
188+
// CHECK: [[INDEX:%[0-9]+]] = load [trivial] {{%[0-9]+}} : $*Bool
181189
// CHECK: dynamic_method_br [[BASE]] : $[[OPENED_TY]], #ObjCProtoOptional.subscript!getter.foreign, bb1, bb2
182-
// CHECK: bb1({{%[0-9]+}} : $@convention(objc_method) (Int, [[OPENED_TY]]) -> ObjCBool):
183-
// CHECK: %17 = apply {{%[0-9]+}}([[INDEX]]) : $@callee_guaranteed (Int) -> Bool
190+
// CHECK: bb1({{%[0-9]+}} : $@convention(objc_method) (ObjCBool, [[OPENED_TY]]) -> @autoreleased Object):
191+
// CHECK: %17 = apply {{%[0-9]+}}([[INDEX]]) : $@callee_guaranteed (Bool) -> @owned Object
184192
// CHECK: bb2:
185193
// CHECK: } // end sil function '$[[SUBSCR_GETTER]]'
186194
func objcProtocolOptionalRequirements() {
187-
_ = \ObjCProtoOptional.optionalProperty
188-
_ = \ObjCProtoOptional.[0]
195+
_ = \ObjCProtoOptional.object
196+
_ = \ObjCProtoOptional.[true]
197+
198+
_ = \ObjCProtoOptional.object!.name
199+
_ = \ObjCProtoOptional.[true]!.name
189200
}

‎test/expr/primary/keypath/swift-keypath-objc-optional-component.swift

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: %target-typecheck-verify-swift -disable-objc-attr-requires-foundation-module -enable-objc-interop
2+
3+
@objc class Object {
4+
var name: String
5+
6+
init(name: String) {
7+
self.name = name
8+
}
9+
}
10+
11+
@objc protocol P {
12+
@objc optional var object: Object { get set }
13+
14+
@objc optional subscript(_: Int) -> Object { get set }
15+
}
16+
17+
func assertExactType<T>(of _: T, is _: T.Type) {}
18+
19+
// An optional storage component makes the key path read-only...
20+
do {
21+
let kp_property = \P.object
22+
let kp_subscript = \P.[0]
23+
24+
var p: P
25+
// expected-error@+1 {{cannot assign through subscript: 'kp_property' is a read-only key path}}
26+
p[keyPath: kp_property] = Object(name: "nope")
27+
// expected-error@+1 {{cannot assign through subscript: 'kp_subscript' is a read-only key path}}
28+
p[keyPath: kp_subscript] = Object(name: "nope")
29+
30+
assertExactType(of: kp_property, is: KeyPath<P, Object?>.self)
31+
assertExactType(of: kp_subscript, is: KeyPath<P, Object?>.self)
32+
}
33+
34+
// ...unless a reference-writable component shows up later.
35+
do {
36+
let kp_propertyForce_name = \P.object!.name
37+
let kp_subscriptForce_name = \P.[0]!.name
38+
39+
let p: P
40+
p[keyPath: kp_propertyForce_name] = "yes"
41+
p[keyPath: kp_subscriptForce_name] = "yes"
42+
43+
assertExactType(of: kp_propertyForce_name, is: ReferenceWritableKeyPath<P, String>.self)
44+
assertExactType(of: kp_subscriptForce_name, is: ReferenceWritableKeyPath<P, String>.self)
45+
}

0 commit comments

Comments
 (0)