SE-0365: Allow implicit self for weak self captures, after self is unwrapped

+1. This seems like a natural extension to the current behavior.


This has only been possible since Swift 5.3 with the SE-0269 proposal.

2 Likes

+1 woohoo! This looks great

1 Like

-1.
Closures are the places where self should better be referenced explicitly for clarity. In following example its's impossible to understand without context is perform a global or a local function

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

If this proposal would block self-rebinding for instances other than a captured self reference, then I would be very much against it (-1). Not needing to write self in some places is great, but it shouldn‘t be limited to only the captured outer self. This way, it would be a wasted opportunity.

Related discussion:

4 Likes

If we had a with global function for binding self to an arbitrary value, it could have a much-desired overload for weakly capturing an AnyObject and transforming the return value into an optional.

Agreed.

-1

While the proposal itself is well versed,
I join the folks who like their selfs explicit.

As was pointed out above, this is about allowing implicit self for this specific case. If you don't like implicit self in general, that's a separate conversation.

6 Likes

As the author of SE-0269 I'm very excited to see this come to review! It feels like a natural extension to me.

Funnily enough, I actually join the folks who lean more towards explicit self in general, but in codebases that use implicit self I find it better if implicit self can be used consistently so that it's easier for my brain to fill in the self across the board, rather than having explicit self in some spots and implicit self in others.

7 Likes

Note that "explicit self" capturing can be done two ways (see ***):

option 1:

queue.async {
    self.foo() // ***
    self.bar() // ***
}

option 2:

queue.async { [self] in // ***
    foo()
    bar()
}

One is not necessarily better than the other. Unless instead of one/two properties you are using a dozen, at which point the noise of "self." in option 1 starts to be significant.

This pitch just generalises option 2 to the common case of weak self:

queue.async { [weak self] in
    quard let self else { return }
    foo()
    bar()
}

You don't have to use this feature if you don't like it. Prefixing with "self." is always an option, and you can even rename it:

queue.async { [weak self] in
    quard let this = self else { return }
    this.foo()
    this.bar()
}

(IIRC, in some previous swift versions I had to use this instead of "self").

4 Likes

Sure,

Let the folks who want to have it have it, and other keep using the old way, except, I would not want to be in the situation to review code like this:

class A {
    func  notOnSelf() {
        
    }

    func b() {
        func onSelf() {
            
        }
        
        let d = { [weak self] in
            guard let self else { return }
            onSelf()
            notOnSelf()
        }
    }
}

As others have pointed out it is mental arithmetic to keep track, and I would find myself inserting self.

1 Like

By the same reasoning you may want to use "self." everywhere where it's possible (not many people doing this; to be fair not many people doing the opposite either). Then prepend vars with "v", constants with "k" and methods with "f" (otherwise how could you tell one from another), etc... SwiftLint custom rules may be of help for you and your team.

I do not feel this change would be good for the Swift's expressibility and clarity. While it is might be a bit uncomfortable to handle this weak references with each escaping closure, explicit self provides important information either for author or the reader of the code. This language behaviour, from my experience, also encourages better structure of the code and extracting smaller methods, since it is easier to handle a closure with one line of method call, improving overall readability of the code.

The given examples with the single dismiss method inside closure, as for me, do not play on the proposal's side, the code of the closures from the proposal examples does not requires unwrapping self at all (this and the following examples of code are aligned with the current behaviour):

button.tapHandler = { [weak self] in
    self?.dismiss()
}

And the examples in the proposal just complicate that simple code. As for the scenarios, when we have, say, 10 lines of code inside closure and each referencing self, then we have a great candidate for a separated method, and this returns us to the example above.

The suggested behaviour also increases complexity of the code in case of nested closures. Currently, thanks to the current guard behaviour with the ability to shadow self in one line, we receive unwrapped self an any level of closures nesting:

button.tapHandler = { [weak self] in
    guard let self = self else { return }
    self.showAlert()
    execute {
        self.dismiss()
    }
}

I do not see any beneficials in the examples from the proposal, compared to the code in the block above.

1 Like

I get the arguments against this but it seems that many of those arguments would also apply to SE-0269. And the ship has already sailed on that proposal. For consistency's sake and because it is a bit more ergonomic, it's a +1 for me.

1 Like

Can you clarify what you mean about this proposal increasing complexity in this example?

As the proposal states, each nested closure will require explicit usage of self:

button.tapHandler = { [weak self] in
    guard let self else { return }

    execute {
        // error: call to method 'method' in closure requires 
        // explicit use of 'self' to make capture semantics explicit
        dismiss()
    }
}

(example from the proposal)

Right now, if you unwrapped the self once, you have consistency at any level of nesting — the self is explicit everywhere (second example in my previous post), while with the proposal suggestions, it will be implicit at the first level and explicit at the followings. So I would prefer a more straightforward flow in such cases.

If I'm not mistaken, the very error message is a nice side-effect of this proposal, which gives us an opportunity to think about retain cycle (memory leak).

Although I guess it depends on its context but this would be a potential memory leak if execute is escaping:

button.tapHandler = { [weak self] in
    guard let self else { return }

    execute {
        self.dismiss()
    }
}
1 Like

There is still consistency. In any scope in which a weak self has been unwrapped, it will be unnecessary to explicitly prefix members of self with self.. Naturally this is different than the status quo. The difference is the entire point of the proposal.

In other words, do you mean that each nested closure has to explicitly specify [weak self]? As I understood the proposal, the [weak self] behaviour will remain unchanged, so each nested closure still has a weak self reference once it is captured for the first time:

// current behaviour
button.tapHandler = { [weak self] in
    execute {
        // self is weak since we captured 
        // a weak reference in the parent closure
        self?.dismiss()
    }
}

In the case of weak self remains as it is, then there is a question of how to unwrap it in the nested closures? Will the guard let self else { return } statement limited to concrete closure only, changing this option too?

// assumed behaviour
button.tapHandler = { [weak self] in
    guard let self else { return }
    dismiss()
    execute {
        // cannot use guard here to unwrap self, 
        // since self has been already unwrapped.
    }
}

Or, if the weak will be forced to be explicitly captured with the each nested closure, to unwrap self it would be required to write guard statement every time?

// assumed behaviour with the modified weak self capturing
button.tapHandler = { [weak self] in
    guard let self else { return }
    // explicitly capture weak reference to self in the nested closure
    execute { [weak self] in 
        // and now we can use another guard to unwrap
        guard let self else { return }
        dismiss() // or self?.dismiss() without guard
    }
}

The last one seems a bit unnatural, since the guard usually takes the effect to all scopes following it.

The reason we currently need to use explicit self in closures is to reveal potential retain cycles. This proposal keeps that safety by requiring explicit self at least once per closure. I find myself scanning for self to debug retain cycles so I appreciate this aspect of the proposal.