`guard` capture specifier for closure capture lists

Indeed

if we decide this particular aspect is "more bug than a feature" it's probably very easy to fix - just treat var c: Int? as if it was written as var c: Optional<Int> when it comes to auto generated default initializer. but that'd be a source breaking change, so needs to go via "warning" -> "deprecation" -> "error" stages over a few years.

1 Like

I think this is an important point that probably deserves more attention. What if self being nil is actually indicative of a problem which is being silently ignored? weak self has legitimate uses, but it can also be used thoughtlessly, and it seems like this proposal could make it easier to do so.

5 Likes

Good question. I agree that [guard self] will not be applicable to all [weak self] closures that return Void. Folks will need to reason through the implications of "this closure will not run if self no longer exists".


I think it's worth considering what overall portion of weak self closures fall into the category of "it is clearly correct for the closure to not run if self no longer exists".

I took a brief survey of [weak self] closures in the Airbnb codebase (2M+ lines of Swift code):

  • ~9,500 uses of weak self
  • ~2,000 weak self closures that start with guard let self = self else { return } (30%)
  • ~4,500 weak self closures that use self?. in the first line (which, for Void closures, implicitly does not handle the case where self is nil) (47%)

These two cases (that I could trivially regex for) cover 70% of all weak self closures in our codebase. It seems likely that this could cover an even larger percentage with more sophisticated regexes (that handle multi-line guard cases, for example.)

Within the context of a UI codebase, it seems reasonable to make the claim that the proposed semantics of [guard self] cover the large majority of existing use cases of [weak self].

3 Likes

Absolutely correctness is the biggest concern here. However, unowned self is a legitimate optimization in many cases that also makes an assertion about the object graph. How many of those [weak self] could be upgraded to [unowned self]?

2 Likes

There are valid reasons to prefer [weak self] with a guard over [unowned self]. Many folks consider unowned self as "risky" as force-unwrapping optionals.

This is why we have ~9,800 weak self closures in our codebase, but only ~250 unowned self closures.

5 Likes

So, I want to make a non-technical case for weak self. I've made a similar post in the past, but want to reiterate it here. For me, the largest difference between weak and unowned is the fact that when an assumption about a weak reference is broken, it's handled via a low-priority bug report that gets fixed in the next release. A broken assumption about an unowned reference results in a P0 crasher that has to have everything dropped in order to put out a hot fix. It might be more likely to catch unowned issues before shipping, but I'd take many weak bugs over one unowned bug, purely from a quality of life perspective.

Even if I can fully plan out the ownership graph and be certain that unowned is appropriate in a particular case, such a decision isn't robust against future changes and refactorings that may have non-local implications.

8 Likes

Thanks, both. I think those two posts capture the tradeoffs pretty well: with weak the consequence of your assumptions being wrong is that an action doesn't happen (occasionally catastrophic for users if it results in data loss, but usually benign; hard to debug); with unowned the consequence is that the app crashes (pretty much always catastrophic for users; easy to debug). "Easy to debug" is extremely valuable if the situation comes up during your testing, but may not be worth the "catastrophic for users" when it doesn't—and object graph issues are, for better or worse, hard to test.

Still, as someone who believes force-unwrapping is often the right tool for the job, a policy of "default to weak" definitely rubs me the wrong way, and adding this sugar encourages that.

Avoiding crashes with unowned requires non-local reasoning. It is impossible to know whether or not the captued objects will still exist when the closure is called without reading all code that can potentially call the closure.

Closures aren't called arbitrarily. They're event handlers, or pipeline transformers, or tasks-on-queues. You should have an idea of the lifetime of a closure just like you have an idea of the lifetime of a framework object. (And none of this discussion applies to non-escaping closures, which can always assume self exists. If you're using [weak self] with a non-escaping closure, you can freely delete it.)

8 Likes

FWIW, in cases where the ‘self is gone’ situation really Should Never Happen, I’ll usually combine weak self with an appropriate assertionFailure, which IMO is the best of both worlds to some extent: highly visible and easy to debug when testing, but not catastrophic if it slips through to production.

4 Likes

I still see the proposal valuable for example;

class Model {
  func printDate(_ date: Date) {
    print(date)
  }
    
  var sub: AnyCancellable?
    
  init() {
    sub = Timer
      .publish(every: 1, on: RunLoop.main, in: .default)
      .autoconnect()
      .sink { [weak self] in
        self?.printDate($0)
      }
  }

Would that be safe to change it to unknown?
Or in such cases


class MyViewController {
  func bind(_ publisher: AnyPublisher<Int, Never>)
    -> AnyCancellable? {
    
    return publisher
      .sink(receiveValue: { [weak self] in
        self?.displayValue($0)
      })
  }
}

If the view was dismissed, no reason to do any display operation on it.

Also, as the proposal states, closure won't be called at all so maybe it gives additional opportunity to the compiler to optimize all of those calls for weak self... dance closures that immediately return.

Examples for the convinience taken from Using self, weak, and unowned in Combine

Not to be a wet blanket, but I have to give this a -1. I get what the proposer is getting at, and I get why a lot of people like this in theory, but it feels like one of many examples of features that aren’t really helpful to programming.

We as programmers often tend towards code that looks clean. We try to abstract things away to make a bit of code appear simple, especially during and after writing said code. But that doesn’t necessarily make the code better. Optimising for writability or perceived cleanliness is often a mistake as it comes at the expense of learnability, readability, and debugability.

I understand the appeal. guard let self = self else {} is boilerplate, but it is good boilerplate. As others have said, the important part is what happens in the else. Even in simple cases, you may want more than just a simple return. It’s also far easier to debug if things are going wrong. There’s also the point about implicit self. which appeals to those who use that feature, though I’d argue implicit self. is a feature is the poster child for the “optimising for writability & perceived cleanliness over learnability, readability, and debugability” issue (as anyone who has tried to learn about some Swift code without syntax highlighting can attest!)

The intention is good, but I think this falls foul of too many genuinely important things in favour of some superficially important things. It makes things easier in the short term, but does it help long term maintainability? I’d argue no. Eliminating boilerplate that has no purpose and is used all over is good (see when @property syntax was added to Obj-C). Eliminating boilerplate that is arguably there to make you think, and only in a narrow set of cases at that (I’d argue this has to support all return types to be considered), isn’t really worth adding syntax to the language.

Your self in the here and now may hate it, but your future self will likely thank you that it’s not there :wink:

13 Likes

I'm mostly a lurker around here, but this one is something that I've been giving a lot of thought to, so I thought I might actually weigh in.

We can talk ourselves in circles all day about the philosophical implications of optionals — and people regularly do — but I think there will always be a little bit of a divide between the people who view the inconvenience of optionals as a barrier to clear code and the people who see that inconvenience as a significant language feature that forces developers to explicitly account for edge cases. @jrose makes some good points about why this pattern is often the beginning of other troubles, and did a perfect job of characterizing why I often think the semantic of something like unowned is underused in these contexts, but addressing those concerns is really above my proverbial paygrade. So I'll focus on the hypothetical of if this existed.

I think the more important thing here is the question of how this will impact debugging, especially for the beginning Swift developer. In my observation, [weak self] is already a knee-jerk default choice for many developers starting out even when a closure has no chance of causing a retain cycle (something that can cause interesting bugs if an object has to register a certain piece of work was finished, for example), because the safety is reassuring. In that regard, [guard self] would become a habitual behavior for many Swift users virtually overnight, and unfortunately, as currently proposed, it seems to me like this could create a very beginner-hostile situation during debugging.

Probably the most common reason you'd lose self in a closure like this is some kind of race. This means that for most beginning developers, the first time they're not seeing their closure work correctly may be one of the most difficult bugs they've ever had to troubleshoot. Add on top of that the fact that it would not really be possible to breakpoint inside the closure and see that this implicit guard statement is the source of one's problem.

Maybe the solution to this situation is not necessarily a silent failure from a guard, but an assertion? In development settings, this would give you very clear visibility into potential issues as they crop up, and it would indeed be incredibly difficult to ignore that there's now a near-invisible bug in the app, but in production, users wouldn't see the issue. Perhaps an [assert self] or [expect self] might be a way to preserve this functionality while still retaining some kind of visibility into the mechanics of what's going on?

4 Likes

This is an interesting direction. It seems reasonable to have a capture specifier that behaves in this way:

button.tapHandler = { [assert weak self] in
    // ...
}

// is desugared into:
button.tapHandler = { [weak self] in
    guard let self = self else { 
        assertionFailure("Weakly captured variable 'self' no longer exists.")
        return 
    }
    // ...
}

I'm also coming around to @jrose's argument, that we should consider including weak in any spelling for this feature (since guard or assert on their own don't necessarily imply weak, unless you've memorized this).

Some related precedent is how unowned has unowned(safe) (the default) and unowned(unsafe) (here be dragons!) variants -- so stricly following this precedent would give us weak(assert). But I almost feel like unowned(unsafe) is trying to be visually abrasive to dissuade folks from using it :laughing:.

Any thoughts on [assert weak self]?

Alternatively, would [guard weak self] be a suitable way to spell a capture specifier that behaved in this way (a guard with an assertionFailure)?

IMO, hiding an assertionFailure behind a guard keyword is unintuitive. I like assert weak or weak(assert) better.

2 Likes

Thinking about this more, it seems like having [assert weak self] would almost certainly improve safety and correctness over the current status quo.

The status quo today, for many people, is to add a guard let self = self else { return } or use self?.. Both of these widely prominent defaults silently ignore cases where self no longer exists. This is often ok, but is not always ok -- so silently ignoring this case can be harmful.

If [assert weak self] existed, I suspect it would become the status quo default for Void closures, since it has better ergonomics (it avoids some boilerplate and enabled implicit self). This would mean the status quo in this case would no longer be silently ignoring potential failures, which would be unambiguously better with respect to correctness.

2 Likes

Other than differences in the diagnostics (which, granted, is no small thing), what is the difference between this and [unowned self] without the flexibility of choosing where to assert?

"Doesn't crash in production" is a pretty important consideration -- this is the main reason why many folks in the app development community prefer avoiding unowned self.

What do you mean?

1 Like

Nice proposal, +1 for the boilerplate cleanup. I agree, code is read more often than it is written, and removing boilerplate makes it more readable (for me).

What about:

{ [with self, with button] in 
   ...
}

?

1 Like

This is real cool @cal and would certainly make it more convenient to write safe Swift code in a very common use case.

Given that the capture is weak I feel like [guard weak self] is more clear than [guard self]. I also wonder if the “chaining” approach (ie adding guard to an existing weak capture) is more generalizable to other use cases throughout the language :thinking:

1 Like

It’s been a hot second, but isn’t it the case that a runtime trap occurs at the first attempt to use unowned self, such that anything before that in the closure is still executed? By contrast, your proposed assert would always occur at the start of the closure?

I have to agree with others that this is an anti-goal, and a commonly raised one.

I do think sometimes when I and others push back on it by reminding folks that crashing is safe, we make it seem like we want our apps to crash left and right for any trivial scenario. Of course not. It makes lots of sense not to want to crash!

But here’s the trade-off that Swift offers—and consistently so—that guides the user to write tangibly better code: If you don’t want to crash, then you state explicitly the alternative instead of crashing. Either handle the unhappy path yourself or don’t, the choice is yours; the language rules even help out by making it so that you’ll (usually?) have to choose explicitly not to handle the unhappy path rather than just forgetting. The guarantee (and it’s a feature, not a bug) is that, if you don’t, it’ll be handled for you by halting execution.

So, yes, while it’s a big deal and a positive thing for an app not to crash in production, you’ve elided the rest of the motivation here, which is to not crash and also not have to spell out the alternative instead of crashing (i.e., return). But that’s just not Swift’s deal. Nor, I’d venture to say, should it be.

9 Likes

well said.

being somewhere between -1 and +1/2 about the feature itself, i'd risk to assume that the proponents of the feature just consider the short form notation [guard weak self] to be equivalent to some full form which could look like [guard weak self else return] or something similar in case of returning nil or default value. so it could be argued that they are still specifying the unhappy path behaviour, just using a shortcut notation to spell it out. similar to how we can write "expression" instead of "return expression" or $0 instead of explicit parameter name in closures.

i wrote above about related but slightly different possibility: extend "guard let self = self else { ... }" syntax to allow implicit "self" usage afterwards. is it worth exploring?