SE-0258: Property Wrappers (second review)

Can't one effectively raise the visibility level of the synthesized property simply by declaring a (public) wrapperValue getter that returns Self?

No, more complex than that. You can raise it from private to internal, but not past. There’s no way in the current proposal to make the backing store public.

1 Like

I found a few places in the proposal text and examples which look like inconsistencies. If they are, I think it would worth fixing them to help people understand this fairly complex proposal:

Proposed solution

var $foo: Lazy<Int> = Lazy<Int>(initialValue: 1738)
var foo: Int {
  get { return $foo.value }
  set { $foo.value = newValue }
}

Shouldn’t the wrapper property be private?

Composition of property wrappers

var $path: DelayedMutable<Copying<UIBezierPath>> = .init()
var path: UIBezierPath {
  get { return $path.value.value }
  set { $path.value.value = newValue }
}

Same comment as above.

Initialization of synthesized storage properties

This section does not mention the initialization that allows declaring the wrapper with arguments while also receiving the initial value through the property declaration. So there are technically four ways to initialize a property wrapper.

Secondly, this section also mentions:

When there are multiple, composed property wrappers, only the first (outermost) wrapper may have initializer arguments.

This sounds weird because I don’t see why the limitation exists. It is also in contradiction with an example a few sections below where it’s the last inner most wrapper that has initialized argument.

@A @B(name: "Hello") var bar = 42
// type inference as in ...
var $bar = A(initialValue: B(initialValue: 42, name: "Hello"))
// infers the type of '$bar' to be 'A<B<Int>'

Detailed design

I’m if I’m not mistaken, the proposal doesn’t clearly state that the generated property wrappers are private. If that’s the case, could a small section be written about that under Detailed design?

3 Likes

As to the proposal in itself, I am super happy with how much it has improved since it’s earlier iterations. I’m especially delighted about wrappers being able to be declared with arguments as well as being initialized with the initial value.

Nonetheless, I have the feeling that it would be important to address the accessibility level question before we bake in a private default. Like @tanner01, I would really prefer the wrapper (and wrapperValue) to default to the same accessibility level as the property and allow tweaking them with a private(wrapper) modifier.

5 Likes

Could you clarify what is synthesised in the various cases. For example, in the UserDefaults case it seems we don't need any stored properties at all. The simple value is always returned through a computed property for all cases (right?) and for UserDefaults the storage doesn't require any stored property either since it is also computed. Is there a way to avoid creating stored properties? That would be very convenient when extending types. Though that could lead to abuse.

I am (still) very very +1 on this feature and direction, thank you for diligently pushing this forward and iterating. The changes are mostly all very sensible, this is a great step forward! Also, thank you for providing a "diff" in the second review notes, that is very helpful.

Most of the changes are great, but I'm confused about the motivation for the $$ / wrapperValue stuff: it is complicated, confusing, and makes the user side syntax worse. I'd love to see more elaborate explanation of why this is an important addition to the proposal, because honestly I'm just confused and don't get it.

If it turns out that we really do need "two things" on the caller side, then I recommend making these changes:

  • Move the prefix x.$foo thing to use postfix $ for the normal wrapped thing, so we have x.foo and x.foo$. We can explain the $ as meaning "storage".
  • Move the x.$$foo thing to have a name, e.g. x.foo$something where "something" is a word that describes what this thing is (I'm still confused, so I can't come up with a concrete suggestion).

This achieves several goals: 1) it allows an using a word for the thing instead of yet-another magic dollar sign. 2) it makes these things work better in code completion, 3) it provides room for future expansion when (in the future) the proposal is inevitably feature crept even further forward 4) it solves the LLDB naming conflict.

On the declaration side, I find the names wrapperValue and wrappedValue to be so close to be confusing and easy to mistake. If we need to have two, it would be great to make them be more lexically different. I'm not sure why value got renamed (some oblique mention about a name collision)?

Caveat that I don't really understand why we need this thing in the first place, but I find this access control answer to be super confusing and weird. I thought we agreed to make it private - not make one thing private and add a new non-private thing?

What is the use-case? This proposal is obviously motivated by SwiftUI - can you add SwiftUI examples to the proposal to be more transparent and direct? It would make this so much easier to understand what you're actually achieving and why.

Yes. Yes. N/A. Tons.

-Chris

19 Likes

I like this direction. Right now, as I see it, Property Wrapper is doing 2 things.

  • Add common getter and setter boilerplate. (via wrappedValue)
  • Create access to relatedValue of a property. (via wrapperValue)

Both are very close to each other, and it's almost impossible to add them both orthogonally (at least, I think), perhaps because both features use base storage.

Anyhow, I feel like base storage is not exactly usable on its own. It's very common to use function/accessor on it. So I think suffix $ would work well here.

While wrapperValue (2nd feature) is generally useful as a variable on its own, and you may not even need to access the variable's function/accessor. Prefix $ would be appropriate here.

So

@propertyWrapper
struct Wrapped {
  var wrappedValue: ... { ... }
  var wrapperValue: ... { ... }

  func someWrapperFunction() { ... }
}

@Wrapped var foo = ...

foo$someWrapperFunction() // This access baseStorage.someWrapperFunction
foo$self // This access baseStorage.self
$foo // This access wrapperValue

As I think baseStorage and wrapperValue have different use cases, this should work.

If using suffix only on baseStorage doesn't work, using suffix on both of them doesn't seem like a bad idea either.

1 Like

See @tanner0101's comments above for a non-SwiftUI example of why just making the backing store private is not a viable solution.

2 Likes

Why not just use \.name?

Agreed.

My suggestion:

wrapperValue should be renamed to wrapperBinding and that should be the only thing exposed via $. If i need access to the backing storage I should be able to access via $someProp.wrappedValue

1 Like
  • What is your evaluation of the proposal?

I love the idea of this proposal and it definitely seems to be well refined at this point. I'm willing to accept this as is.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes. I often find myself reaching for a solution like this. It's a great example of removing boilerplate and making code easier to read and reason about. The fact that it has the potential to replace several builtin features is another clue that it's a good idea.

  • Does this proposal fit well with the feel and direction of Swift?

Sure.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Nothing comes to mind.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I've read the proposal (and some previous versions of it) and experimented with the Xcode 11 beta and with SwiftUI.


My biggest complaint about this proposal is delegateValue/wrapperValue. It seems kind of magical and non-vital. I can see why SwiftUI would want this feature, but it took me forever to figure out how State was being magically transformed into a Binding, even after reading an earlier version of this proposal. This seems like something that is going to make the language harder to understand, even if it makes SwiftUI easier to learn. I would be in favor of renaming it to something longer and more self documenting, or eliminating it altogether (and forcing things like SwiftUI to use $foo.binding).

I'm also disappointed to see that referencing the enclosing self isn't possible in this iteration. One use case I'd like to use this for is to replace @NSManaged with different behaviors, but that would require having access to methods on the enclosing object. The proposed solution for that in future directions looks good to me though.

I'd also like to see more clarification on how this interacts with Codable. The other generated conformances seem obvious enough, but could this be used to do lightweight customization of the Codable process? For instance, changing how a single property gets encoded/decoded without implementing Codable for the entire type? Could you customize the key that it used like: @Codable(key: "user_name") var name: String?

6 Likes
  • What is your evaluation of the proposal?

First, I want to thank @Douglas_Gregor for working so hard on this proposal. This has been a long haul and it has been very much worth the effort. The proposal has improved dramatically and I’m a huge fan!

The one disappointing detail for me is that we have yet another last minute surprise with the introduction of $$. I thought part of the motivation of wrapperValue was to hide the instance of the wrapper from users. In fact, the proposal still contains this sentence:

A property wrapper type can choose to hide its instance entirely by providing a property named wrapperValue .

This is an important aspect of what sold me on wrapperValue. If we take away this hiding then I don’t think wrapperValue carries its weight. It would place no new tools in the hands of library authors. It would just be come rather fancy and potentially confusing syntactic sugar that says $foo becomes $$foo and $foo.binding becomes $foo (we would use a meaningful name like binding if wrapperValue was not available).

On the other hand, the original design simply allows wrapperValue to redefine $foo. This is still magical, but it provides important benefits to both users and library authors. Users get to avoid having to type .binding and the library is able to hide the instance of its wrapper type.

The cost of this feature is that users who don’t understand how wrapperValue works will find it a bit magical. If we’re going to have it we should keep the amount of magic to a minimum and make sure it delivers the maximum value that it can.

While we evaluate wrapperValue we must keep in mind that while $foo.binding (and similar) over and over would get noisy and annoying it is substantially more clear about what is going on. This also has benefits, especially for less experienced programmers.

Overall, I think wrapperValue is a valuable feature, but it is only worth the cost in its previous form. The new version adds complexity for users while giving up an important encapsulation capability for library authors.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Absolutley!

  • Does this proposal fit well with the feel and direction of Swift?

It fits very well.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

N/A

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I have been actively involved in all phases of this proposal.

8 Likes

I've been following property wrappers for awhile, but about 1/3 between when it started and now I stopped following it until after SwiftUI's announcement at WWDC. I dropped it off my radar because the feature was getting pretty technical and confusing with what the capabilities were and the requirements for implementation.

This latest proposal is almost at 100% - but I want to echo sentiments @tanner0101 gave:

I'm consistently having to explain to new Swift programmers the difference between all of the access modifiers and how to restrict property access due to the state of Swift's access control, and I feel like this adds yet another complication of "it depends" to something that should be much more straight forward.

What is the compelling use case of locking down the access to internal as the maximum? I haven't seen it explained why this isn't an (seemingly) arbitrary decision.

7 Likes

I suggest associatedValue as a replacement for wrapperValue. I think it could ease teaching: "Use foo to access the property value, as usual, and $foo for the associated value that provides extra functionalities such as frobnication".

The renaming of value to wrappedValue is indeed, if I understand well, an attempt at giving room to SE-0252 Key Path Member Lookup, which is the new hotness. I'm not sold on this. It looks like SE-0252 puts pressure on API designers, just in case someone would like to add some syntactic sugar. This pressure leads to obfuscated identifiers that reduce the probability of name collision. This is an unexpectedly bad side effect of SE-0252, and I think we should try to resist to this pressure first, refuse preventive obfuscation, and look for a better support for SE-0252 eventually.

My opinion:

  • Let's rename wrappedValue to value
  • Let's rename wrapperValue to associatedValue
2 Likes

Tentative -1.

I do not personally think that this feature should be adopted without serious reconsideration of the access control issue stated by @tanner0101. As it stands, I find the access control proposal to be disjunct from what I see as one of the major benefits of Swift: an expressive language that reads easily and communicates ideas clearly. Locking down access control for this feature is antithetical to this and would make it more confusing and more difficult for this feature to be used in production code. In the interest of full transparency, I am speaking from a Server Side Swift perspective here, but I do think that these concerns apply to iOS as well.

Aside from that, I am also concerned about the implications of wrappedValue and wrapperValue. I really like the suggestion put forth by @gwendal.roue to rename to value and associatedValue.

Apart from those two issues, I am a big +1 on the feature. Property wrappers seem like a logical step towards mixing in dynamic-like properties into the language without convoluting the type system. Huge props to @Douglas_Gregor.

Absolutely, 100%

Definitely. This feels incredibly "Swifty" and I'm happy with the syntax proposed, with the exception of the two points I raised above.

N/A

I have been following this proposal from the beginning and have put significant time into understanding, learning, and experimenting with them, especially in the context of server side Swift.

Thanks to everyone who poured countless hours into this!

7 Likes

We could also rename wrapperValue to selfValue.

self is some kind of magic variable, so using it in a property name that look magic may help.

1 Like

Hi there, I've been influencing the design of the currently proposed feature a lot during the pitch and review threads and it's my fault that value property was renamed to wrappedValue. I don't want to revise this change and strongly remain in favor of it. However I pointed out in the previous pitch thread that wrappedValue and wrapperValue should have a little impact on anything, but I meant this for the last design of wrapperValue. Like @anandabits said, there is again a new surprise from @Douglas_Gregor in this iteration. It reminds me of the very first delegateValue introduction during last review where that feature was not discussed during the original pitch.

New $$ approach and access level

In general I like the proposal and the proposed feature, but I don't think I'd say the same about the new $$ approach. I'm strongly against the solutions pitched in this review thread regarding the access control issues as well. The access level as proposed will be private by default, while others in the thread would like to see an alignment of the access level with the wrapped computed property. I think non of these solution is ideal, because it again requires new rules we have to learn about access control behavior. Just remember the radioactive threads about access control on extensions and global members.

Making the stored property having the same access level as the wrapped property would be fatal as it means that one will almost always write explicitly the access level for the stored property.

public private(wrapper) var property: Value

This also would add more accidental exposures of the stored properties if the user forgets to properly configure it.

That said I think the default should remain internal. This is the sweat-spot in Swift today and property wrappers should be no different. As soon as access_level(wrapper) is implemented everyone who want the wrapper to be hidden can do so. This is just a matter of time and should not increase the cognitive load on the user, as the access control is already confusing to some community members.

In this section I started describing my concerns regarding $$ approach but then discussed the future direction of the access level. That was intentional, because with access_level(wrapper) we wanted to change the access of the stored property. However now we have two new properties, one prefixed with $ and the other with $$. We're in an unlucky situation now, because it's not clear anymore what access_level(wrapper) will do and how to control the exposure of both properties. Therefore I would revise this surprising change and use the previous design where the whole stored property becomes unaccessible if a special member is present that shadows its access.

wrappedValue vs. wrapperValue

As I said above already, I'm the one responsible for the renaming of value into wrappedValue. I still think this was a good move because it does indeed avoid conflicts with @dynamicMemberLookup types, especially if those are also property wrapper types. value is too generic and is used widely, so the collisions will definitely happen a lot if we don't call the property differently. (Since SwiftUI has a property wrapper called State that returns another type that uses @dynamicMemberLookup, the collision with value in view models is nearly inevitable.) wrappedValue on the other hand describes well its semantics and also signals to the user that it's likely to be related to @propertyWrapper attribute. Therefore I strongly think we should keep that property name here.

wrapperValue however was just a rename that happened during the transition from property delegates to property wrappers. A previously called delegateValue property was simply aligned to wrapperValue. I mentioned that I had little to no concerns about the name difference which was okay with the previous design without $$. However right now I also think that the difference in the naming is too small and very hard to spot. I read the previous posts and there is only one name that caught my attention. @gwendal.roue pitched associatedValue.

My personal opinion on associatedValue is that it's a little too magical as it does not describe it's purpose on a property wrapper type. You can ask yourself: "The property wrapper has an associated value, but what for?"
However the naming scheme was almost perfect from my point of view. As @Chris_Lattner3 pointed out above, we could require a more "lexically different" name for that feature that will standout but also remain describing well the semantics, just like wrappedValue does.

I propose to revise wrapperValue (with R) and rename it to wrapperAssociatedValue.

Mistakes in the proposal

As noted by @hartbit the current proposal has some oversights. I pushed a PR to fix them.

Review

What is your evaluation of the proposal?

I support the proposal.

Is the problem being addressed significant enough to warrant a change to Swift?

100%

Does this proposal fit well with the feel and direction of Swift?

Even though Swift tends to fit a lot of value into attributes these days I think this particular feature is worth it, but we should be careful in the future with other features to not to overflow the attribute syntax too much.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

n/a

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I participated in every pitch and review thread of the proposal and influenced the design of the feature.

3 Likes

propertyValue?

If that's the answer, then I find it even more confusing, semantically.

Why not just “wrapper” instead of wrapperValue? Then wrapper / wrappedValue would be asymmetric enough.

7 Likes