[Pitch #4] SE-0293: Extend Property Wrappers to Function and Closure Parameters

Hello again, Swift Evolution!

@filip-sakel and I have been working on the next revision of SE-0293 based on feedback from the second review. The latest proposal draft is available here.

Here is a list of changes to the design in this revision:

  • The distinction between property wrappers that are API and property wrappers that are implementation detail is formalized via the api option in the @propertyWrapper attribute, i.e. @propertyWrapper(api)
  • Implementation-detail property wrappers on parameters are sugar for a local wrapped variable.
  • API-level property wrappers on parameters use caller-side application of the property wrapper. The design of API-level property wrappers attached to parameters is the same as the previous revision of SE-0293.
  • Overload resolution for property wrapper initializers will always be done at the property wrapper declaration.

We believe this addresses the feedback about protocol requirements, because implementation detail property wrappers are not part of the function signature and therefore can be used on protocol witnesses without the requirement including the property wrapper. We have also included a future direction for the previously pitched property wrappers in protocol requirements for API-level property wrappers.

Both of us think there's a better spelling for api out there, but we need your help with brainstorming! A few other ideas that Filip came up with were apiLevel and apiSignificant. Please let us know what you think or if you have other ideas - now is the time for bike shedding! Of course, any other feedback is also welcome.

-Holly

14 Likes

Super exciting, thanks for pushing this along @hborla and @filip-sakel !

I worry about whether this distinction pulls its weight; certainly we've never had another part of a function's declaration be "API or not" based on some other declaration.

The property wrapper feature is already an advanced one, to be sure, and doubly so its use in function and closure parameters. We've been focusing on "API-significant" property wrappers on previous iterations of this because they enable new expressiveness that's demonstrated real benefit to users in a way that can justify the addition of such an advanced feature.

Do "implementation-detail" property wrappers which are sugar over locally wrapped variables have similarly weighty justifications? And is it properly a type-level concern for a property wrapper or an instance (or rather, parameter)-level concern? I would tend to think the latter, and in that case I would be inclined to think that this should be up to the writer of the function that's using the property wrapper simply to use a locally wrapped variable, not for the writer of the property wrapper to decide...

7 Likes

I should add that, as I commented last time, I really thought the last revision was pretty much perfect.

With a complex feature such as this, implied is that there is a delicate balance of trade-offs, and the last revision really achieved a balance that (at least for my intellect) pushed to the limits of what I could understand reliably without going over, and my gut feeling is that this additional complication here unfortunately tips it over.

I think the ideal response to the core team’s feedback would be to make the minimal addition necessary to the previous version that addresses this:

The overall model explained in the proposal is that the exposed function type does not have the wrappers on it. One approach would be to be consistent with that where implementations with wrappers on them would fulfill the projected types, an alternate approach would allow fulfilling requirements with the wrapped types.

Personally, I think fulfilling requirements with the wrapped types makes the most sense to me.

5 Likes

I think that part of the reason why property wrappers are so complicated is that the design of the feature is wrestling between two fundamentally different use cases. The current design of the feature solves this issue with access control - the property wrapper storage is always private, but the property wrapper type can also opt into promoting the storage to API via projectedValue. This turns into a more fundamental modeling issue when property wrappers are applied to parameters, because access control doesn't help - either the wrapper is part of the function signature or it isn't.

One could argue that only API-level wrappers belong in the function declaration - I argued this point throughout the last review. My view was that if a property wrapper is implementation detail, it doesn't belong in the function declaration itself. I argued that all parameter attributes I could think of were relevant to the caller. Because of this, I thought it didn't make sense to write a wrapper attribute in a protocol witness without the requirement having the same wrapper attribute. The core team disagrees with the last part, as per the decision notes.

I had a conversation with @John_McCall to help me understand the review feedback. He prompted me to take a step back and think about how a function declaration with a property wrapper attribute on a parameter will be interpreted by the programmer. Because most property wrappers are just sugar to get a common effect, the programmer wouldn't expect a function declaration with such a property wrapper to have any effect external to the function. For example,

func min(@Logged a: Int, @Logged b: Int) -> Int { ... }

Reading that declaration, it doesn't make any sense for the property wrapper to affect the function externally, affect whether it witnesses a protocol requirement with type (Int, Int) -> Int, etc.

This was also a point brought up throughout the original review of SE-0258 when debating the access control rules. The design ended up letting the property wrapper type decide whether it adds API to the wrapped declaration. I do think it makes sense for the wrapper type to decide, because the semantics that are attached to the wrapped declaration are defined by the wrapper (and those semantics determine whether or not the wrapper itself should be API). It's worth noting that under the pitched design, an implementation-detail wrapper can be promoted to API by composing it with an API-level wrapper, e.g.

@propertyWrapper(api)
struct API<Value> {
  var wrappedValue: Value
}

func insert(@API @Logged text: String) { ... }
EDIT: I misread "wrapped" as "wrapper", and the following argument doesn't make sense

I don't think this makes sense because the function will then be called differently in a generic context versus directly on a conforming type. The biggest change in the last revision was that you cannot pass the wrapper storage directly - this is why I think that wrapper attributes should be part of the protocol requirement itself.

5 Likes

+1.

I have similar initial reactions to @xwu to @propertyWrapper(api), and my immediate thoughts to the Core Team's feedback were almost identical:

I'm going to sit with this revision for a bit to see if it grows on me, but wanted to respond to this argument against the "witness-by-wrapped" strategy:

I, personally, am not super troubled by this, since if I were implementing the conformance myself, this would have to be the case anyway.

My example from previous pitch thread, which dealt directly with protocol witnesses
protocol P {
  func foo(a: String)
}

struct S {
  func foo(@Lowercased a: String) { ... }
}

func callFoo<T: P>(_ t: T) { t.foo("") }

let s = S()
callFoo(s)

Under the previous revision, a 'manual' conformance of S to P in order to let the above compile would require me to write something like:

extension S: P {
  @_implements(P.foo(a:))
  func _P_foo(a: String) { self.foo(a: Lowercased(wrappedValue: a)) }
}

I.e., writing out the conformance by hand already causes the call to occur differently in a generic context, and will similarly break wrappers like @Asserted as @filip-sakel called out.

I suppose there's an argument that the 'manual' conformance makes this obvious in a way that extension S: P {} wouldn't, but as @xwu notes this is getting into a niche corner case of an expert-level feature, so I'm not super troubled. Also, the potentially surprising behavior is relatively easily explained, IMO, to any user who is advanced enough to even be concerned about the difference between caller-side callee-side wrapper application.

I think it's reasonable to talk about applying wrappers at the "witness site" when they fulfill protocol requirements, i.e., at the point where the conformance is declared. We could still base init(wrappedValue:) overload resolution on the generic constraints of the extension that declares the conformance, and the 'magic' constants could just point to the extension to indicate "you were called through a protocol context, so your wrappers were applied at the witness site."

1 Like

Sorry, I misread "wrapped" as "wrapper" :slightly_smiling_face: my argument against this doesn't make sense

2 Likes

Yeah, I'm confused by that too. If anything, fulfilling the requirement by the wrapped value would be the most consistent, is pretty much what one would expect, and is exactly what would happen if one were to create a boilerplate in absence of the synthesis, especially in regard to generic context. I think it's worth pursuing on the basis of consistency alone.

2 Likes

I think it still makes sense to be concerned about the fact that you lose caller-side application of the wrapper (which is what I thought you were referring to, despite your misreading :grinning_face_with_smiling_eyes:), though this is obviously a much more subtle concern compared to the "witness-by-wrapper" strategy.

1 Like

Agreed. Honestly, when I commented in the previous pitch thread about punting on protocol conformances for the time being this is pretty much where I expected us to end up eventually, it just didn't seem urgent enough to myself to require inclusion in the proposal. But given that the Core Team wants to see the problem attacked now rather than as a future step, I think this is the right direction. (Unless, of course, there's more serious soundness problems that we haven't thought of... :sweat_smile:)

I originally had a similar thought, but quickly lose it. Given that we already lose "overloads" for protocol witness. It's not too different from losing the caller-side resolution:

protocol X { func foo<T>(_: T) }
protocol Y { func foo(_: Int) }

@propertyWrapper
struct Wrapper {
  init(wrappedValue: Int) { print("Wrapper.Int") }
  init<T>(wrappedValue: T) { print("Wrapper.generic") }
}
struct Foo: X {
  func foo(_: Int) { print("Foo.Int") }
  func foo<T>(_: T) { print("Foo.generic") }
}
struct Bar: X {
  func foo<T>(@Wrapper _: T) { ... }
}

let foo = Foo(), bar = Bar()
(foo as X).foo(1) // Foo.generic
(bar as X).foo(1) // Could/Should be Wrapper.generic
(foo as Y).foo(1) // Foo.Int
(foo as Y).foo(1) // Could/Should be wrapper.Int

Oddly enough, the fact that protocol witness behaves this way ends up saving enough room for this.

2 Likes

Slightly off-topic, but if this sort of approach becomes the general rule for protocol witnesses, it would also be easily extended to allowing witnesses with default arguments:

protocol P {
  func foo()
}

struct S {
  func foo(_ arg: Int = 0) {}
}

extension S: P {} // OK!
1 Like

In general, this approach of generating the witness that will apply the wrapper is effectively achieving the same thing that the implementation-detail wrapper is achieving. By generating a witness that applies the property wrapper inside the witness rather than at the call-site, it's effectively turning that property wrapper into implementation detail. When called through the protocol (e.g. in a generic context or via existential type), the caller knows absolutely nothing about the wrapper. The wrapper is an implementation detail internal to the witness. This is exactly the purpose of the implementation detail property wrapper.

This is also not really what you want for property wrappers that are supposed to be API, and it even prevents you from using property wrappers without init(wrappedValue:) with protocols at all. So, there's still the fundamental issue that some property wrappers cannot compose with protocol requirements, and this approach is kinda sorta trying to infer that a property wrapper can be implementation detail from the presence of init(wrappedValue:).

However, it's not always the case that wrappers with init(wrappedValue:) should be implementation detail. For example, one of the potential concurrency use cases for property wrappers on parameters is @UnsafeTransfer. The entire purpose of this wrapper is to change the function to accept something that conforms to ConcurrentValue, which is the wrapper itself and not necessarily the wrapped value. Now, this wrapper as written does nothing to the wrapped value, but you could imagine a similar wrapper that performs a deep copy that's used for the same purpose. It would not be sound for such a wrapper to witness a protocol requirement using the wrapped value type. I'm not sure whether async actor methods can be called via protocol witness table (I haven't been doing the best job of keeping up with all of the concurrency proposals), but if that's desired, then we really want a way to enforce that the wrapper does not become implementation detail.

At a higher level, some of these wrappers are communicating some kind of assumption to the caller. With @Asserted, the assumption is that the value meets some condition. With @UnsafeTransfer, the caller promises not to use the transferred object. Rather than lose these assumptions in certain contexts, I think the better approach is to formalize the distinction.

3 Likes

Wait, could you elaborate on this? The deep copy would still be performed when calling through the witness, we just wouldn't be evaluating the DeepCopy.init(wrappedValue:) initializer from within the generic/existential context, we'd be evaluating it from the context where the conformance was declared.

If anything, @UnsafeTransfer is the more worrying wrapper for the "witness-by-wrapped" scheme since it doesn't perform any modification of the argument.

I think one thing that bothers me about the @propertyWrapper(api) naming is that, as far as I can tell, we've added this parameterization to the property wrapper attribute for concerns that only come into play when wrapping arguments. If we're seeing conceptual divergence between these two use cases, then perhaps it would be worth diverging the naming as well? Spitballing here, but I'm imagining something like:

  • @propertyWrapper receives no parameterization and retains its original meaning. Types declared as @propertyWrapper cannot be used to wrap arguments.
  • Introduce @argumentWrapper. Wrapper types can be both @propertyWrapper and @argumentWrapper, or just one or the other.
  • @argumentWrapper types are always API.
    • If a user wants the "local wrapper" functionality, they are free to use an @propertyWrapper on a local variable, just as they can today (I really don't think we should worry too much about optimizing for this use case).
  • If a property is wrapped by a type that is also @argumentWrapper, then the synthesized init gets an @Wrapper attribute for that type.
3 Likes

Right - my assumption for "unsoundness" is that there could be time between the function invocation and the function execution where the value being passed may have changed (e.g. if it's actor-isolated state or global state). So, the contents of the object could actually be different if you're copying in the function body rather than the call-site. I am not a concurrency expert, but I think this is possible based on my understanding of the actor proposal.

I don't think this is necessarily true (though it is for this proposal alone, of course). For example, there's a future direction about using this distinction to decide whether a property wrapper attribute can be part of a protocol requirement. This can apply to properties as well as parameters.

2 Likes

If @UnsafeTransfer were an implementation detail, the caller wouldn't (statically and intraprocedurally) understand that it was okay to pass a non-concurrent value there when calling the function asynchronously, because it wouldn't see the wrapper. So it's not that it would be thread-unsafe to do it as an implementation detail, it's that it wouldn't be solving the language problem anymore.

With that said, I'm not convinced that @UnsafeTransfer should be a parameter wrapper, because unsafe transfers really shouldn't be invisible at the call site. It's a poor use of the language feature.

5 Likes

Right, there are two key things here:

  • The true type of the parameter must conform to ConcurrentValue, or else Swift won't let you call the function asynchronously because that would entail sharing a non-concurrent value between actors / concurrency domains. So presumably DeepCopy conforms to ConcurrentValue, but if you erase DeepCopy as an implementation detail, you end up with a type that may not be concurrent anymore, and asynchronous calls will be rejected.
  • The copy needs to be done by the caller, at least when doing an asynchronous call, because if it's done by the callee, it'll be done after moving to the target actor / concurrency domain, which is to say that the original value will have been shared between actors / concurrency domains. There's no "unless we're doing a deep copy" exception which makes that safe, so the copy needs to happen on the original actor and only then be captured for transfer.

Now, again, I'm not really sure that doing implicit deep copies of objects is actually a great idea from a language design perspective, at least for types other than the conventional almost-a-value reference types like NSString and so on. Passing an object and having the caller receive a different object can be a huge semantic difference. But if we want to do it, these are the constraints.

5 Likes

Thank you both, that makes total sense! I wasn't considering the fact that we'd have to cross actor boundaries in order to call the witness, before we call into the actual implementation (with the wrapped parameters).

I think this speaks to some of my discomfort with the protocol conformance situation under this revision. If some method S.foo(_:) has wrappers and we want to make it a witness for some requirement P.bar(_:) with a matching signature (wrappers notwithstanding), we can write a wrapper relatively easily:

extension S: P {
  func bar(_ arg: T) {
    foo(arg)
  }
}

In particular, even if S.foo has @UnsafeTransfer or other "API" wrappers, we can discard them at the conformance site without even realizing what we're doing. I don't think we're protected much here from accidentally discarding API simply by having to write out the call to foo explicitly.

But if the protocol requirement happens to share the same name as our proposed witness... we're out of luck. We can't write the conformance if the protocol requirement were P.foo(_:), since the overloading rules would prohibit the redeclaration of foo based solely on the wrappers.

So, what does prohibiting same-name witnesses really protect against? If the author of S.foo(_:) is the same as the author of the conformance S: P, aren't they appropriately positioned to decide whether discarding the API is appropriate? And if the conformer of S: P is different from the author of S.foo(_:), why do we only protect them from discarding API wrappers in the same-name case and not the different-name case?

Furthermore, what's the official escape hatch for someone who really does want to discard API wrappers in order to write the conformance S: P. I don't think @_implements quite fits the bill since it's an underscored feature, so I guess the only option would be to write a wrapper type around S which forwards to S.foo(_:)?

I suppose on the whole I'm just more comfortable saying "don't conform to protocols in invalid ways," and that would include making sure that the wrappers of your witness have appropriate semantics for the protocol requirement.

1 Like

I'm a little confused how this revision introduces this potential footgun - seems to me like the previous revision has the same issue. The "new thing" in this revision is the implementation-detail property wrapper. The API property wrapper existed in the last revision - we just gave it a name in this one.

Regardless, I don't think somebody will immediately jump to writing their own wrapper function when implementing a protocol conformance. Presumably, the programmer tried to conform to the protocol using the function with API wrappers first, and got a good error message for why you can't do that.

I think this concern is partially stemming from me presenting a poor example. If you do this with @Asserted, you'll still get an assertion failure if you pass an invalid value. My main point against the "synthesized witness" approach was that it is achieving the same thing as what's proposed here, but it does not provide a solution for property wrappers that don't support init(wrappedValue:) (not to mention the implementation complexity!)

2 Likes

As to the bike shedding part :smiley:

The name "api" seems a bit too general to me. I guess the core distinction is caller versus callee. Or public versus private. Or being a pure wrapper versus e.g. having a projected value.

To me property wrapper option names like

  • @propertyWrapper(open)
  • @propertyWrapper(public)
  • @propertyWrapper(exposed)
  • @propertyWrapper(projected)

would better imply that this property wrapper is not just privately/secretly wrapping a value.

I guess as soon as you have a projectedValue in a property wrapper, it's no longer a purely wrapping property wrapper and is probably meant to be exposed ("api"). So perhaps another option could be to make all property wrappers with a projectedValue "api". That would probably be too obscure, though.

3 Likes