Yielding reference to underlying storage

So I am working on an implementation for Pitch: didSet Semantics and I need to tweak the modify coroutine to yield a reference to the underlying storage and call didSet (if there is no willSet and didSet doesn't reference oldValue).

It was suggested that a short-term simple solution would be to change the ReadWriteImplKind to be Modify. However, changing the accessor's storage's StorageImplInfo with ReadWriteImplKind::Modify can only be done if the WriteImplKind is Modify or Set and the ReadImplKind is either Get, Address or Read.

What would be the right combination of ReadImplKind & WriteImplKind here? cc @John_McCall

It seems like we yield a direct_to_impl reference to the storage if ReadImplKind is Get and WriteImplKind is Modify (and ReadWriteImplKind is Modify as well):

(brace_stmt implicit range=[/Users/suyashsrijan/Desktop/test.swift:2:7 - line:2:7]
  (yield_stmt implicit range=[/Users/suyashsrijan/Desktop/test.swift:2:7 - line:2:7]
    (inout_expr implicit type='inout Int'
      (member_ref_expr implicit type='@lvalue Int' decl=test.(file).Foo.value@/Users/suyashsrijan/Desktop/test.swift:2:7 direct_to_impl
        (declref_expr implicit type='Foo' decl=test.(file).Foo.<anonymous>.self@/Users/suyashsrijan/Desktop/test.swift:2:7 function_ref=unapplied)))))))

for something like:

class Foo {
  var value: Int {
    didSet { print("Hello") }
  }

  init(value: Int) { self.value = value }
}

let foo = Foo(value: 0)
foo.value = 1

Oh, sorry. I should have said that it makes sense to make the ReadWriteImplKind be StoredWithSimpleDidSet or InheritedWithSimpleDidSet, kindof like with the WriteImplKind.

1 Like

Thanks for clarifying! Just to make sure I am on the right path, I am gonna ask a couple of questions:

  1. In synthesizeCoroutineAccessorBody(), if the modify accessor's ReadWriteImplKind is StoredWithSimpleDidSet or InheritedWithSimpleDidSet AND the storage does not have willSet AND there is a didSet which doesn't refer to the oldValue then we build the body of the accessor differently by simply yielding a reference to the storage and calling didSet?
  2. You mentioned we need a direct_to_impl reference, I suppose that can be done by setting the storage's TargetImpl to be Implementation and let buildStorageReference() handle it?

I think you shouldn’t set this impl kind unless you know that there’s no willSet and the didSet is simple. But yes, that’s the right approach. We should already have code to generate a well-typed l-value reference to the underlying storage that’s used in the synthesis of the getter and setter for observed storage, and that should take care of all the direct-to-impl details for you.

1 Like

Thanks! I think it seems to work based on the generated SIL:

// Foo.value.modify
sil hidden [transparent] @test.Foo.value.modify : Swift.Int : $@yield_once @convention(method) (@guaranteed Foo) -> @yields @inout Int {
bb0(%0 : $Foo):
  debug_value %0 : $Foo, let, name "self", argno 1
  %2 = ref_element_addr %0 : $Foo, #Foo.value
  %3 = begin_access [modify] [dynamic] %2 : $*Int
  yield %3 : $*Int, resume bb1, unwind bb2

bb1:
  end_access %3 : $*Int
  %6 = function_ref @test.Foo.value.didset : Swift.Int : $@convention(method) (@guaranteed Foo) -> ()
  %7 = apply %6(%0) : $@convention(method) (@guaranteed Foo) -> ()
  %8 = tuple ()
  return %8 : $()

bb2:
  end_access %3 : $*Int
  unwind
}

I haven't added those ImplKinds as I can infer the presence/absence of the observers from the storage, but I'll add them now.

EDIT: oops, I think its still doing direct_to_storage, not direct_to_impl. Actually, when I call buildStorageReference() and pass TargetImpl::Implementation, then _modify still calls the setter. When I pass TargetImpl::Storage, then I get the SIL above. I am no SIL expert, so maybe that's not actually what the SIL should look like if everything is working properly.

Either TargetImpl::Storage or TargetImpl::Super is right, depending on whether the observers are added to an override to not; see how the setter is defined.

The ReadWriteImpl thing matters not for code-generation of the _modify accessor but for code-generation of a direct read-write access to the observed declaration.

1 Like

Thanks! I have updated the implementation to support this new feature.

1 Like