[Pitch #2] Property Delegates by Custom Attributes

Yes, it's fine for storageValue to be a stored property.

Yes, that's all correct!

Doug

Yeah, I think this is perhaps an argument that we need something like the private(storage) bit described in future directions, here: https://github.com/DougGregor/swift-evolution/blob/property-delegates/proposals/NNNN-property-delegates.md#specifying-access-for-the-storage-property

Doug

There seems to be a lot of signal that private(storage) is considered an important part of this feature. Perhaps I should drag it out of future directions and into the proposal proper.

Heh, go for it.

As others have noted, I would like these to be separate, follow-on proposals. This proposal is already fairly large, and because I'm going for breadth in my examples, none of them have gotten sufficient care and feeding to necessarily be good API.

I think these are possible to do, but I haven't tried yet. unowned and weak are tied into the compiler, but we could potentially put property-delegate veneers over them to eventually hide the keyword versions.

Others have answered, but I added a short blurb to future directions so the proposal at least acknowledges that it's not trying to solve this now.

If the compiler is doing the composition, we could possibly avoid having a PropertyDelegate protocol by nesting the delegates (Lazy<Atomic<Int>>) and having the synthesized computed property access $property.value.value. I haven't thought about this route much.

Doug

1 Like

Even if this is an important thing to consider in the long term, why bog down the existing proposal with something that is controversial and non-obvious. It seems like you either rush the longer term design or delay the basic proposal.

It seems sensible to land the base proposal, get actual experience with it (not theoretical desires) and re-evaluate this based on that practical experience.

-Chris

2 Likes

No, that would not be sufficient.

In my use case, if I have a @Variable var bar: String, I always need access to the underlying storage. Since that's where the .subscribe method lives.
If the Variable<String> storage field is private, I can no longer subscribe.

I think what I would need is something like this:

@Variable public public(storage) private(set) var bar: String

Where the setter for bar becomes private, but that also makes the setter for $bar private.

And to complicate matters more, Variable also has a setValue(_ value: T, animated: Bool) function that also needs to become private, if the setter of bar is private...
(Or maybe fileprivate, so that the enclosing class can still access it)

Something like this could work, as an extension to the definite initialization rules. Perhaps we'd call it @uninitialized self or similar. It's worth investigating in a follow-on proposal; the self issue is way too big to tack on to this already-large proposal.

This has come up a number of times in this thread, and I disagree. I feel like this logic hinges on the name of the delegate type not conveying what is going on. If the word Lazy in @Lazy doesn't convey enough meaning (I think it does), it's just a type---look it up and see what it does. It should have a documentation comment that makes it clear. I think the @delegate(...) suggestions come from a perspective of unfamiliarity that will wear off very, very quickly.

Ah, I've always used "property" as the general term, encompassing var and let declarations in any context, with/without getters and setters, etc. I do think it's important for the attribute that goes on the delegate type to mention what kind of delegate it is ("property delegate", "variable delegate", whatever).

You can declare a @Foo let x: Int that suppresses the setter, or @Foo private(set) var x: Int to make it private.

Doug

That's a very densely qualified declaration. You might want to take a look at the storageValue stuff in the proposal to see if that might address your use case somehow.

Doug

I donā€˜t really want to start any debate about access control now as per proposal it should deferred, but Iā€˜d like to mention one or two things in that regard.

If one would expose the storage there will be one more issue. The storage itself can be mutable, but the developer might not want the user to mutate the whole storage, which means it could have theoretically public private(set) access level.

So what about if we allow access modifier before the delegate attribute and prefer that to be written above the delegated property?

Assuming you donā€˜t want to allow your users to mutate the whole storage property:

public private(set) @Variable
public private(set) var bar: String

Unsurprisingly, there's conflicting signal here, but I am certainly inclined to limit additional growth of the proposal. We have quite a pile of nifty examples to start with, and much to learn about this feature. The implementation is itself close to ready.

Doug

2 Likes

@Douglas_Gregor can you elaborate on storageValue please? This part your revised proposal is totally new and quite magical. There were several mentions in this thread that itā€˜s quite something unexpected.

I also provided an alternative idea to this in a different thread as I think you should apply dynamicMemberLookup with key-path member lookup the delegate with storageValue to achieve nearly the same functionality you propose but also make it type-safe, predictable and more importantly not magical.

I added some more examples to the proposal. including a Ref / Box example that ties together key path dynamic member lookup with this proposal.

Doug

While this might rather warrant a meta discussion, this seems like it doesn't fit the evolution process really. The problem with that approach is that the "experiment" is already baked into the language then. While I do agree that your suggested approach sounds actually best, I think this would be better suited by having the much talked about "Boost process" to support such advancement in general.

My comment should not suggest that the current implementation of the proposal is half-baked or not worthy.

Sry for the off-topic.

4 Likes

I just read the new examples and it still does not address my issue. Why do we need storageValue at all? This seems to be the duty for 'key path dynamic member lookup' in first place.

// The proposal says this should be: Ref<Rectangle>
let rect2 = $rectangle 
// This is shadowing and instead should be: Box<Rectangle>

This whole part of the proposal is kind of like implicit 'key path dynamic member lookup' and as others (including myself) pointed out:

storageValue is about truly hiding the delegate instance, so you can control the API that's visible via $foo. In the Ref / Box example, you really want the box to be hidden, because the data should be accessible by value (foo) or by abstracted reference ($foo).

Doug

Oh, I see that my text got mangled a bit. I'll try to clean it up and push an updated version of the proposal.

Doug

I do kind of understand where you want to go with this, but as I mentioned this a few times already. This behavior is new and was only introduced with this revision of the proposal. I'm concerned about it as it's not what property delegates originally aimed for and adds more cognitive load to the Swift users.
If we really wanted something like this, I think it would be best to introduce this in a separate future proposal. Right now it feels like we want to sneak in a feature that not everyone will be aware of, let alone fully understand it.

@Douglas_Gregor About the name. Can you call it @synthesizedStorage instead?

the delegation part is really not the main feature and only mentioned at the very end https://github.com/DougGregor/swift-evolution/blob/property-delegates/proposals/NNNN-property-delegates.md#delegating-to-an-existing-property

the word synthesize its all over the place in the doc.

I can re-write all the storageValue examples using @dynamicMemberLookup:

protocol Copyable: AnyObject {
  func copy() -> Self
}

@propertyDelegate
@dynamicMemberLookup
struct CopyOnWrite<Value: Copyable> {
  init(initialValue: Value) {
    value = initialValue
  }

  private(set) var value: Value

  var storageValue: Value {
    mutating get {
      if !isKnownUniquelyReferenced(&value) {
        value = value.copy()
      }
      return value
    }
    set {
      value = newValue
    }
  }

  subscript<T>(dynamicMember member: KeyPath<Value, T>) -> T {
    mutating get {
      if !isKnownUniquelyReferenced(&value) {
        value = value.copy()
      }
      return value[keyPath: member]
    }
  }

  subscript<T>(dynamicMember member: WritableKeyPath<Value, T>) -> T {
    mutating get {
      if !isKnownUniquelyReferenced(&value) {
        value = value.copy()
      }
      return value[keyPath: member]
    }
    set { value[keyPath: member] = newValue }
  }
}


@CopyOnWrite var storage: MyStorageBuffer

// Non-modifying access:
let index = storage.index(of: ā€¦)

// this uses key-path dynamic member lookup now
$storage.append(ā€¦)
class StorageManager {
  func allocate<T>(_: T.Type) -> UnsafeMutablePointer<T> { ... }
}

@propertyDelegate
@dynamicMemberLookup
struct LongTermStorage<Value> {
  let pointer: UnsafeMutablePointer<Value>

  init(manager: StorageManager, _ initialValue: Value) {
    pointer = manager.allocate(Value.self)
    pointer.initialize(to: initialValue)
  }

  var value: Value {
    get { return pointer.pointee }
    set { pointer.pointee = newValue }
  }

  var storageValue: UnsafeMutablePointer<Value> {
    return pointer
  }

  subscript<T>(
    dynamicMember member: KeyPath<UnsafeMutablePointer<Value>, T>
  ) -> T {
    return storageValue[keyPath: member]
  }
}

let manager = StorageManager(...)

@LongTermStorage(manager: manager, initialValue: "Hello")
var someValue: String

print(someValue)     // prints "Hello"
someValue = "World"  // update the value in storage to "World"

// this uses key-path dynamic member lookup
let world = $someValue.move()
$someValue.initialize(to: "New value")
@propertyDelegate
@dynamicMemberLookup
class Box<Value> {
  var value: Value

  init(initialValue: Value) {
    self.value = initialValue
  }

  var storageValue: Ref<Value> {
    return Ref<Value>(read: { self.value }, write: { self.value = $0 })
  }

  subscript<T>(dynamicMember member: KeyPath<Ref<Value>, T>) -> T {
    return storageValue[keyPath: member]
  }
}

I think we should not do that until we resolved access control on property delegates, as otherwise this would be just unnecessary visual noise. The API user does not need to know about the implementation detail unless they are explicitly exposed.

Iā€™m really not happy with the storageValue thing. $foo to access the delegate is magical enough as it is; having an arbitrary property on the delegate redefine what $foo means is bizarre. I see two simpler options:

  • Always expose the delegate; the convenience of storageValue isnā€™t obviously very great. The COW :cow: example in particular would be better written as $storage.uniqued() or some such. @DevAndArtistā€™s key path approach makes sense for Box/Ref.
  • Never automatically expose the delegate; only create a $ property if thereā€™s a storageValue [name bikeshedding needed]. If the delegate wants to expose itself, it can declare var storageValue: Self { return self }.

The first option is more approachable and makes the purpose of having a $ property in the first place much clearer. Also, Iā€™m on team $-is-unncessary-just-use-_. :slight_smile:

Separately, the Atomic example reads like a cautionary tale. With atomics, exposing get and set-and-discard as extra easy operations is risky, and the first usage example has a likely bug as a result. This is effectively using delegates to get implicit conversions in inappropriate places (my new band name).

Wait, what? As far as I know, this would be the first feature that lets you add side-effecting behaviour to lets, since computed and lazy let arenā€™t permitted, presumably for this reason. (Iā€™d like to see computed and lazy let when/if Swift gets a ā€œpure subsetā€ as per other discussions.)

2 Likes