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

Hello Swift community,

The review of "SE-0293: Extend Property Wrappers to Function and Closure Parameters" begins now and runs through Dec 13, 2020. The proposal is available here.

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 the review manager. When emailing the review manager directly, please keep the proposal link at the top of the message.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

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

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

Chris Lattner
Review Manager

14 Likes
  • 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

I like this proposal, except for this:

    ForEach($shoppingItems) { (@Binding item) in
      TextField(item.name, $item.name)
    }

I’d really like to see the @Binding become unnecessary there. Instead, the ForEach initializer should have a signature something like this:

extension ForEach where Data.Element: Identifiable {
  init(
    @Binding _ data: Data,
    content: (@Binding Data.Element) -> Row
  ) { ... }
}

And we should infer the application of @Binding to the closure’s item parameter.

However, this proposal seems like a step in the right direction, so for that reason, I support it. I just hope we won’t stop here.

7 Likes

I agree that @Binding is unnecessary in the closure because Binding is a contextual type, so it can be inferred. However, I think there's a better solution than making property wrapper attributes some kind of type attribute. A few reasons why:

  • What type does an anonymous closure parameter with a contextual property-wrapped type get? If it's the wrapped value type, do you access the backing wrapper and projected value with _$0 and $$0 (not the most readable syntax IMO)? If the anonymous closure parameter gets the backing wrapper type, then the type would change when you name the parameter, which would be really unexpected IMO.
  • Using property wrapper attributes as type attributes would be really limiting when working with generics. Currently, the ForEach initializer has nothing to do with Binding. It would be really unfortunate if API authors had to add a new overload for each kind of property wrapper that API might want to accept. We could alleviate this by adding some kind of language feature or property wrapper protocol that lets you use a generic parameter as a property wrapper attribute.

I think my favorite solution to the inference problem so far is what @Lantua suggested in the pitch thread, which is in Future Directions:

    ForEach($shoppingItems) { $item in
      TextField(item.name, $item.name)
    }

The downside to this approach though is that not every property wrapper has a projected value, so it's sort of conflating the backing wrapper with the projected value.

2 Likes

I assume it would be this and we would simply recommend using actual names if you want to access the projected value or wrapper.

Well, SwiftUI will have to add something, but I suppose you’re imagining that SwiftUI would add an extension Binding: RandomAccessCollection where Value: RandomAccessCollection instead of adding specific initializer overloads for @Bindings? I suppose that would be cleaner in some ways.

As usual, one of the downsides of not using a protocol for property wrappers is that they can’t easily be used generically. Sigh...

Yeah, I think that would be okay. (The second half of the description of that idea confused me into thinking they were necessarily a package deal.)

Personally, I’m not super-worried about this. The underscored wrapper property’s access control is so tight that, in practice, property wrappers that have non-trivial API usually re-expose themselves through the projectedValue property anyway. In other words, if a property wrapper doesn’t have a projectedValue, it’s probably not interesting enough to get passed through to a closure anyway.

1 Like

How does this compose with function builders? Can a parameter have a property wrapper around an @ViewBuilder closure, for example?

2 Likes

Thank you for your comments, and I see how unapplied references can be a major concern for this proposal. However, I think that the changes proposed do match the current behavior API author expect. For one, consider a library that manages tasks. The idea is simple: we have a task dictionary and we just add our tasks at the user's command, and then somehow initiate this task (we don't care about the last part):

// However, we have an implicit parent task at ID 0,
// so we’ll implicitly raise the provided tasks' IDs by 1.
addTask(withId id: Int) {
  let adjustedId = id + 1 ⚠️

  tasks[adjustedId] = … 
}


addTask(withId: Int.max) ❌
// Shouldn’t the library handle this case explicitly?

Furthermore, while we explore integer overflows, let's examine some examples with floating points. Imagine a drawing library that has some built-in drawer types, such as rectangles, circles, triangles, etc:

struct Rectangle : Drawer { 

  let width, height: Double
  
  init(width: Double, height: Double) { ... }


  func draw(to canvas: Canvas) {
    canvas.addRectangle(width: width, height : height) ⚠️
    // Are we sure these are safe.
  }
 
}


let canvas: Canvas = ...

let rectangle = Rectangle(width: .infinity, height: .infinity) 

canvas.draw(rectangle) ❌
// Shouldn't the library catch this and handle this explicitly,
// instead of continuing with undefined behavior?

Let's also examine class references, as you also mentioned them in your post. Consider this furniture-related library that allows us to create furniture instances and to propose their estimated price:

class Material {

  … 

  func makeMoreShiny() { … }

} 

struct Furniture {

  let name: String

  let material: Material ⚠️
  // This is a reference, and it is not 'protected'
  // to ensure that when we create a furniture,
  // its material remains constant.

} 


let wood: Material = … 
// This is a reference.

let plainDesk = Furniture(name: “desk”, material: wood)


var premiumWood = wood
// This is also a reference.

premiumWood.makeMoreShine() ❌
// Now our plain desk has premium wood.
// This obviously not desirable, as creating furniture 
// and then seeing it having changed without
// understanding why, is bad API design.

Of course, saying "oh look, the language already has things library authors should be worried about, let's add some more" is a bad approach to evolving a language. However, library authors also need custom features (atomics, unsafe memory management, etc.). Therefore, I think that unapplied function references should remain in the proposal, because not doing so would seem as too much of a limitation. An example of why a library author may benefit from unapplied references is when wanting to provide some behavior through a closure, while also providing an easier-to-use function API:

@propertyWrapper
struct Lowecased {

  var rawValue: String


  init(wrappedValue: String) { ... }

  var wrappedValue: String { ... }

}


protocol Task { ... }

struct EffiecientTask : Task {

  func changeName(@Lowercased to newName: String) { ... }

}

func addTask<SomeTask : Task>(_ task: SomeTask) {
  nameManager.add(task.changeName)
  // Here, 'nameManager' may need to have access to the 
  // raw value ofthe provided name, which _is_ provided
  // by the backing wrapper type.
}

In the above example, should the unapplied reference of changeName(to:) take a String or be banned, library authors would not benefit. In the contrary, they would need to separate the function into private and publicly-exposed ones or resort to other workarounds, just to avoid checking that their assertions are valid.

I hope I've addressed your concerns!

Filip

1 Like

To be clear, I'm not saying that I see no value in the behavior as-proposed. I recognize that among the three alternatives I've laid out, there are benefits and downsides to each. It's possible, perhaps even likely, that propagating the backing storage is the most 'correct' option that leaves the language in the best position long-term.

However, I don't believe that, at this juncture, the case for any one solution over the alternatives is strong enough to justify locking us into a particular design that could prove to be problematic down the road. As with any new feature, certain predictions about future usage are purely speculative. My examples were just meant to illustrate that I believe there is a plausible case that the proposed unapplied reference behavior will result in user confusion.

The behavior of unapplied references seems to me to be easily separable from the core problem this proposal is meant to address, and there are viable alternatives to the proposed behavior that have drastically different tradeoffs. Without compelling reasons why the alternatives are untenable, I strongly believe that this aspect of the proposal would be better served by further discussion and a more targeted pitch/review.

Any users who have become familiar with using property wrappers will likely have internalized the "backing storage is private" rule. This proposal would let backing storage creep into public API, and IMO we should be very cautious about features which allow library authors to introduce public API surface in non-obvious ways!

1 Like

Good question. I don't see an obvious reason not to allow it, unless the property wrapper's init(wrappedValue:) has a result builder on wrappedValue (since there can only be one result builder transformation applied to a closure).

I wonder if we should restrict the ordering of the attributes, though. Today there doesn't seem to be any such restriction for stored properties, but this bit of code is hard for me to think about:

@resultBuilder
struct Builder {
  static func buildBlock<T>(_ args: T...) -> T {
    return args.first!
  }
}

@propertyWrapper
struct Wrapper<T> {
  var wrappedValue: T
}

struct S {
  @Wrapper @Builder @Wrapper var fn: () -> Int
}

var s = S {
  1
}

Conceptually, what's happening is the result builder transformation happens on the closure first, and then the property wrapper is initialized. I wonder if it makes sense to force the programmer to write all property wrapper attributes first and the result builder attribute last, so it's more clear that the result builder is being applied to the wrapped value. It seems weird to be able to write a result builder attribute in the middle of the wrapper composition chain.

1 Like

IMO, the exception you've listed here is a decent reason to at least approach this with ~caution~ (which is apparently my word of the day :slightly_smiling_face:). What if there are multiple init(wrappedValue:)s, some with builder attributes and some without? What if we can't determine which will be called from the declaration of the func with the parameter wrapper? What if we can determine which will be called and none of the builder-wrappedValue overloads are in the set? Will it be too complex to expect the user to reason about all of these questions at the call site?

Urgh. What you've written here definitely seems like the right thing to do in a vacuum, but the fact that we don't already enforce this for stored properties is disappointing. If I had to answer I think I'd be in favor of breaking consistency with property wrappers in order to force a more coherent ordering, but I don't have much to support that position other than a gut feeling...

1 Like

It's better if the arguments in unapplied reference are passed via the initializer since that's the how the user would call it:

@propertyWrapper
struct Wrapper<Value> {
  init(wrappedValue: Value) { ... }
  init(wrappedValue: Value)
    where Value : Collection { ... }
}

func generic<T>(@Wrapper arg: T) { ... }

let a = generic // Ambiguous
let b: (Int) -> Void = generic // First `init`
let c: ([Int]) -> Void = generic // Second `init`

Wrappers are split into two categories

  • Those that are passed around using the wrapper storage (Binding)*, and
  • Those that are passed around via initializer (Clamped).

Those two don't really mix. You can't really use/expect the wrapper to be passed around incorrectly.

The current proposal then has a serious problem since that takes control away from the API author to restrict the user to pass data via the initializer. The author can no longer expect Clamped to be enforced, for one.

* Granted, Binding has a self initializer but it's easy to see a type that is similar to that.

1 Like

I don't see how these examples connect. Sure, the API author can miss some non-sensical cases, but I don't think user storing a function reference is as fringe as user using out-of-bound values. Furthermore, the author needs to handle two entirely distinct types of argument passing, which is not great in giving control to API author.

If the API author really wants to make sure that they're using the wrapper with the right parameter, they'd need to do:

func a(@Clamped(2...3) value: Float) {
  assert(value ~= 2...3)
  ...
}

defeating the purpose of the wrapper.

Also, I don't see the target usage for the current behaviour. As mentioned earlier, there are two types of wrapper:

  • Functions with wrappers like Clamp wouldn't be using this feature since the user could bypass the initial value easily.
  • OTOH, functions with class-bound wrappers wouldn't be using it either since users can't call the function via the initializer.

So it ends up catering to no-one.

Note that property wrappers with arguments in the attribute are not supported on parameters in this proposal for this reason. One way to support this in the future is to add a mechanism for allowing property wrappers to have per-declaration storage, so those values aren't part of the property wrapper instance at all. This idea has been floating around because for wrapped instance properties that use attribute arguments today, those values never change in most cases, so it's just burning storage on every instance of a type that has such a property.

I also want to point out that another direction we can take is to provide good autocompletion suggestions for closures taking property wrappers, as in the ForEach example. Thus, instead of introducing a new short-hand and not-so-expressive $item syntax, autocomplete could automatically apply the wrapper attribute, after determining that they type being passed is a @propertyWrapper.

Fair point. I don't agree, though, with your saying that "backing storage is private" is an internalized rule for users, as it implies so for all language users. Of course, I agree that this is probably true for regular language users; however, library authors, whom the issue you mention mainly concerns, don't treat the backing property as being private to their library, but to their users' types instead. And that's my point in general: library authors should not make any assumptions unless certain, so as to allow for more freedom, and as a result better APIs (I've have some examples in my previous post). Regardless, if a library author wants configuration points on their wrapper type, while ensuring that such a configuration in a function declaration is constant no matter what, they should probably opt for type-level parameterization (a.k.a. generics):

protocol BoundsProvider {
  
  associatedtype Value : Comparable


  static var range: ClosedRange<Value> { get }

}

@propertyWrapper
struct Clamped<Bounds : BoundsProvider> {

  ...

}

struct ZeroToOne : BoundsProvider {

  static var range = 0.0 ... 1.0 

}


func useClamped(
  @Clamped<ZeroToOne> clampedValue: Double
) { ... }

Admittedly, this is not really an elegant solution; however, I don't think that's a problem just this specific issue. I think that generic value parameters would better address this problem:

@propertyWrapper
struct Clamped<let Range: ClosedRange<Double>> {

  ...

}


func useClamped(
  @Clamped<0 ... 1> clampedValue: Double
) { ... }

Anyway, that's a whole topic on its own. But my point is that if the author doesn't want to use generic and provider protocols, they could just assert that the configuration point is as desired:

@propertyWrapper
struct Clamped {

  let range: ClosedRange<Double> = 0 ... 1


  ...

}


func useClamped(
  @Clamped clampedValue: Double
) { 
  assert(_clampedValue.range == 0 ... 1)
}

All in all, I think it's important for authors to have freedom and variety in the tools available to them so as to ease their library's development. Furthermore, despite being unexpected for a regular user, an experienced library author should already assert unknown factors and unapplied function references are just a different aspect of such factors.

I also want to add that despite my believing that unapplied references of functions with property-wrapper parameters are a desirable feature, I'm, also, not orthogonal to their separation into a different discussion.

Filip

1 Like

Ah, when I've been talking about "library authors," I've mostly been thinking of an intermediate between the property wrapper author and the ultimate end user (e.g., someone writing a SwiftUI utility library who uses the @Binding wrapper on their properties and function parameters). These are the authors that I think are likely to have internalized a rule to the effect of "adding a wrapper attribute does not expose the backing storage as API," an assumption which this proposal would break.

I alluded to this briefly in my first post (and @Lantua has mentioned something similar), but IMO the issue of unapplied references as 'solved' in this proposal actually decreases author freedom. Authors of parameter-wrapped functions have no choice but to expose the backing storage of the wrapped parameter as API, whereas for wrapped properties they have total control (and the default is that the storage is not exposed, even outside the type itself, not to mention publicly).

Punting on this part of the proposal would preserve the status quo. Authors who really do want to expose the backing storage for their wrapped parameters are free to create multiple entry points. It requires a little boilerplate, but we've historically been fine with requiring boilerplate in order to make public API explicit (e.g., forcing the synthesized memberwise initializer to be at most internal).

@hborla's posts in the pitch thread (and your examples here) provide several alternative (and compelling!) solutions to some of the problems I've raised with exposing the backing storage, but there doesn't seem to be much of a consensus around which directions are most promising. Furthermore, the answer supplied in this proposal to the unapplied references question would cut off evolution in directions that I think are promising, e.g.:

(That is, even if we decide that exposing the backing storage is desirable, it doesn't obviously follow that the backing-storage version of the function should receive the blessed foo(arg:) spelling as opposed to the 'as-called' version of the function.)

2 Likes

Let's be careful here. Whatever we choose for the function ref, it won't be giving any more freedom to the API author. The current behaviour only gives more case for the lib author to handle (direct storage passing, which would be precondition-ed away anyway).

If an experienced lib author would need to assert not to use unapplied method reference, it's a liability, not an asset.

2 Likes

I should add, (2) also reinforces the notion that the property wrapper is not part of the parameter’s type, in line with what happens to argument labels.

And in line with the proposed roadmap for those labels, one might contemplate some way of spelling the unapplied reference such that it would be possible to restore the property wrapper to a parameter (or, perhaps, even to apply a different one):

func foo(@Lowercasing arg: String) { ... }
let bar(@Lowercasing arg:) = foo
let baz(@Uppercasing arg:) = foo

The proposal as currently written forecloses exploration of any of these possibilities.

2 Likes

I recall from the pitch phase that there was some distinction in behavior when property wrappers are used for closure parameters; I don’t see it in this version of the text—has that been taken out or changed?

I'm not 100% sure which distinction in behavior you're talking about. I did restructure the Detailed design section between pitch and review (and added missing details based on the pitch discussion), but the design itself didn't change.

The main distinction that I can think of is that the call-site transformation that wraps the argument in init(wrappedValue:) doesn't happen for closures. This is in the proposal under the Call-site transformation section. There's an additional restriction on function parameters that the wrapper must support init(wrappedValue:) which is outlined in the Restrictions on property-wrapper parameters section.

3 Likes