CS: optional storage key path components are read-only

lib/IRGen/GenObjC.cpp

@@ -1279,7 +1279,10 @@ irgen::emitObjCGetterDescriptorParts(IRGenModule &IGM,
 ObjCMethodDescriptor
 irgen::emitObjCSetterDescriptorParts(IRGenModule &IGM,
                                      VarDecl *property) {
-  assert(property->isSettable(property->getDeclContext()) &&
+  // Optional properties support mutation on the Objective-C side, but not the
+  // Swift side.
+  assert((property->getAttrs().hasAttribute<OptionalAttr>() ||
+          property->isSettable(property->getDeclContext())) &&
          "not a settable property?!");
 
   Selector setterSel(property, Selector::ForSetter);
@@ -1320,7 +1323,11 @@ irgen::emitObjCSetterDescriptorParts(IRGenModule &IGM,
 ObjCMethodDescriptor
 irgen::emitObjCSetterDescriptorParts(IRGenModule &IGM,
                                      SubscriptDecl *subscript) {
-  assert(subscript->supportsMutation() && "not a settable subscript?!");
+  // Optional subscripts support mutation on the Objective-C side, but not the
+  // Swift side.
+  assert((subscript->getAttrs().hasAttribute<OptionalAttr>() ||
+          subscript->supportsMutation()) &&
+         "not a settable subscript?!");
 
   Selector setterSel(subscript, Selector::ForSetter);
   ObjCMethodDescriptor descriptor{};

lib/Sema/TypeCheckStorage.cpp

@@ -3309,7 +3309,9 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
     WriteImplKind writeImpl = WriteImplKind::Immutable;
     ReadWriteImplKind readWriteImpl = ReadWriteImplKind::Immutable;
 
-    if (storage->getParsedAccessor(AccessorKind::Set)) {
+    // TODO: Writing to optional storage requirements is not supported.
+    if (!storage->getAttrs().hasAttribute<OptionalAttr>() &&
+        storage->getParsedAccessor(AccessorKind::Set)) {
       readImpl = ReadImplKind::Get;
       writeImpl = WriteImplKind::Set;
       readWriteImpl = ReadWriteImplKind::MaterializeToTemporary;

test/SILGen/keypaths_objc.swift

@@ -158,32 +158,43 @@ func dynamicMemberLookupMixedKeypaths(foo: DynamicClass<Foo>) {
   _ = foo.bar.foo.nonobjc.y 
 }
 
+@objc class Object: NSObject {
+  var name: String
+  init(name: String) {
+    self.name = name
+  }
+}
 @objc protocol ObjCProtoOptional {
-  @objc optional var optionalProperty: Bool { get }
+  @objc optional var object: Object { get set }
 
-  @objc optional subscript(_: Int) -> Bool { get }
+  @objc optional subscript(_: Bool) -> Object { get set }
 }
 
 // CHECK-LABEL: sil hidden [ossa] @{{.*}}0B28ProtocolOptionalRequirementsyyF
-// 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]+]]
-// 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]+]]
+// 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]+]]
+// 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]+]]
+// 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,
+// 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,
 // CHECK: } // end sil function '${{.*}}0B28ProtocolOptionalRequirementsyyF'
 //
-// CHECK: sil shared [thunk] [ossa] @$[[PROP_GETTER]] : $@convention(thin) (@in_guaranteed ObjCProtoOptional) -> @out Optional<Bool> {
+// CHECK: sil shared [thunk] [ossa] @$[[PROP_GETTER]] : $@convention(thin) (@in_guaranteed ObjCProtoOptional) -> @out Optional<Object> {
 // CHECK:   [[BASE:%[0-9]+]] = open_existential_ref {{%[0-9]+}} : $ObjCProtoOptional to $[[OPENED_TY:@opened\("[-A-F0-9]+"\) ObjCProtoOptional]]
-// CHECK:   dynamic_method_br [[BASE]] : $[[OPENED_TY]], #ObjCProtoOptional.optionalProperty!getter.foreign, bb1
-// CHECK: bb1({{%[0-9]+}} : $@convention(objc_method) ([[OPENED_TY]]) -> ObjCBool)
+// CHECK:   dynamic_method_br [[BASE]] : $[[OPENED_TY]], #ObjCProtoOptional.object!getter.foreign, bb1
+// CHECK: bb1({{%[0-9]+}} : $@convention(objc_method) ([[OPENED_TY]]) -> @autoreleased Object)
 // CHECK: } // end sil function '$[[PROP_GETTER]]'
 //
-// CHECK: sil shared [thunk] [ossa] @$[[SUBSCR_GETTER]] : $@convention(thin) (@in_guaranteed ObjCProtoOptional, UnsafeRawPointer) -> @out Optional<Bool> {
+// CHECK: sil shared [thunk] [ossa] @$[[SUBSCR_GETTER]] : $@convention(thin) (@in_guaranteed ObjCProtoOptional, UnsafeRawPointer) -> @out Optional<Object> {
 // CHECK:   [[BASE:%[0-9]+]] = open_existential_ref {{%[0-9]+}} : $ObjCProtoOptional to $[[OPENED_TY:@opened\("[-A-F0-9]+"\) ObjCProtoOptional]]
-// CHECK:   [[INDEX:%[0-9]+]] = load [trivial] {{%[0-9]+}} : $*Int
+// CHECK:   [[INDEX:%[0-9]+]] = load [trivial] {{%[0-9]+}} : $*Bool
 // CHECK:   dynamic_method_br [[BASE]] : $[[OPENED_TY]], #ObjCProtoOptional.subscript!getter.foreign, bb1, bb2
-// CHECK: bb1({{%[0-9]+}} : $@convention(objc_method) (Int, [[OPENED_TY]]) -> ObjCBool):
-// CHECK:   %17 = apply {{%[0-9]+}}([[INDEX]]) : $@callee_guaranteed (Int) -> Bool
+// CHECK: bb1({{%[0-9]+}} : $@convention(objc_method) (ObjCBool, [[OPENED_TY]]) -> @autoreleased Object):
+// CHECK:   %17 = apply {{%[0-9]+}}([[INDEX]]) : $@callee_guaranteed (Bool) -> @owned Object
 // CHECK: bb2:
 // CHECK: } // end sil function '$[[SUBSCR_GETTER]]'
 func objcProtocolOptionalRequirements() {
-  _ = \ObjCProtoOptional.optionalProperty
-  _ = \ObjCProtoOptional.[0]
+  _ = \ObjCProtoOptional.object
+  _ = \ObjCProtoOptional.[true]
+
+  _ = \ObjCProtoOptional.object!.name
+  _ = \ObjCProtoOptional.[true]!.name
 }

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

@@ -0,0 +1,45 @@
+// RUN: %target-typecheck-verify-swift -disable-objc-attr-requires-foundation-module -enable-objc-interop
+
+@objc class Object {
+  var name: String
+
+  init(name: String) {
+    self.name = name
+  }
+}
+
+@objc protocol P {
+  @objc optional var object: Object { get set }
+
+  @objc optional subscript(_: Int) -> Object { get set }
+}
+
+func assertExactType<T>(of _: T, is _: T.Type) {}
+
+// An optional storage component makes the key path read-only...
+do {
+  let kp_property = \P.object
+  let kp_subscript = \P.[0]
+
+  var p: P
+  // expected-error@+1 {{cannot assign through subscript: 'kp_property' is a read-only key path}}
+  p[keyPath: kp_property] = Object(name: "nope")
+  // expected-error@+1 {{cannot assign through subscript: 'kp_subscript' is a read-only key path}}
+  p[keyPath: kp_subscript] = Object(name: "nope")
+
+  assertExactType(of: kp_property, is: KeyPath<P, Object?>.self)
+  assertExactType(of: kp_subscript, is: KeyPath<P, Object?>.self)
+}
+
+// ...unless a reference-writable component shows up later.
+do {
+  let kp_propertyForce_name = \P.object!.name
+  let kp_subscriptForce_name = \P.[0]!.name
+
+  let p: P
+  p[keyPath: kp_propertyForce_name] = "yes"
+  p[keyPath: kp_subscriptForce_name] = "yes"
+
+  assertExactType(of: kp_propertyForce_name, is: ReferenceWritableKeyPath<P, String>.self)
+  assertExactType(of: kp_subscriptForce_name, is: ReferenceWritableKeyPath<P, String>.self)
+}