SE-0258: Property Wrappers (second review)

rumnat · 2019-06-19T04:01:47.862Z

is "$" the final decision already? What if use "#" as a suggestion? Like we did in "#selector". I think it feels better.

Douglas_Gregor · 2019-06-19T04:40:29.112Z

mattpolzin:

My only other concern with property wrappers is that they do not compose well with a Decodable conformance because property wrappers and Decodable conformance both rely on initializers. I don't think this concern is the burden of property wrappers alone and it certainly does not make me want the current feature as-designed to be rejected, but it is worth noting in case there is some future direction that has not been considered that could allow having a property wrapper both accept initial arguments and also define custom decoding rules. I think such a direction would require changes to how property wrappers are synthesized but hopefully not a drastic change to how they are written or any change to how they are used.

I'd like to understand your concern better. The current behavior of property wrappers with Decodable is the behavior that is implied by the desugaring, such that the property wrapper type itself controls how decoding is handled:

extension Lazy: Decodable where Value: Decodable {
  init(from decoder: Decoder) { ... }
}

The property wrapper itself can encode whatever is needed to reconstitute itself when decoded, including both the wrapped value and any extra state that's required. What are we not able to express properly with this?

Doug

Douglas_Gregor · 2019-06-19T04:44:57.203Z

DevAndArtist:

That would be fine by me if that was the final design, but if I had to choose I'd prefer wrapperAssociatedValue to give that feature more semantical description right from the property name. It's more verbose, that's true, but it also stands out more clearly to the reader.

"Associated value" generally refers to an enum case that carries a value with it. I think "wrapper" has to be in the name, because we're replacing the wrapper. How about something like replacementWrapper?

Doug

Lantua · 2019-06-19T04:47:19.466Z

How about wrapperReplacement?

Still, I feel that, if wrapper is to remain accessible (albeit privately) even in the presence of wrapperValue. They might as well be treated independently and separately.

Douglas_Gregor · 2019-06-19T04:55:03.505Z

anandabits:

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 .

For reference, the $$ has been in the implementation for weeks, and people have needed access to the real backing storage on occasion. Rather than change that, I documented the behavior; the proposal could be more clearer on this, however.

There's a philosophy here, which is that I feel fairly strongly that a type should be able to refer to all of its stored properties. You need to be able to access your stored properties for initialization, comparison, encoding/decoding, etc. For example, let's say A uses wrapperValue, and we try to go from:

struct X {
  @A(id: 0) var foo: Int
}

to a form that provides the id value for foo via an initializer:

struct X {
  @A var foo: Int

  init(fooId: Int) { 
    $$foo = A(id: fooId)
  }
}

I suspect usage of $$ will be rare, but it being available as a private member of the type---so the type can initialize itself and such---is important for basic usage and refactoring of property wrappers.

Doug

Douglas_Gregor · 2019-06-19T04:56:38.392Z

rxwei:

I was not planning to support this.

Is there any parsing ambiguity? I feel it would be nice to support this syntax for consistency with the initializer syntax, and there are good use cases that would otherwise look less concise with explicit parentheses.

I don't see an ambiguity, but I'd love to see one of these use cases

EDIT: Oh, of course I see your derivative use case above.

Doug

Douglas_Gregor · 2019-06-19T05:00:02.181Z

Adrian_Prantl:

I'm concerned by the use of $ in the syntax, since this invades the namespace reserved for the debugger (as in -Xfrontend -enable-debugger-support ). Ideally I would prefer using a different syntax than $ for property wrappers. If the use of $ for this feature is very compelling, the proposal should include new wording for which namespace is going to be reserved for the debugger henceforth.

As noted in the proposal, only the compiler is allowed to introduce $-prefixed names, so the number of likely collisions is really tiny. Moreover, the debugger's $ names should take take precedence, because most $ names introduced by the compiler are already going to be in scopes where qualification can be used to disambiguate (e.g., self.$foo).

I don't think we need any additional reservation for the debugger.

Doug

Chris_Lattner3 · 2019-06-19T05:10:30.401Z

Douglas_Gregor:

There's a philosophy here, which is that I feel fairly strongly that a type should be able to refer to all of its stored properties. You need to be able to access your stored properties for initialization, comparison, encoding/decoding, etc. For example, let's say A uses wrapperValue , and we try to go from:

Ok, but I don't understand. Why do you need two levels of wrapper here? Why isn't foo$ enough to refer to the storage?

-Chris

GreatApe · 2019-06-19T07:31:17.900Z

I think the idea is that sometimes you may want to expose something other than the actual storage through $foo, like a pointer, and it seems that then the actual storage is shifted to $$foo.

I was shocked and confused by this concept but I'm starting to come around to it... But I still think you could just expose the pointer or whatever on your storage type instead, since that won't be hidden anyway.

Jean-Daniel · 2019-06-19T08:08:09.088Z

Chris_Lattner3:

Ok, but I don't understand. Why do you need two levels of wrapper here? Why isn't foo$ enough to refer to the storage?

The second level is only required when the type declare a wrapperValue property. In that case, the single $ become access to the wrapperValue, and you don't have access to the storage anymore. So double $$ was introduced to give access to the storage in such case.

anandabits · 2019-06-19T13:16:11.340Z

Douglas_Gregor:

For reference, the $ has been in the implementation for weeks, and people have needed access to the real backing storage on occasion.

Given how controversial wrapperValue has been I’m disappointed that something like this existed in the implementation but was not mentioned here. How were people supposed to discover it? I would have given the same feedback weeks ago had I been aware of this behavior.

Douglas_Gregor:

There's a philosophy here, which is that I feel fairly strongly that a type should be able to refer to all of its stored properties.

If you want to get into philosophy we could debate here what “its” stored properties actually are. Consider the following:

struct Foo {
    private var x = 42
}

struct Bar {
    var foo: Foo
}

Here, foo.x is not directly a property of Bar, but it is part of the storage area of Bar. Nevertheless, the implementation of Bar does not have access to it because access control allows the author of Bar to hide its properties if desired. I don’t see a wrapper type hiding its storage using wrapperValue as being materially different from this. It is only the mechanism used to encapsulate that is different. There could be endless philosophical debate about this topic and that probably wouldn’t be productive, but I think this perspective should be represented and considered.

Douglas_Gregor:

You need to be able to access your stored properties for initialization, comparison, encoding/decoding, etc.

If we can’t find ways to support out-of-line initialization and non-synthesized encoding / decoding without exposing $$ and these are considered crucial use cases then perhaps that is a sign that wrapperValue is a bit too much magic.

I stand by my prior review. If you feel strongly that a wrapper should not be able to hide the actual storage then I am opposed to wrapperValue. The final design provides no encapsulation tools for library authors. It only provides very magical syntactic sugar with a lot more potential for confusion than usual.

I’ll restate that $foo.binidng is not that bad and is much more clear about what is going on. I know very experienced and talented programmers who spent a bunch of time scratching their heads trying to figure out how State turns into Binding. If you don’t want to go with the original version of this feature which has a lot less magic and offers encapsulation to wrapper authors maybe it should just be dropped.

anandabits · 2019-06-19T13:18:35.328Z

Chris_Lattner3:

Ok, but I don't understand. Why do you need two levels of wrapper here? Why isn't foo$ enough to refer to the storage?

Reading between the lines, it appears to be because Apple wants people to pass Binding references to @State properties in SwiftUI with the $foo syntax instead of having to write $foo.binding.

calebkleveter · 2019-06-19T13:43:42.209Z

Could we make it possible to get the backing storage for a key-path?

struct Foo {
    @Lazy var bar: Int = 42
}

var foo = Foo()
var bar: Lazy<Int> = foo[storage: \.bar]

Lantua · 2019-06-19T13:46:34.369Z

anandabits:

instead of having to write $foo.binding .

I find $foo.binding rather troubling.
Binding does provide a way to access data via dynamic member lookup, so if I’m to add binding to foo.bar, I would instead write $foo.binding.bar which still isn’t exactly obvious. Granted it may not be common, but it’s not something we should just ignore. Though I couldn’t come up with a satisfiable solution that changes only property wrapper itself, or the Binding.

I still think that wrapperValue/wrapperReplacement is a useful concept, but it should distance itself from the base storage.

anandabits · 2019-06-19T14:03:54.187Z

Lantua:

I still think that wrapperValue / wrapperReplacement is a useful concept, but it should distance itself from the base storage .

I agree. I support the earlier version. I don't like the last minute change. If forced to choose between the version under review and removing wrapperValue from the proposal I would do the latter. I don't think the design that is under review is the right one. If the earlier version isn't going to fly then we need more design iteration on this concept.

Apple kind of boxed themselves into a corner by announcing a major framework that relies on language features that haven't been accepted yet. It's disappointing to see so much pressure to accept put on a feature that remains somewhat controversial. That isn't a good thing for Swift. Even if we need more time to iterate on wrapperValue it seems unlikely that we will get it.

Lantua · 2019-06-19T14:30:18.543Z

anandabits:

If forced to choose between the version under review and removing wrapperValue from the proposal I would do the latter.

That’s actually not a bad idea. The proposal without wrapperValue is still very much useful. Then we could pitch & add wrapperValue back semi-additively.

mattpolzin · 2019-06-19T14:37:51.096Z

I agree that the desugaring implies the current behavior. I think there is a missed opportunity if you step away from the given name "property wrapper" and look at it how it reads in code, as simply a modification of a type's behavior. In general, my concern applies to property wrappers looking to conform to any protocol that declares initializers, but I will use Decodable as an example of such a protocol and I'll use Clamping as an example of a property wrapper.

If I was the author of Clamping, I would want the meaning of the following line to be "x is an integer between 0 and 255."

@Clamping(min: 0, max: 255) var x: Int

This meaning holds at face value, but now imagine that x is a property within a type that I want to decode from JSON sometimes but create from user input other times. Even worse, I am using an existing API to retrieve the data I am decoding so I do not have control over the JSON and therefore the minimum and maximum values of the Clamping construct cannot be decoded (they are not offered up by the API). I would want the "guarantee" of a clamped value to apply to all initialization paths, but it does not.

I have not come up with any brilliant ideas for future directions, so here is where things fall apart a bit. Imagine for a moment that there was a way to allow property wrappers to take control of all initialization paths rather than simply offering overloads that the compiler chooses under some situations.

Maybe one way to express this idea is that a type with the @propertyWrapper annotation is allowed to conform to protocols that require specific init definitions in a way that adds arguments to those initializers. Here's a strawman for an initializer that provides conformance of Clamping to Decodable:

@propertyWrapper
struct Clamping<V: Comparable>: Decodable {
  var value: V
  let min: V
  let max: V

  init(initialValue: V, min: V, max: V) { ... }

  // the following would satisfy `Decodable`
  init(from decoder: Decoder, min: V, max: V) { ... }
  ...
}

More generally, a tuple of arguments might look alright, too.

init(conformingInitArgs: (decoder: Decoder), min: V, max: V)
init(conformingInitArgs: (/* a tuple of labeled arguments conforming to a different protocol */), min: V, max: V)

Douglas_Gregor · 2019-06-19T17:01:15.801Z

anandabits:

struct Foo {
    private var x = 42
}

struct Bar {
    var foo: Foo
}
Here, foo.x is not directly a property of Bar , but it is part of the storage area of Bar . Nevertheless, the implementation of Bar does not have access to it because access control allows the author of Bar to hide its properties if desired. I don’t see a wrapper type hiding its storage using wrapperValue as being materially different from this.

I don't see this as related at all, because it's not about whether Bar can reason about the storage of foo---it can't, because Foo is a distinct type. It's about whether Bar is able to create a Foo using the available API to initialize its own foo property.

anandabits:

Given how controversial wrapperValue has been I’m disappointed that something like this existed in the implementation but was not mentioned here.

Let's be clear: the stored property has to exist (it's fundamentally part of the compilation model), and it will be discoverable in metadata. When we get something like StoredPropertyIterable, you're going to be able to get key paths to these backing stored properties fairly easily, with names. Whatever name we choose for this backing storage, it's going to be exposed to the user at some point. My initial thought that it was an implementation detail---hence not documented explicitly---was factually incorrect, and (as I've noted before) wrong for real-world uses, led me to document this.

anandabits:

I stand by my prior review. If you feel strongly that a wrapper should not be able to hide the actual storage then I am opposed to wrapperValue . The final design provides no encapsulation tools for library authors. It only provides very magical syntactic sugar with a lot more potential for confusion than usual.

I emphasized the above sentence of yours, because I disagree with it. In the proposal, the backing storage property $$foo is always private---there is no mechanism to escalate its access, and I would oppose any change that did so. Having the stored property be private is encapsulation. It "hides" the stored property from outside of the type (and outside of the file) in any meaningful sense of the word. Obfuscating the name of the stored property (or pretending it has no name) doesn't give any more encapsulation tools to the library author, but it does take away their ability to reason about the stored properties of their own type.

In this proposal, the actual backing stored property is always private, which is Swift's strongest access-control mechanism to to achieve encapsulation. Property wrapper type authors can introduce wrapperValue to explicitly opt-in to having the $ property exposed further to provide wrapper-specific API via a custom type, providing per-wrapper control to cover the gamut of use cases.

I can understand the position that wrapperValue is too magic/unnecessary, even if I don't agree with it. However, I don't feel like the position that "having a name for the private stored property breaks encapsulation" is tenable.

Doug

blangmuir · 2019-06-19T17:19:02.641Z

Chris_Lattner3:

I don't understand why $property is better than property$ . The later is better for code completion ...

I have no strong opinions on $prefix vs. postfix$, but could you say more about the advantages you see for code-completion? If you're doing fuzzy matching they're nearly equivalent once you are typing the property name. If you aren't typing out the name, currently the '$' impacts the relative sort order for symbols in the same scope, but we could change that in other ways if we wanted.

John_McCall · 2019-06-19T19:38:14.123Z

The proposal authors have informed me that they'd like to further revise this proposal in response to feedback. You are all welcome to continue discussing the proposal here in the meantime, but the authors may be less responsive while they're making those changes. The community will have a fair chance to respond to these revisions; we'll make a decision when they're ready whether to simply extend the review or treat it as a third review.