Revisiting requiring explicit `self.` when passing a method as an escaping closure

Evolution Pitches
1

A little over a year ago, I posted a message asking whether an explicit self. should be required when passing an object's method as an escaping closure.

For example:

class KittenPartyController: UIViewController {
    private func loadKittens() {
        webService.fetchKittenPhotos(completion: displayKittenPhotos)
    }

    private func displayKittenPhotos(…) { … }
}

From a reference-counting perspective, this is roughly equivalent to:

webService.fetchKittenPhotos(completion: { self.displayKittenPhotos(…) })

Both code samples create strong references to self. But, in the second case, if you omit the self., you get a compiler error:

Reference to property 'displayKittenPhotos' in closure requires explicit 'self.' to make capture semantics explicit

In the first case, there is not even a warning.

For the same reasons that it makes sense to require explicit self. in the closure, I think it would make sense to require an explicit self. when passing a method. Like:

webService.fetchKittenPhotos(completion: self.displayKittenPhotos)

The motivation for my original post was that this pattern had led to a retain cycle in our code. My motivation today is the same -- I came across another retain cycle that was hard to spot due to this implicit self capture.

Thoughts?

7 Likes
2

This doesn't seem like the kind of thing that we could require, but even a warning would be very much appreciated here. I'm in favour of the idea!

3

This doesn’t seem like the kind of thing that we could require

Yeah, it would be source-breaking, so it would probably need to be introduced as a warning, and then maybe upgraded to an error in the next major version.

4

I am +1 for this, but I think that if we are going to introduce such a breaking change, we may as well add some syntactic sugar for passing self weakly, since this is something that you often mean to do when passing a method as a closure, but forget about it since the compiler doesn't stop you.

Right now if we want to do it, we go like:

webService.fetchKittenPhotos(completion: { [weak self] args in self?.displayKittenPhoto(args) })

which seems a bit ugly to me, especially as the number of arguments of the method grows.

Maybe something like

webService.fetchKittenPhotos(completion: [weak self]?.displayKittenPhotos)

In this case the type of the expression [weak self]?.displayKittenPhotos would be ((Args) -> Result)? (assuming the the type of the method itself was (Args) -> Result). And if the signature of a higher order function doesn't allow nil arguments, we could write

webService.fetchKittenPhotos(completion: [weak self]?.displayKittenPhotos ?? {})

But that doesn't seem to look good to me either. Maybe someone has a better idea about it?

2 Likes
5

I would also like to see an easier way to pass self weakly, but I'd prefer to keep that as a separate discussion.

The requirement for an explicit self. here is specifically targeting cases where self is being captured strongly.

2 Likes
6

It is not immediately obvious to me that prepending self. to the method name would imply that self is being captured strongly by the callee. Most uses of self. are to disambiguate. While you are correct that use within escaping closures are an exception, it doesn't strike me that the example would be interpreted that way, versus the more common use for disambiguation.

3 Likes
7

TLDR +1 for warning for all questionable scenarios for non-explicit capturing.

There are a lot of scenarios where it is not noxious is self captured or not. For example following scenario can seriously puzzle developers:

class KittenPartyService {
    func displayKittenPhotos(…) { … }
}

class KittenPartyController: UIViewController {
    let service = KittenPartyService()
    private func loadKittens() {
        webService.fetchKittenPhotos(completion: service.displayKittenPhotos)
    }
}

Does this captures self or just a service?

8

Your example only captures service. What's puzzling about it?

1 Like
9

Yeah, this is the answer, it is not a trivial answer to arrive. Also these two things expressed very closely but do have significant difference and quality impact. There is no trivial way for an engineer to figure out what is actually will happen besides a research or an extensive application of logic. At this point I even curious, what source of information in Swift documentation should give an engineer a knowledge about what is captured when in an original scenario and in one that I provided.

1 Like
10

In the case of a method reference foo.bar it is pretty simple -- a new closure is constructed capturing the result of evaluating foo. The tricky part is that if bar is an instance method and you're inside an instance context, then bar by itself is equivalent to self.bar.

2 Likes
11

+1 on the required self and also +1 on introducing some kind of sugar to use self on the function reference weakly.

12

I agree that it doesn't necessarily aid readability. In my view, it's less about making it easy to read correct code, and more about making it impossible [hard] to write incorrect code.

13

Yes -- here is another type of implicit capture that I've seen:

class KittenPartyViewController: UIViewController {
    func hideKittens(animated: Bool) {
        func changeOpacity() {
            kittensView.alpha = 0
        }

        func hide(_: Bool) {
            kittensView.isHidden = true // implicitly captures strong self
        }

        if animated {
            UIView.animate(withDuration: 2, animations: changeOpacity, completion: hide)
        } else {
            changeOpacity()
            hide(true)
        }
    }
}

The implicit strong reference to self in the local hide() function escapes when passed as the completion handler for UIView.animate(). It might be nice to have a warning in this case too, but that seems like a much more difficult problem to solve. (In practice, I've never seen a retain cycle created this way either.)

For the case of passing a method as a closure, I think the benefits are balanced with the difficulty of implementing the check. (This is just a guess, I don't actually know if it would be hard to implement.)

14

I had a proposal to also resolve this. Its similar to yours: https://github.com/hartbit/swift-evolution/blob/improving-capturing-semantics-of-local-functions/proposals/XXXX-improve-capture-semantics-of-local-functions.md

15

@nonsensery does changeOpacity() not capture self? Mhm, seems I am missing something here.

16

Yeah, changeOpacity() also silently captures self. It’s passed as the animations argument, which is more-or-less non-escaping though.

17

IMO if we're going to make a change in this area that increases the requirement for classes, we should be thinking about decreasing it for other types. The compiler also seems to force explicit self in many cases where there's no good reason for it.

2 Likes
18

To summarize the discussion so far, here are the additional suggestions that I've noticed in this thread:

  1. Add syntax for weakly capturing self, without creating an explicit closure (via @broadway_lamb -- link)
  2. Extend this requirement to cases where strong self escapes via a locally-declared function (via @hartbit -- link)
  3. Loosen the explicit self. requirement for cases where self is not a reference type (via @dabrahams -- link)

It's really helpful to see these additional ideas, since it's important to keep the big picture in mind when proposing changes to the language.

I like the scope of the change I've proposed because it addresses a real pain point (for me at least), seems like it will be reasonably easy to implement, and shouldn't conflict with any of these other suggestions.

I think a formal discussion would be helpful now, so I'm going to write up a draft proposal and solicit feedback on that.

Thanks for all of your responses!

1 Like
19

I was about to post something similar but then I found this old thread. I'd love to see this change and the future release of Swift 6 might allow us to introduce it as a source-breaking change or even just with a warning. I've run into the issue several times in the past few months while doing code review and tracking down memory leaks. It's a hard edge in the language IMO, hard to spot, and also inconsistent with how self capture is handled elsewhere.

20

It seems like a solid idea to me
+1