SE-0258: Property Wrappers (third review)

The review of SE-0258: Property Wrappers (previously called "Property Delegates") begins now and runs through July 3rd, 2019. This is the third review of SE-0258. After the first review, the Core Team requested further development of the proposal to address some concerns raised during the review. That development led to a major revision and a second review. After considering the early feedback during the second review, the proposal authors asked to make a second round of revisions, and the Core Team has decided that those changes are substantial enough to require a third review.

Doug Gregor has provided the following list of differences from the previous revision:

  • The synthesized property storage is always named with a leading _ and is always private; hence @Wrapper var foo synthesizes the storage private var _foo: Wrapper.

  • The wrapperValue property has been renamed to projectedValue to more clearly distinguish it visually from wrappedValue. This also gives us the term "projection" or "projected property" for talking about the $ property.

  • The projected property (e.g., $foo) always has the same access as the original wrapped property, rather than being artificially limited to internal. This reflects the idea that, for those property wrapper types that provide a projection, the projection is equal in importance to the wrapped value in the API of the property.

The rest of SE-0258 remains unchanged. Accordingly, the Core Team felt that it was reasonable to consider the feedback it received during the second review, at least as far as that feedback didn't relate to the aspects being further revised. We continue to believe that this is a highly worthy proposal, and we feel that the first round of revisions addressed most of the concerns we raised before. After the first review, we declared our acceptance of certain aspects of the proposal in the interest of forestalling further debate on points that we considered well-enough settled. We'd now like to augment that:

  • The Core Team accepts the proposed approach to wrapper composition and feels that any further refinements can reasonably be left as possible future work.

  • The Core Team accepts the spelling $property for a wrapper's primary projection.

  • The Core Team does not currently see sufficient justification for secondary projections (e.g. property$view) and is persuaded that this can be left as possible future work.

  • The Core Team is satisfied with the name "property wrapper" even though this may be inapposite for other kinds of variable in the future. The term property is not overly intrusive in the source, the alternatives seem over-technical and sometimes misleading, and properties are undeniably the most important subject for this pattern.

We would like to encourage reviewers to focus on the latest revisions and how they change your view of the proposal.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager via email or direct message on the forums. If you send me email, please put "SE-0258" somewhere in the subject line.

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thank you for contributing to Swift.

John McCall
Review Manager

33 Likes

+1

I’m still not convinced that projectedValue carries its weight, but the biggest problem with it since it was introduced was the way it changed the meaning of $foo. The clear separation of storage and projection in this version makes it both a useful and comprehensible design.

I’m not entirely happy about projections being implicitly public when the wrapped value is, but given their close association with the wrapped value and the $ sigil, I don’t foresee it actually bring a problem.

3 Likes

I feel good about the changes; definitely still +1. I stand by the rest of my previous review with no need to rehash any of it here.

Thanks for the hard work.

I very much dislike that change:
The underscore prefix looks like a C-legacy, and I think it would be better for Swift to prefer descriptive names over special characters without inherent meaning.
When people decide to use hungarian notation or cryptic variable names it's their right to do so, but baking it into the language is another story.

I personally would prefer property wrappers to be as simple as possible, and so besides the getter and setter, there should only be a way to access the wrapper itself (so object.foo could also be written as object.$fooWrapper.value).

6 Likes

This feature requires autogenerated unique names for each wrapped property, what are you proposing how it should be named!? object is clearly the wrong term here and would uglify the interface drastically.

I think the wrapper storage will remain private forever and _ prefix is still widely used across the community and stdlib.

7 Likes

I think object is just the base expression in Tino's example, not a new piece of syntax. Tino is saying that he doesn't think projections should exist, which would make the $foo syntax available for the synthesized name of the wrapper property. I don't think that's an approach we can take, but he's entitled to that opinion.

Also, please don't express incredulity like that; it's rude.

9 Likes

Quite happy with the improvements. I'm still giving a +1 :) And thanks to the core team and proposal authors to work on the feedback and get the proposal to where it is today. :clap:t2:

About the recent changes I'm happy with projectedValue. It is a new term that need to be learned but I prefer it to the visually confusing wrapperValue vs wrappedValue. :+1:t2:

:grey_question: One feature I would like still to see improved is a 3rd way of initialising. (I don't see it in the proposal so I'm assuming is not possible, correct me if I'm wrong):

Now we can pick between giving an initial value as normal Swift @Lazy var foo = 17 or if the wrapper has arguments we need to pass it to it directly @UserDefault(key: "FOO_FEATURE_ENABLED", defaultValue: false). What I would like to be able to do is to pass config options to the wrapper directly but keep the initial value with = X.

@UserDefault(key: "FOO_FEATURE_ENABLED")
static var isFooFeatureEnabled: Bool = false

Would this be possible?

I would accept the proposal as is anyway, I'm just wondering if is doable now or in the future ;)

3 Likes

Normally I'd leave that question for Doug, but I remember that one because it was a line item added in the second revision:

  • Initial values and explicitly-specified initializer arguments can both be used together; see the @Clamping example.
3 Likes

+1 for me, but I think that Changes from the first reviewed version should be updated. They still reference to wrapperValue vs wrappedValue with $ and $$

Not trying to be rude by any means, criticism or differences in opinions can sound sometimes inappropriate that‘s true though.

Your example should be possible but it requires some alignment:

init(initialValue: Value, key: String)

Such initializer with that order will do the trick allow you the syntax form you mentioned in your example.

+1
I am quite happy with separation of projectedValue and wrappedValue. But I think there is a scenario which can cause ambiguity:

// Always holds the initial value (Acts as UndoStackManager)
@propertyWrapper
struct Stacked<Value> {
    var wrappedValue: Value { get {...} set {...} }
    var projectedValue: Self { get {...} set {...} }

    func pop() -> Value? { }
    func mutateLast(_ mutation: (inout Value) -> Void)
    func reset() { }
}

class ViewController {
    @Stacked 
    var model: Model = .empty()
}

And now we might end up seeing this code:

// in the same file
extension ViewController {
    func doSomething() {
        _value.reset()
        $value.pop()
        _value.mutateLast { /* mutate $0 */ }
    }
}

_value and $value are referring to the same thing and are identical, but somehow it feels we're dealing with two different variables. In cases where projectedValue returns Self, _value and $value become identical, hence will affect coding-styles and add ambiguity for future readers.

It can even get weirder:

$value.wrappedValue == _value.wrappedValue
_value.mutateLast { _ in $value.wrappedValue = .empty() }
_value.mutateLast { model in $value.mutateLast { $0 = model } }

I feel just adding a guideline to only use _value inside the type-scope and use $value outside of type-scope is not sufficient either. Unless be forced by language which will limit projectedValue's abilities.

EDIT: You can ignore the ideas from this post, see my reasoning a few posts bellow, the proposal already suggests a more flexible future direction.



@Douglas_Gregor thank you again for the hard work. Do you think the following would be a reasonable generalization to consider?

Currently you require projectedValue to have the same access level as the property wrapper type. Together with the synthetization rule for the $ prefixed projected value we get an equal exposure.

Can we make this behavior a little lighter?

Here is what I have in mind:

  • Do not require projectedValue to have the same access level as the property wrapper
  • If the compiler can see projectedValue it will project a $ prefixed property but its exposure can be controlled
  • The projected property can only have a lower or equal access level then the wrapped property

In your own module you could have a public property Wrapper A with an internal property projectedValue. Then you apply that wrapper to a public property on the public type B. The compiler will generate internal var $foo: Value and limit the exposure to remain internal in B.

// In module X
@propertyWrapper
public struct A<Value> {
  ...
  internal var projectedValue: Value { ... }
}
// In module X - in file `B.swift`
public struct B {
  @A public var property_1: Value

  // results in
  public var property_1: Value { ... }
  internal var $property_1: Value { ... }
  private var _property_1: A<Value> = ...
}

Such an approach do not require any public(wrapper) addition in the future, but only require the property wrapper designer to apply the correct access level to projectedValue to control the exposure.

Also if C is a property wrapper type that is not nested in B and it has a private projectedValue then the compiler won‘t generate a projection in B at all.

// In module X
@propertyWrapper
public struct C<Value> {
  ...
  private var projectedValue: Value
}
// In module X - in file `B.swift`
public struct B {
  @C public var property_2: Value

  // results in
  public var property_2: Value { ... }
  private var _property_2: C<Value> = ...
}

If D has a private projectedValue but is nested in B then the compiler will create a private projected property in B.

// In module X - in file `B.swift`
public struct B {
  @propertyWrapper
  public struct D<Value> {
    ...
    private var projectedValue: Value
  }
  @D public var property_3: Value

  // results in
  public var property_3: Value { ... }
  private var $property_3: Value { ... }
  private var _property_3: D<Value> = ...
}

If E has a fileprivate projectedValue and it‘s in the same file as B the compiler can again synthetize a projected value.

// In module X - in file `B.swift`
@propertyWrapper
public struct E<Value> {
  ...
  fileprivate var projectedValue: Value
}

public struct B {
  @E public var property_4: Value

  // results in
  public var property_4: Value { ... }
  fileprivate var $property_4: Value { ... }
  private var _property_4: E<Value> = ...
}

These are just the same rules we have today and we could completely control the projection with it today. No additional syntax would be required.

If F is public and has a public projectedValue then the compiler will project a public property.

// In module X - in file `B.swift`
@propertyWrapper
public struct F<Value> {
  ...
  public var projectedValue: Value
}

public struct B {
  @F public var property_5: Value

  // results in
  public var property_5: Value { ... }
  public var $property_5: Value { ... }
  private var _property_5: F<Value> = ...
}

I do think that we should control the projection right from the beginning as it would be a breaking change to hide leaked projections later on. And having an ability to have internal projections that do not leak but provide you the $ syntax would be a great benefit.

2 Likes

I don't think the compiler would reason how projectedValue is actually implemented. It can be read only for example or do more things during the access. Therefore I think merging _value and $value when projectedValue is of type Self would complicate the design. Can you elaborate why in your example projectedValue is needed at all? You can omit it and then the user will be forced to use _value, but the access will be limited to the declaration file.

I also though about that approach. The main difference between your solution and public(wrapper) one is that with your solution, the visibility is imposed by the propertyWrapper, while the public(wrapper) solution let you define the visibility per property.

class Foo {
 @A var a;
 @A var b;
}

With your proposition, $a and $b must have the same visibility, while the class author may wish to give them different visibility.

I don't like the idea of having the visibility of a class property imposed by its wrapper type. It will prevent adding more flexible rules in future evolution.

3 Likes

That is totally true and we need to keep that in mind, but I think there is still an issue with that approach.

@propertyWrapper
public struct Wrapper<Value> {
  ...
  public projectedValue: Value { ... }
} 

public struct S {
  @Wrapper
  public internal(projection) var property: Value
}
  • Do we leak that property is wrapped by Wrapper as these parts are public?
  • If we do, wouldn't the client expect a projected value on S because Wrapped contains a public projectedValue that tells the compiler to project a property?! However we stated that we want to suppress that behavior explicitly. Do we leak internal(projection) to the client to tell him that there won't be any projected property?

You can compare this to this behavior:

public struct T {
  public private(set) var property: String
}

Correct me if I'm wrong, but IIRC .swiftinterface file will only contain public property: String { get }.

This confusion is why I am not fully sold on having both _ and $.

I could see an alternative approach where the wrapper is accessible via $ and a projection always has to be explicit.

Adapting an example from the Proposal:

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

let world = $someValue.pointer.move()
$someValue.pointer.initialize(to: "New value")

The benefits would be:

  • No confusion (in private scope) when to use "_" vs "$"
  • All potential projections are treated equal
  • No name clash of wrapper (i.e. storage) with a potential non-wrapper variable which uses a leading underscore
  • Explicit typing makes it clear what kind of projection is used at the use-sites

Downsides:

  • More typing
  • Wrapper will not be private by default
  • WWDC material is outdated etc

As John mentioned in the introduction of this thread - the Core Team will not open this particular discussion again. Maybe I am just missing a bit more rational in the Proposal why such a simple alternative is insufficient.

3 Likes

I saw this restriction in the Proposal:

  • A property with a wrapper may not be declared inside a protocol.

My question now - how can I make a projected-value of a wrapped property defined in a concrete type available through a protocol adopted by that concrete type?

1 Like

Now that I'm thinking it through, it's actually current proposal's shortcoming which is mentioned in future directions.

In my example I was just trying to make my wrapper accessible outside of the current type-scope, and with current implementations, that's only possible through projectedValue. So yeah, in full-version of this feature where you can define access-level of the storage, public(storage) will do the job, hence projectedValue is no longer needed. (IMHO instance.$value is a lot more readable than instance._value when you're outside of the type-scope though. Because $ means that this variable is special while _ doesn't say much)

I wasn't suggesting for compiler to merge projectedValue and wrappedValue. I actually don't have a solid solution for it per se. These are all the solutions that came to my mind, but all have shortcomings:

  • Disallow usage of _value when projectedValue is mirroring Self
    • causes inconsistency in code-style when dealing with storage
  • Disallow usage of $value inside the type-scope
    • will impact lots of features projectedValue will add to the language (e.g: @Published)

I think the only way to stop it from happening is following a good code-convention :/ (A warning can be raised as well)

_value will always be read write, while $value can be read only. Therefore disallowing _value might be too restrictive change here.

This likely won't happen as using $value closely to the wrapped property is one of the major selling points for projectedValue. Feel free to correct me if I misunderstood your point here.

2 Likes