[swift-evolution] [Draft] Guarded closures and `@guarded` arguments

[Matthew Johnson]

> On Feb 19, 2017, at 11:35 PM, Brent Royal-Gordon <brent at architechies.com> wrote:
> 
>> On Feb 19, 2017, at 2:57 PM, Matthew Johnson via swift-evolution <swift-evolution at swift.org> wrote:
>> 
>> A guarded closure may be created by prefixing a bound instance method reference with the `?` sigil:
>> 
>> ```swift
>> let guarded = ?myObject.myMethod
>> 
>> // desugars to:
>> let guarded = { [weak myObject] in
>> guard let myObejct = myObject else {
>>   return
>> }
>> myObject.myMethod()
>> }
>> ```
> 
> I like this in principle, but I don't like the syntax. The `?` is cryptic and looks pretty strange in prefix position. I think what I'd like to see is:
> 
> 	let guarded = weak myObject.myMethod
> 
> Or:
> 
> 	let guarded = weak(myObject).myMethod
> 
> This second alternative could give us the opportunity to specify a default return value if we'd like:
> 
> 	let guarded = weak(myObject, else: nil).myMethod

I am certainly not wedded to the syntax I proposed.  Using your syntax you could conceptualize `weak` as a function that stores a weak reference and forwards all members.  That’s an interesting approach and could be useful in other places.  That said, it doesn’t work with inline closures.

> 
>> A guarded closure may be created by prefixing an inline closure with the `?` sigil:
>> 
>> ```swift
>> let guarded = ?{ flag in
>> flag ? someMethod() : someOtherMethod()
>> }
>> 
>> // desugars to:
>> let guarded = { [weak self] flag in
>> guard let strongSelf = self else {
>>   return
>> }
>> 
>> flag ? strongSelf.someMethod() : strongSelf.someOtherMethod()
>> ```
> 
> This runs into the same problem as previous suggestions for addressing the weak-strong dance: There's no particular reason to privilege this behavior over any other.

The reason is that it is a pervasive idiom when dealing with callback closures (which are extremely common).  There’s no “reason" to privilege single expression closures either but we do.

> 
> I think I'd prefer to use a slightly wordier two-part solution here:
> 
> 	1. If the capture list is just `[weak]` (or `[unowned]`), that's the default for all variables.
> 
> 	2. A `guard` with no condition—in other words, `guard else`—tries to strengthen all weakly captured variables and enters the `else` block if any fail. (Alternative: `guard let else`.)
> 
> That makes your example into:
> 
> 	let guarded = { [weak] flag in 
> 		guard else { return }
> 		flag ? self.someMethod() : self.someOtherMethod()
> 	}
> 
> This is many more characters, but I think it's also much clearer about what's going on.

What we lose by writing this out manually is the ability for an API to express a change in default capture and require callers to acknowledge that change in default by using a sigil (or some other syntactic differentiator).  Unless you suggest that the `[weak]` capture list be that indicator.  If that’s the case I think it would be acceptable.  Additional syntactic sugar could always be added later and the real problem I am aiming to solve is allowing APIs to require a change in default capture.

> 
>> #### Self reference in escaping guarded closures
>> 
>> Guarded closures do not extend the lifetime of any objects unless a `strong` capture is specified in the capture list.  Because this is the case users are not required to explicitly reference self using the `self.` prefix inside a guarded closure.
> 
> I think this is a bad idea, because the implicit `self` problem still exists here—it just has a slightly different shape. Here, if you accidentally use `self` without realizing it, your closure mysteriously doesn't get executed. This misbehavior is at least easier to notice, but it's still going to be hard to track down, since it's caused by something invisible in the code.

Lets consider the possible reasons somebody might make the mistake you describe above:

1. The most likely scenario where requiring `self.` would help catch a mistake is when a user intended to bind a local name and capture that instead.  Requiring `self` would indeed help prevent this accident and also help readers to spot it.  They did not intend to capture `self` at all.  Requiring them to type `self.` makes it clear that they did.

2. They intended to capture a strong reference to self and forgot to specify the capture list entry.  Would requiring them to type `self.` help remind them to add the capture list entry?  Probably not - they intended to add it and just forgot.  but I suppose it’s possible.

3. They don’t really understand how Swift reference counting and guarded closures work.  If this is the case they are in trouble until they get up that learning curve.  How much does requiring `self.` in guarded closures help with that?

4. The only other possible accident is that the user referenced some aspect of `self` despite not actually needing to use it.  This can happen but I doubt it’s worth placing too much emphasis on.

The question in my mind is how do we get the most value out of requiring the `self.` prefix in closures.  Do we get the most benefit if it is required exclusively where it may cause a reference cycle?  Or do we get the most benefit if it is required any time users should pay close attention to what they are doing with `self`.  If we reduce the need to use it that will call additional attention to places where it is required.

How frequently will requiring `self.` help catch these kinds of errors relative to the increased reference cycles it might help draw attention to if it were not required so pervasively?  I could be convinced either way.  I don’t think the answer is obvious.

> 
>> ### `@guarded`
>> 
>> This proposal introduces the ability for arguments of function type to be annotated with the `@guarded` attribute.  When `@guarded` is used the caller must supply a guarded or non-capturing closure as an argument.  This ensures that users of the API think very carefully before providing an argument that captures a strong reference, and requires them to explictly state their intent when they wish to do so.
> 
> Even in this modified form, I think this is a bad idea. The person writing the closure is the one who knows whether they want it to extend objects' lifetimes. The library author accepting the closure has no idea whether the objects the closure captures are models it needs or controllers it shouldn't keep—or, for that matter, models it shouldn't keep or controllers it needs.
> 
> If we really think that this is a serious problem, we should change `@escaping` closures to always capture weak unless they're explicitly told to capture strong and be done with it. If that sounds like a hare-brained idea, then we shouldn't do it half the time and not do it the other half.

What I am proposing to do is pretty much exactly that, except it uses a mechanism that makes that change in default visible at the call site be requiring a sigil.  This means that callers must be aware of the change in default - if they aren’t then they will get a compiler error.  

I believe there is an important class of API for which strong capture is the wrong default.  That is why I am proposing a mechanism to allow an API to declare this change in default and require callers to acknowledge the change in default.

> 
> -- 
> Brent Royal-Gordon
> Architechies
>