[Returned for revision] SE-0258: Property Delegates

The review for SE-0258 has concluded. There was a lot of great discussion in this review, leading to some very useful feedback for the Core Team and the proposal authors. That feedback can be broken down into two categories: (1) procedural feedback about the state of the proposal and its review and (2) technical feedback about the actual proposal. Both are useful, and we'll consider each in turn.

Several members of the community felt that this proposal just wasn't ready for review, and the Core Team agrees with these concerns. Some aspects of the proposal seem a little under-justified, while others seem awkwardly designed. Both of these points suggest that further development is required, whether to improve the rationales in the proposal text or to substantively improve the design. Accordingly, the Core Team has decided to return the proposal for further design and development. Crucially, this result differs from rejection in that the Core Team accepts that this is a worthwhile feature in general and would be pleased to accept a version of this proposal that satisfies the concerns covered in this announcement. The Core Team feels that this result is consistent with the general tenor of the feedback received.

The Core Team wishes to establish a productive framework for further discussion, and so we are providing the following guidance about specific technical aspects of the proposal:

  • The Core Team accepts the general custom-attribute design laid out in this proposal as a useful way of generalizing existing modifiers (whether keyword- or attribute-based) and creating a namespace for future extensions.

  • The Core Team accepts the specific use of custom attributes in this proposal for specifying the delegate type. We are not concerned that this will exhaust the attribute namespace or "claim" it for just this one feature. We are also convinced, after some preliminary investigation, that choosing an attribute-based spelling will not preclude composition of delegates in the future.

  • The Core Team agrees with the proposal that composition of delegates is not necessary to solve in the initial design.

  • The Core Team accepts the proposal's use of an attribute and ad hoc requirements to define a delegate type rather than using a formal protocol. A formal protocol would substantially limit the expressivity of the feature.

  • The Core Team feels that the proposal fails to justify delegateValue well enough for it to be evaluated. This feature was developed late in the pitch phase and added to the proposal without much discussion, and it adds a substantial amount of conceptual complexity. The proposal should explain the purpose of this feature well enough that the community (and Core Team) can meaningfully evaluate the proposed approach and consider alternatives to it.

  • The Core Team feels that the ability to explicitly specify getters and setters is similarly inadequately justified.

  • The Core Team agrees with the community that the inability to control access to the delegate storage is a serious concern. It's okay in principle for a proposal to just pick a reasonable default rule like this for certain language interactions, but we do need some assurance that we're not going to be stuck with bad compromises in the future. For something like access control, establishing that there's room for an acceptable design basically means doing most of the work, so the proposal really just ought to include it.

  • The Core Team agrees with the community that "property delegates" isn't a good name for this feature. First, the feature isn't fundamentally about delegation, it's about giving the property different behavior. Second, it's not hard to imagine that we might someday add a feature that is fundamentally about delegation, i.e. something to make it easier to define storage as an alias for other storage. Finally, "delegate" has an idiomatic meaning in Apple UI programming that's quite different from this. Among the better names suggested in the review:

    • "property behaviors", even though that name was already used for a similar-but-rejected feature in the past
    • "property wrappers"
    • "property decorators"

I'd like to thank the community for its patience and its commitment. I know we've had a lot of proposals recently, and some of them have been contentious, and it can be a lot of work to keep up with Swift Evolution. You really are appreciated; thank you for everything you do to help make Swift a better language.

John McCall
Review Manager

42 Likes

Can you explain this better? I’m not sure I understand.

The proposal allows the storage to define a getter or setter itself; it simply defaults whichever is missing.

I have a bit of a process question. Why is it that the proposal was returned for revision, yet the implementation has already been merged to master? I'm trying not to be some kind of stick-in-the-mud here, but I don't quite understand how it can be returned for revision, yet already merged into the codebase.

I understand that it could be ripped out if, for whatever reason, this feature becomes untenable. But now that it's in, the amount of work to remove it will gradual increase over time. However, this feels like an indication that this proposal is almost certain to be accepted in some form, regardless of what discussions happen here on SE. So what exactly is the point of returned for revision in this case vs just accepting it pending some convergence, which sounds extremely wrong in many senses.

2 Likes

That's a very reasonable question. We chose to merge this change because the infrastructure to support it is both fairly invasive (and thus awkward to maintain in a branch) and relatively general (and thus useful for other features in development). That infrastructure — e.g. the code to support custom attributes in general — is unlikely to need significant rework from these revisions. The code that will need revision is mostly superficial and self-contained, and in our experience, other features don't develop tendrils into code like that which would make it hard to change or remove. And we're not worried about users coming to rely on the feature because it's currently spelled with an underscore, flagging it as private/experimental, which means that any such uses are Not Our Problem.

21 Likes

That's what I guessed, and makes sense to me. I'm sure every developer has felt the pain of trying to maintain a feature branch against an ever changing upstream branch; it can be quite maddening.

That tells a lot about "our community" ;-) - but actually, I think that this "merge but flag" tactic could be a big improvement for the whole evolution process, especially bigger proposals like this:
It allows a larger audience to play with a feature and find its rough edges, so that problems can be fixed without breaking compatibility with an official release.

Imho it would have been better if the merge had been communicated in the conclusion of the review, though.

8 Likes

Synthesis for Encodable , Decodable , Hashable , and Equatable using the underlying value of the property when the property delegate type contains an init(initialValue:) and the delegate's type otherwise.

I’m concerned this could be confusing and unexpected. I’d prefer to just always use the delegate’s type. I don’t think you can assume based on the initializer alone that the value is sufficient. With synthesized implementations I don’t think there’s any additional burden on the programmer from using the delegate’s type always.

An example:

@RxProperty var x: Int

init(source:  Signal<Int>) {
   $x = RxProperty(source: source, initialValue: 10)
   $x.sendNext(100)
}

This might still have an init(initialValue:), it doesn’t use it here. The property might want to use reference semantics for equality.

Being able to be initialized with some state doesn’t mean that the type is equivalent to that state.

Alternatively with DelayedMutable you can define == without needing the value to be set yet, but trying to compare the values themselves would crash. It’s entirely reasonable for it to have init(initialValue:) though, but it may not be used every time it’s initialized.

Another example:

@Rounded var x: Double = 1.234
print(self.x) // 1
$x.places = 2
print(self.x) // 1.23

JSONEncoder().encode(self) // {“$x”: {“value”: 1.234, “places”: 2}}

Perhaps use the delegate’s type if it conforms to the protocol, otherwise use the value’s type of it does.

1 Like