Pitch: Syntactic sugar for circumventing closure capture lists

I use it often in iOS coordinators when closures on vc need to access parent vc’s (like a navigation controller). Tricky to catch.

What situations are there where that would have a negative result? Assuming the performance is close enough to not be relevant.

It obviously depends on the case but 90% of async stuff happening in an app screen has this problem. The screen (self aka VC or VM or whatever) may go away at any point and operations may finish after. In some apis callbacks will still be called (for example giving you a cancellation callback) and thus weak self is the safer bet. In that case guard self is fine, silently ignoring is what you want.

2 Likes

Happy to be re-educated here, but here's my understanding:

Say we have a UIView with a UIViewPropertyAnimator:

class MyView: UIView {

   ...
   var propertyAnimator: UIViewPropertyAnimator?
   ...
}

At some point, due to some event, we want to animate some properties on that view and do some cleanup so we have:

class MyView: UIView {

    func performImpressiveAnimation() {
        let animator = UIViewPropertyAnimator(duration: 1, timingParameters: UISpringTimingParameters())
        animator.addAnimations {
            self.myFactoredOutAnimationInstanceMethod() // strongly held self, retain cycle
        }
        animator.addCompletion { _ in
            self.myPostAnimationCleanupInstanceMethod() // strongly held self, retain cycle
        }
        animator.startAnimation()
        self.propertyAnimator = animator // retain the property animator or no animations
    }

}

If we don't retain the property animator in this situation – the animations don't happen. So we make it an instance property on our UIView.

Therefore when we add the animation blocks and completion handler we have a retain cycle via: self -> animator -> animationBlock -> self.

We need a capture list for our closures.

For the addAnimations(_:)call we're OK with an unowned self – our instance always kicks off the animation so we can guarantee its safety. For the addCompletion(_:) call we need to use weakly as the view may be removed through some user/system event and deallocated prior to the animation block completing and an unowned self would yield a crash.

2 Likes

Unowned is not good default, agreed. That was my mistake.

I guess between strong and weak, it's question of balance. With strong you don't lose the reference and then you don't get crashes. With weak you might, especially if you force it, but usually you have the self?.foo, i.e. question marks to take care of it (and makes programmer knowledgeable about the issue). With weak you break most, if not all reference cycles, so you're not burning memory, whereas currently with strong it's given that your program will have one or more closure reference cycles unless you explicitly do the weak dance yourself (and you have to know about it).

At least for me, the default in closures is almost always that the (view) controller outlives the lifetime of the closure (e.g. a closure running on button click). So the weak reference to self will always be valid when it needs to, but also will not retain the closure in memory when controller goes away. so ARC does the right thing automatically, instead of me having to jump into manual memory management mode.

If you take a look at github repos and just to try find:

guard let strongSelf = self else { return }
guard let `self` = self else { return }`
...

you will get bazillion results.
The main problem is to understand when weak unowned should be used and even for experienced Swift users it is sometimes hard to grasp which one should be used as it strongly depends on the context. Thats why we have such situations were people add [weak self] regardless if is needed or not but it’s easier, and faster to add this than thinking about each case and for sure finding a memory leak which in case of closures is rarely pleasant experience.

I believe that whole community would love to see improvement in this area as writing guard let for self became like a mantra. We can improve this situation by compiler warnings and better tooling or/and improving the syntax of closures to lesser the problem.

I would be personally satisfied if at least for closures that returns Void we could solve it by the syntax sugar in a form that was already proposed few times. For any other closure I don’t think we need any additional syntax as probably it won’t be significantly shorter and more descriptive than typing guard inside the closure as we use to now.

{ [weak(guard) self] in ... }
{ [unowned(guard) self] in ... }

I would only suggest to even use shorter syntax to avoid writing weak and unowned and instead ? and !

{ [guard self?] in ... }
{ [guard self!] in ... }
2 Likes

Hi all, thanks for all the feedback so far. To bring further discussion back on track and to summarise:

  • There appears to be wide spread frustration with the current capture list syntax – particularly in regards to class types.
  • Common Cocoa APIs and idioms frequently and pervasively require developers to utilise non-owned callbacks to perform tasks without use of a retain cycle. It seems likely highly that the majority of capture lists 'in the wild' are of this variety.
  • The inline method of passing these callbacks often results in unclear and aesthetically dissatisfying code, with common and repetitive boilerplate that – apparent from these discussions – many developers have an aversion to.
  • The idiom of factoring out these inline methods to instance methods, and passing the instance method instead – whilst cleaning up the call-site – silently creates a strong reference that, from these discussions, clearly many developers aren't expecting.
  • The 'correct' syntax for factoring out these instance methods requires an intermediary forwarding closure of the same arity as the instance method that, I assume, must appear again and again in idiomatic Cocoa applications – and I imagine any code that makes use of class types and asynchronous routines.

Whilst there seems to be some argument on the solution, there is no doubt appetite for
a solution.

5 Likes

In the interim – derived from the ideas of @sveinhal above – I've come up with something that I'll likely use in my own code to clean-up call sites where these callbacks occur.

It's not ideal as 1) it requires any class that uses it to conform to a protocol to gain access to the functionality (we can't extend AnyObject), and 2) it requires a method per arity as the obvious way using generics leads to a retain cycle.

Callbacking

protocol Callbacking: AnyObject {}

extension Callbacking {
    
    // Nullary methods

    func ucall<T>(_ classMethod: @escaping (Self) -> () -> T) -> () -> T {
        return { [unowned self] in classMethod(self)() }
    }
    func wcall(_ classMethod: @escaping (Self) -> () -> Void) -> () -> Void {
        return { [weak self] in
            guard let self = self else { return }
            classMethod(self)()
        }
    }

    // Unary methods

    func ucall<T, A1>(_ classMethod: @escaping (Self) -> (A1) -> T) -> (A1) -> T {
        return { [unowned self] a1 in classMethod(self)(a1) }
    }

    func wcall<A1>(_ classMethod: @escaping (Self) -> (A1) -> Void) -> (A1) -> Void {
        return { [weak self] a1 in
            guard let self = self else { return }
            classMethod(self)(a1)
        }
    }
    
    // further n-ary methods omitted for brevity
}

Usage

From:

animator.addAnimations { [unowned self] in
    self.myFactoredOutAnimationInstanceMethod()
}
animator.addCompletion { [weak self] in
    self?.myPostAnimationCleanupInstanceMethod($0)
}

To:

animator.addAnimations(ucall(Self.myFactoredOutAnimationInstanceMethod))
animator.addCompletion(wcall(Self.myPostAnimationCleanupInstanceMethod))

And here's a version up to 20-arity for those who'd find it useful: https://gist.github.com/tcldr/174f43f6e39379bc5f0cda686ac46f84

3 Likes

FWIW, you can pass self and do it with standalone functions so you don't need to add the protocol adherence:

func ucall<Self: AnyObject, T>(_ passedSelf: Self, _ classMethod: @escaping (Self) -> () -> T) -> () -> T {
    return { [unowned passedSelf] in classMethod(passedSelf)() }
}
func wcall<Self: AnyObject>(_ passedSelf: Self, _ classMethod: @escaping (Self) -> () -> Void) -> () -> Void {
    return { [weak passedSelf] in
        guard let passedSelf = passedSelf else { return }
        return classMethod(passedSelf)()
    }
}

// Unary methods

func ucall<Self: AnyObject, T, A1>(_ passedSelf: Self, _ classMethod: @escaping (Self) -> (A1) -> T) -> (A1) -> T {
    return { [unowned passedSelf] a1 in classMethod(passedSelf)(a1) }
}

func wcall<Self: AnyObject, A1>(_ passedSelf: Self, _ classMethod: @escaping (Self) -> (A1) -> Void) -> (A1) -> Void {
    return { [weak passedSelf] a1 in
        guard let passedSelf = passedSelf else { return }
        return classMethod(passedSelf)(a1)
    }
}

// Usage:
animator.addAnimations(ucall(self, Self.myFactoredOutAnimationInstanceMethod))
animator.addCompletion(wcall(self, Self.myPostAnimationCleanupInstanceMethod))

slightly less concise, but less steps to use it.

2 Likes

I've chewed on this for a few days and I've come to two realisations:

  1. [...] in Swift just feels wrong every time I write it.
  2. Just like !, unowned feels like a disaster waiting to happen (because it usually is).

Specifically, what exactly do we want with unowned other than to skip writing guard... when we know it's safe. That doesn't fee Swifty to me.

I'd go with that - strong for non escaping, weak for escaping. No options.

3 Likes

Perhaps I'm interpreting your argument wrong, but unowned is very useful in certain contexts. Now granted those contexts are pretty narrow, and generally involve some tight coupling between types. This generally being, from my experience, when there's a strong Parent/Child relationship going on, and there's no concurrency involved.

Take this contrived example:

class Parent {
  var name: String
  private var child = Child()

  private struct Child {
    var action: () -> () = {}
  }

  init(name: String) {
    self.name = name

    self.child = Child(action: {[unowned self] in
      print(self.name)
    })
  }

  deinit {
    print("Parent going away")
  }
  
  func doSomething() {
    child.action()
  }
}

var p: Parent? = Parent(name: "bob")

p?.doSomething()

p = nil

Now granted, this is fairly contrived. Most times I've used unowned isn't in capture lists, but as a modifier on a Child's reference to the parent.

It is fairly contrived, but I probably have more contrived code in production. :smile:

I agree that unowned in the case makes the ergonomics nicer, but [weak self] is still valid and safe. And it could be argued it's safer given theoretical (though ill-advised) future changes that would break the tidy ownership model.

1 Like

There's this curious trend in the swift community to "always use weak to prevent a crash!", but actually, it's probably better for it to crash rather than fail silently and create difficult to diagnose bugs.

Anecdotally, most of my capture lists are [unowned self]. I agree that most of the time it's where there is some tight coupling between a parent and child, and so I know if the parent exists the child must also. Beyond providing a nice hard crash if my code is doing something unexpected it also saves me a guard.

For example, if you are submitting an animation to a property animator (which is a synchronous operation), and the view you're performing animatable property changes on is currently in scope, when you add that animation there is little point in using a weakly captured instance as you have the instance in the current scope and you know the animation closure will be executed immediately.

For asynchronous use cases it is often necessary to use weakly, but certainly not as a rule. Many of the reactive bindings I create use unowned because I guarantee the lifetime of the subscription is the same as the instance I'm performing operations on.

2 Likes

IMO unless there's a risk to data/system integrity, crashing is never an option. This seems like a prime use case for an assertion in the guard statement.

The ergonomics around this are currently horrible, but all syntax being equal I'm struggling to find a reason why self shouldn't default to weak in an escaping closure.

In production code, safety and maintainability are the priority. [weak self] fulfills this in, (as far as I can tell), all cases with the only drawback of having to add a few lines of code where it may not be necessary. I don't like the way it looks and boilerplate always sucks...but that's a current shortcoming of the language and seems like more of an excuse than a reason.

TLDR: The reason why my code is safe should be due it being impossible not to be safe. Relying on my own intellect in saying "I'm sure it won't crash" is usually something I regret doing :grimacing:.

3 Likes

This is actually the problem I'm worried about; [unowned self] is just so easy and dangerous (much like !) as it has a hidden fatalError():

[weak self] in
guard let self = self else {
   fatalError()
}

I would consider it far more preferable and safer (for a given definition of safe) to have the following code:

[weak self] in
guard let self = self else {
   assertionFailure()
   return
}
1 Like

But, say on a closure executed synchronously, if there's a crash here using unowned then there's something far more insidious at play going on. If my program goes haywire – I want it to crash so I can get the logs from the relevant place!

Obj-c could be a nightmare when methods would fail silently messaging nil. On balance, I think I prefer my failures explicit. And – to be fair – Swift offers you the choice. If you want to code with your instances bound weakly – you can.

Also, it really depends on your definition of 'safe'. It's hard to know how a silent failure may manifest itself further down the line. Whilst data loss might not be an explicitly clear consequence of a silently failing closure, perhaps it does result in some inconsistency of state that is nevertheless damaging – and extremely hard to track down.

1 Like

So, I guess the problem here is: now you have an application out there. It doesn't crash at this point. Instead it crashes at some later point, or worse causes some unexpected data loss, because it's in some unexpected state which you've chosen to ignore.

Agreed, screwed either way :frowning:. However, I'd say which option we chose is an important business decision and should not be defaulted due to one option being easier to implement.

That's the point of assertions, errors, logging, etc. The decision isn't "crash or do nothing". You can fail gracefully and leave a trail of information in the wake of the problem, ideally without the end-user knowing anything about it.

A part of my UI that's already dismissed not updating properly because of some strange edge case I didn't think of is always preferable to it just crashing.

2 Likes

100%. There are use-cases for strong, unowned and weak. Hence need them all.