SE-0293: Extend Property Wrappers to Function and Closure Parameters

Evolution Proposal Reviews
2
  • What is your evaluation of the proposal?

-1, only because of the behavior discussed under the Unapplied references to functions with property-wrapper parameters heading. With minor adjustments (discussed below), I would be +1.

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

This isn't a problem that I personally have felt constrained by when writing Swift code, but I haven't made extensive use of property wrappers as a library author. The Motivation section is sufficiently convincing to me that addressing this problem is worth it.

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

Yes. This seems like a natural extension of existing property wrapper syntax.

  • 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?

Participated extensively in the pitch thread and did a quick read of this latest version.

Unapplied references

My only objection to this proposal is the fact that unapplied references to functions with wrapped parameters adopt the type of the wrapper rather than the type of the wrapped parameter:

func postUrl(@Lowercased urlString: String) { ... }
let fn: (Lowercased) -> Void = postUrl
fn(Lowercased(wrappedValue: "mySite.org/termsOfService"))

I've gone into more detail in the pitch thread, but I'll summarize again here.

IMO, this aspect of the proposal introduces enough potentially-unexpected behavior that we would be better served deferring it until we see a bit more use of this feature in the wild. Allowing API clients to call functions with the type of the parameter's backing storage introduces a non-obvious API surface that library authors may not be aware of.

The most common usage of this feature will be to write functions which enable a convenient syntax for callers to automatically have their argument wrapped in a new wrapper instance:

When passing an argument to a function with a property-wrapper parameter using the original argument label with no prefix, the compiler will wrap the argument in a call to init(wrappedValue:) .

It seems to me like a perfectly reasonable (though incorrect) assumption for an API author to make that the wrapped parameters of a function will always be initialized with fresh wrapper instances, called as init(wrappedValue:) (and no other arguments). In the pitch thread, I offered a couple examples where that assumption can result in broken code:

  • Wrappers with reference semantics:
@propertyWrapper
class Box<T> { ... }

func foo(@Box arg1: Int, @Box arg2: Int) { ... }

let box = Box(wrappedValue: 0)
let bar = foo
bar(box, box) // potential exclusivity violation!
  • Wrappers which support initialization via more than just init(wrappedValue:):
@propertyWrapper
struct Wrapper<T> {
  var wrappedValue: T

  init(wrappedValue: T, secretSauce: String = "") { ... }
}

// Assumption: _arg always initialized with secretSauce of ""
func foo(@Wrapper arg: Int) { ... }

let bar = foo(arg:)
bar(Wrapper(wrappedValue: 3, secretSauce: "hunter2") // oops!

The proposal indicates that this is undesirable behavior in its justification for banning arguments in the wrapper attribute:

Property-wrapper parameters cannot have additional arguments in the wrapper attribute.

Rationale : Arguments on the wrapper attribute are expected to never be changed by the caller. However, it is not possible to enforce this today; thus, property-wrapper parameters cannot support additional arguments in the attribute until there is a mechanism for per-declaration shared state for property wrappers.

But allowing unapplied references to take on the parameter wrapper types allows such behavior to sneak back in.

These issues already exist to some extent with property wrappers, since backing storage is sometimes exposed via memberwise initializers, but IMO such issues are much more problematic when they arise for function parameter wrappers:

Even if we assume that library authors will use this feature perfectly, and not be caught off-guard by the potential gotchas here, though, it's not obvious to me that the proposed behavior is ultimately desirable. Unless I missed something, the proposal as-written offers no compelling justification for the behavior of unapplied references to functions with wrapped parameters—I don't see it mentioned in Motivation, and the section that addresses it specifically is just a plain statement of the behavior.

In some situations, it results in potentially surprising behavior:

func bar(@Wrapper _ arg: Int) -> String { ... }
bar(0)

func transform(_ arg: Int, by transform: (Int) -> String) -> String { transform(arg) }

transform(0, by: bar) // error: cannot convert '(Wrapper<Int>) -> String' to '(Int) -> String'

// I have to do this; why isn't it the same?
transform(0, by: { bar($0) })

Proposed alternatives

In my view, there's three straightforward ways to handle unapplied references to functions with wrapped parameters:

  1. Adopt the type of the wrapper (currently proposed).
  2. Adopt the type of the wrapped parameter.
  3. Disallow such unapplied references.

Each of these has desirable qualities, and none of them seems obviously correct to me. (1) gives us some consistency between closures with wrapped parameters and functions with wrapped parameters. (2) gives us consistency between the call syntax and the unapplied reference syntax of functions with wrapped parameters (as in the example directly above). (2) and (3), AFAICT, would allow the addition/removal of a function argument wrapper to be an API- (though not ABI-) compatible change.

I'd like to see us adopt a more conservative approach here. If (1) or (2) turn out to be ill-advised design decisions, source-compatibility will leave us stuck with a sharp corner of the language for the forseeable future. I believe that initially, we should disallow the formation of unapplied references, forcing users to use the { bar($0) } workaround to create such references 'manually'. With more information about this feature in the wild, we can make a more informed decision about how to treat unapplied references.

One downside of this 'wait-and-see' approach is that it may encourage API authors to insert wrappers in ways that would be problematic if the backing storage is ever exposed, making it more difficult to justify enabling behavior (1) in the future if it does turn out to be desirable. IMO, we can address this with suitable language in the diagnostic/proposal (to the effect of "this feature is not available yet but may expose the backing storage at some point"). The 'careful' API authors would be discouraged from using wrappers in future-inappropriate ways, and extensive 'non-careful' usage would properly indicate that a change to behavior (1) would likely be undesirable.

Overall, I commend the authors on a well-written proposal, and I greatly appreciated @hborla's detailed engagement on the pitch thread.

9 Likes