Reviews are an important part of the Swift evolution process. All review feedback should either be on this forum thread or, if you would like to keep your feedback private, directly to the review manager by DM. When messaging the review manager directly, please keep the proposal link at the top of the message and the evolution identifier in the subject line.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
Strongly agreed, as written.
Its intention is very clear, and robust.
Is the problem being addressed significant enough to warrant a change to Swift?
Yes. It makes codes much cleaner.
Moreover it is not just a yet another sugar if I understand correctly.
It solves a common pitfall* at last, by cleverly following SE0269.
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()
}
}
If you follow the rule – capture self as weak then unwrap and use it implicitly – compiler assists you to determine if self has to be captured again in inner closures.
This is great and definitely worth a +1, though I personally would love to see it go a step further and add some way of eliding the guard let self else { return } line, which is almost always used when capturing weak self.
Not sure what it might look like exactly, maybe [self?], but the idea would be it would automatically return void if self is nil. I guess would only work for closures that return void, but I think that generally is the case.
Let's keep this discussion focused on reviewing the proposal at hand; there is an extensive archive regarding the design space for unwrapping self, but it is not the topic of this proposal.
Personally I feel self in general should not be elided, point of ensuring there are no implicit reference cycles is one of the ones, but for me it is for readability overall over the cost of a few key presses.
Better for autocompletion (finding the method you want, no global function to list even if you start with self.), better for readability in Pull Requests outside of the editor too (one of the reason argument labels are so useful, if we were always stuck on our IDE’s with smart suggestions/overlays we would need labels in code less as the IDE would remind us what they are supposed to be which is maybe one of the reasons why Kotlin did not make them mandatory for new code).
In the ideal world everything is decomposed to small objects, few vars, and all… but in a lot of code I see the “is this a local var or an instance var” is a question that would not even be there with self, but . There was a Polychromatic extension for Xcode in the Alcatraz days that would color the vars that we’re in the same scope / related… it helped me sooo soo much in having all kinds of code more readable.
Implicit self is all rage in Swift community and I can see how this can implode into the faces of many engineers mistakenly referencing a shadowed variable. It already implodes elsewhere anyway right? That's said I am all in on this suggestion. The code is nice without explicit self.
I have two pieces of feedback on the proposal itself. The examples in the proposal use guard let exclusively although the proposal title says it is for any unwrapped self.
Possibly add at least one example to the proposal that shows this with if let and possibly a sentence that explicitly mentions all of the unwrapping mechanisms this will be used with.
I mention it because the evolution proposal is often the most detailed documentation available for any given language feature.
Also, I think there's a typo in the Detailed design section:
Following the precedent of SE-0269, additional closures nested inside the [weak self] closure most capture self explicitly
I tend to agree. Too often 'less code' is confused with 'clean code', less doesn't always mean clean. In fact being more explicit is often cleaner IMO. This strikes me as yet another proposal geared towards attempting to improve code writability, rather than readability.
I think it’s reasonable to dislike implicit access to self. (I may not agree with you, but it’s an understandable argument.) However, Swift does allow these accesses in many situations today, situations much more prominent than those in the proposal. Given that, I think the argument that implicit accesses are bad in general does not justify restricting them only in these situations; the remedy does not suit the diagnosis. You can argue that implicit accesses should be removed in general; you are unlikely to prevail, but you can argue it. Otherwise, I think you need to find a more specific argument why they’re especially bad in the situations in the proposal and therefore we should keep the current rule.
One of my earliest memories writing Swift was some frustration over moving code with implicit self into closures and immediately having to go through the lines and prefix all my lines with self.. self being completely required inside closures was a double-edged sword in this way, and it seems like this change really will smooth out that sharp edge.
All the other comments about whether or not eliding self should be allowed really seems like a code style question to be chosen by teams and enforced by formatters.
You never had to: you could have put self in the capture list and have the code unchanged.
+1 on the pitch, it is very natural and allows the same convenience for weak self capture as we had for strong self capture.
Is only the short quard let self else ... form supported for this feature or guard let self = self else ... supported as well? No strong opinion, if it is simpler to support only the first form it's fine (and will also motivate users towards using the short form.
All forms of optional unwrapping would be supported, including if let self. The only requirement in the implementation is that the type of self is non-optional.