Guarded closures

(Matthew Johnson) #1

The topic of guarded closures came up recently in the thread The Future Of [weak self] Rebinding. I promised to look at the history of the topic and start a new thread for those interested in continuing the discussion. There have been two related proposal drafts in the past.

The first was for a relatively straightforward addition of a guard capture specifier that would be available to closures returning Void (and possibly Optional). The draft can be found here: https://gist.github.com/emaloney/d34ac9b134ece7c60440 and the discussion thread is here: [Draft Proposal] A simplified notation for avoiding the weak/strong dance with closure capture lists.

The second draft is one I wrote a little over a year ago which explores introducing a new kind of closure which uses guarded captures by default (while still allowing an explicit capture list to override that default). The draft also introduces an @guarded function argument attribute which can be used in place of @escaping to require the argument to be a guarded closure. The intent behind this is to allow an API to indicate to users that correct use of the API will not usually extend the lifetime of an object and to make frequent usage patterns syntactically concise.

IIRC at the time I was also thinking about how to introduce guarded references to closures which would need to be Optional and would be notified when one of the guards was released. This would allow a library to free up resources that are no longer needed when a callback becomes a no-op.

For a number of reasons I am less convinced that the additional complexity in my draft is worthwhile than I was a year ago. Additionally, introducing the basic guard capture specifier is a step towards more sophisticated guarded closure features should we wish to introduce them in the future. What do others think? Is there a desire to pursue guarded closures in the Swift 5 timeframe and if so, which direction is preferable? Further, if there is a desire to pursue guarded closures is anyone interested in working on the implementation?

11 Likes
Weak closure reference
Pitch: Weak method storage modifiers (aka weak references)
Easy strongified weak `self` in closures
(Adam Shin) #2

+1 to the guarded closures proposal. This would streamline syntax and offer an easy way to bypass a large set of retain cycle issues.

I’d be more hesitant on the @guarded attribute. The decision of how to capture values in a closure, including self, seems like it should be made at the point of use rather than in the API declaration. What use cases do you have in mind for this?

On a tangential note – would it be a good time to revisit this discussion (removing the explicit self requirement in closures when self is included in the capture list)? It picked up a good bit of support, but looks like it was forgotten. Might be worth revisiting, in the light of this proposal and other discussions on rebinding self in closures.

(Maik) #3

is there any idea for passing functions of class instances as closures?

func someFuncWithClosureArg(_ closure: (Int, Int) -> Int) {
    closure(5, 7)
}

func add(lhs: Int, rhs: Int) -> Int {
    return lhs + rhs + someInstanceInt
}

someFuncWithClousureArg(add) // possible memory leak here

This example does not really make sense, but I guess the use case is clear. Passing an function of a class instance as parameter, will implicit pass a strong self. Some keyword to create weak reference instead would be nice.

(Gwendal Roué) #4

This proposal is nice. It has a lot of acknowledged caveats, and that’s honest and good.

Those caveats are serious.

Self rebinding would address the same problem in a much more general way, IMHO.

1 Like
(Matthew Johnson) #5

@Maik639 I addressed this use case in the draft I wrote by introducing a ? prefix sigil for instance methods of classes.

1 Like
(Matthew Johnson) #6

@gwendal.roue I agree. If we had to choose one or the other I would also choose rebinding. I don’t feel too strongly about guarded closures anymore but there was enough interest in the topic that it deserves a dedicated thread. I see the basic proposal as syntactic sugar that builds on top of self rebinding.

The direction I had been exploring goes a bit further and attempts to solve related problems that self rebinding does not address. As I mentioned in the opening topic, I am less confident in that being worthwhile than I was a year ago.

(Matthew Johnson) #7

I agree that the decision about capturing should happen at the point of use. Nothing discussed here changes that. I don’t have a strong opinion on the topic but do want to make it clear that usage sites would remain in full control of every capture.

It is relatively unusual to want to extend the lifetime of an object by capturing a strong reference when using many async APIs. Accidentally doing so is a common source of memory leaks and bugs in Swift code. It may be easier for some programmers to avoid such bugs if such APIs were able to invert the “default” capture mode. The inverted default would be visible at the usage site in order to ensure readers understand what is happening. An example of an API that might choose to use this feature is an API for performing network requests.

(Tony Allevato) #8

In general I’m not opposed to optimizing for common cases, but if we move forward with self rebinding, I have trouble rationalizing guarded closures. My biggest concerns with the concept are:

  • A syntax like [guard self] or some similar attribute combines two very different concepts—memory ownership and control flow—into a single keyword. That’s an oddly specific and heavy weight to add to the language.
  • This feature only works for a subset of possible closures—those which return Void (or possibly Optional, as mentioned above).

That’s quite a bit of magic to avoid writing guard let self = self else { return }.

9 Likes
(Boris Triebel) #9

I also think that a) self rebinding is a preferable solution to a part of the described problem and b) using the keyword guard in two places in different ways works against the consistency of the language.

(Andrés Cecilia Luque) #10

IMHO here we are mixing two different proposals that solve different issues, and there is no need for opting for one or the other (I would love to have both):

a) Allowing shadowing and rebinding of self [recent discussion, old evolution proposal, recently merged implementation] (please note that this proposal is not being discussed here, I am just considering it in order to explain why I think it is different): it intents to modify the usage of the self keyword. This is an important change for the language, and not syntactic sugar, because (as I understood) it basically proposes to stop treating self as a reserved keyword and start treating it as any other identifier. This allows to unwrap self by reasigning it, instead of requiring a new identifier:

Before:
    let closure: () -> Void = { [weak self] in
        if let strongSelf = self {
            strongSelf.found = true
        } else {
            strongSelf.found = false
        }
    }

After:
    let closure: () -> Void = { [weak self] in
        if let self = self {
            self.found = true
        } else {
            self.found = false
        }
    }

b) Introducing a guard keyword in a capture list [recent discussion, Unofficial evolution proposal]: this introduces a guard keyword as part of the capture list of a closure, which is not only intended to be useful when capturing self, but can also be used to capture any other identifiers:

Before:
    let closure: () -> Void = { [weak self] in
        guard let strongSelf = self else {
          return
        }
        strongSelf.intProperty += 1
    }

After:
    let closure: () -> Void = { [guard self] in
        self.intProperty += 1
    }

This is syntactic sugar for a specific closure usage case, but it is worth considering because this usage case appears a lot. In order to be eligible to use guard in a capture list, the closure has to satisfy that:

  1. The closure intents to capture one or several properties weakly.
  2. The code inside the closure is only intended to be executed if all the weakly captured properties are not nil.
  3. The closure returns Void.

The rationale I have in mind for this proposal is extremely similar to the rationale given for the recently accepted toggle() proposal:

  • commonality: the need to unwrap a weakly captured identifier is a common one across multiple different problem domains

  • readability: the addition of this command can significantly improve readability for the cases in which it is possible to use it (which are many, despite the requirements that the closure has to comply with)

  • consistency: while capturing weakly and then unwrapping is fairly trivial, it is good to have a consistent way of doing this across all swift codebases, rather than having multiple variants (with multiple variants, I mean that the identifier given when unwrapping may be different in every case. For example, when unwrapping self, developers assign the new strong reference to indentifiers such as weakSelf, strongSelf, ss, sSelf, wSelf...)

  • correctness: being able to give a different identifier for the unwrapped and weakly (optional) captured object, can lead to this not-so-correct code (which eventually can lead to bugs):

      let closure: () -> Void = { [weak self] in
          guard let strongSelf = self else {
            return
          }
          strongSelf.intProperty += 1
          self?.anotherIntProperty += 1
      }
The Future Of [weak self] Rebinding
(Xiaodi Wu) #11

A few issues:

  • First, your arguments about inconsistency across code bases now go away because guard let self = self else { ... } is now possible; no one needs to decide anymore between strongSelf, ss, etc.
  • As has been said before regarding other shorthand sugar pitched for guard, eliding the entire else block is not considered a win for correctness. Instead, being explicit about what happens when the condition is not met can be important for reasoning about code. Silently doing nothing is not always correct--it may be that you really want to halt execution with fatalError(), for example--and we should not be creating syntax that makes silently doing nothing the default behavior.
  • The bar for adding new syntactic sugar is higher than the bar for adding new methods; the same criteria are a good starting point but wouldn't be sufficient.

This topic is now very old. I think that, if you have new concrete use cases that can strongly motivate the addition of new syntax in the face of recent changes that enable guard let self = self else { ... }, it would be worth launching a new discussion. However, in the absence of new evidence, if anything the need for new syntax is less rather than greater now, in my view, especially seeing how some of the arguments you're raising in favor are now superseded by the recent change.

2 Likes
(Nikita Leonov) #12

It makes simple to write a code similar to if without else and end up with an unpredictable / undesired behavior. An existing weak self jump through the hoops always expect you to provide else where you should do all the handling, logging etc. [guard self] makes it normal to not handle a faulty scenario, this will enforce a bad habits for less experiences engineers.

(Rod Brown) #13

Has this been confirmed? I thought this was still considered a bug...

(Xiaodi Wu) #14

The fix has been merged into master.

2 Likes
(Andrés Cecilia Luque) #15

My first point was to get clear that the two proposal solve different issues. Thus, there is no need to choose one or another. As I see it, discussing if we want [guard self] is independent of the guard let self = self else { ... } proposal. Can we agree on that?

About the rest:

In case guard let self = self else { ... } is confirmed, you are right about the consistency argument.

If you want to halt execution with fatalError(), then you do not use [guard self]. As explained, [guard self] is syntactic sugar for a very concrete scenario, where you dont want to execute the code in the closure if any of the identifiers in the capture list is nil. And despite being a very concrete case, in my opinion the [guard self] proposal makes sense due to the high amount of times this pattern appears on the code.

I understand. As I see it, this toggle() proposal that was recently accepted is adding syntactic sugar that is really useful only in a very concrete case: when in larger nested structs. If you are not in an scenario with a large nested struct, then the advantage for using it is not so much.
If such a proposal was accepted, I wonder why not this syntactic sugar proposal that is being discussed and requested during years would not be accepted, or at least seriously considered as a proposal.
I think the acceptance and rationale of the toggle() proposal is a good starting point for, at least, making an official [guard self] proposal, and end up the recurring discussions about it.

Yes, this topic is very old, but even after discussing it a lot, I have been reading all the information I could find about it and the result is:
a) This was never oficially proposed in swift evolution.
b) I could not find a proper rationale of why there has never been an official proposal, or why this syntactic sugar is undesired. Seems to me that when the discussion was happening, it was not the right time for this, or the people supporting the discussion gave up.
c) Because there has never been an official proposal, there is no official rationale from the core team for or against it.
d) From the discussion and the comments in here, seems to me that there is still interest on this, or alternatively, on a solution for the capturing weakly and then unwrapping commonly repeated scenario.

I see your point. Maybe guard is not the right keyword for this. It seems to me the right one, but of course having a similar implementation with another keyword that fits better is an alternative.

Well, I think what less experiences engineers do is not use the capture list, thus creating reference cycles. And, at the moment they realise that when using closures almost everything should be captured weakly and then unwrapped, is when you start wondering: is there any alternative for this commonly repeated code [weak self] in guard let strongSelf = self else { return }?
Also, if you are using [guard self] you should be aware of what you are doing. Using the features of a language without even reading what they are intended for is a bad habit. Using features of a language that allow you to write clearer, more readable code, I think is a good habit.

(Xiaodi Wu) #16

In the Swift Evolution process, proposals are reviewable when the author creates a document that achieves some amount of consensus during the pitch phase. As you can see from the preceding conversation here, members of the community have doubts that this is the most desired solution.

This may fine as a general principle when you're supervising junior developers, but from the perspective of language design you can't wave away the potential of creating sharp edges in the language by saying, "you should know what you're doing." I'm not aware of any place in the language that has hidden control flow like this.

In any case, it can be interesting to visit old topics when something new changes the context. To drive the conversation forward, do you have concrete use cases that haven't been previously considered?

(Tino) #17

btw:
Has it ever been discussed to make weak the default for closures?
It's probably to late for such a change, but besides compatibility, I think it would be better if you could omit [weak self] and had to explicitly state when you want a strong reference.

(Andrés Cecilia Luque) #18

No. I am bringing back the discussion because:
a) It is being mentioned lately during the discussion of the "Allowing shadowing and rebinding of self" proposal (with people for and against it), and I see it keeps getting support here.
b) I could not find a rationale with purely technical reasons agains it (most of the reasons agains it that I have red are very subjective to the developer talking).
c) Considering that I could not find purely technical reasons agains it, I looked for a subjective reason against it coming from the core team. I could not find it either.
d) I saw the recently approved toggle() proposal, and I think the rationale for accepting it has several similarities with the rationale I would give to this [guard self] proposal.
e) Due to point d), and considering that this discussion has happened already several times without a clear acceptance/rejection outcome, I think it would be interesting to push a bit more for it, or even make an official proposal in order to get more visibility, and eventually an acceptance/rejection rationale, making things clear and preventing the discussion from happening in the future.

This discussion being repeated over an over is an indicator to me that several developers are seeking for an alternative for [weak self] in guard let strongSelf = self else { return }. Having a clear rationale for or againt it would help.

As an example, the "Allowing shadowing and rebinding of self" proposal has been accepted in September 2016, but an implementation of it did not happen until March 2018, even though the context did not change from 2016 to 2018. And I believe that the reason for this is that in 2016, the focus of the community was on other more important topics, but now in 2018 the focus is shifting thowards this not so important - but still possible to improve - syntactic issues that we are seeing lately.

(Andrés Cecilia Luque) #19

@Tino agree, but that would be another discussion thread, in my opinion. Gather some old threads and discussion about it, and go for it ;P

(Xiaodi Wu) #20

The Swift Evolution process doesn't really work this way. An idea needs to achieve a certain amount of uptake and consensus before review. When people raise reasons why they are not in favor of the proposal, these need to be addressed or answered in some way and not dismissed as "subjective." Most ideas are brought up and fade out because it never gets to a point where it's ready for review, and pushing an idea forward after extensive discussion requires more than "+1."