[Review] SE-0009: Require self for accessing instance members


(Brent Royal-Gordon) #1

(Note: though I use the term “variable” often, in most places this term should be read broadly to encompass constants, properties, functions, methods, etc.)

What is your evaluation of the proposal?

···

-----------------------------------------------------

I am not a fan.

I actually recently found and fixed a bug that this proposal would have avoided. I had a morass of related functions which formed a sort of pipeline that recursively fetched data from the network and created matching objects. When this process grew too large and complex, I moved it into its own type, and changed some of its local variables into properties. Inevitably, I forgot to delete a crucial local variable, so the resulting objects weren’t being put in the property, and my code was breaking. Because of the complexity of the surrounding code, chasing this problem down took perhaps fifteen minutes.

But you know what? It wasn’t actually that hard to fix. Once I realized the bug was being caused by that property being empty, it took seconds to find the site where it was supposed to be filled in, scan up the method, and notice the declaration I’d forgotten to delete. And in a year and a half with Swift, this was the first time I’d made a mistake like this. It was really no big deal, and on balance, I’m happy to trade that debugging time for the less-cluttered code that implicit self lets me write.

Finally, it’s worth keeping in mind that this proposal still doesn’t make all identifier references unambiguous. A variable could still refer to a local variable, a variable closed over from a surrounding scope, a module-global variable, or a variable imported from another module. A type name could refer to a concrete type or a generic type parameter at any level of nesting, all the way to module-global and imported types. Certainly implicit-self is an especially common sort of scoping confusion, but if you really want to thoroughly solve this problem, it’s merely the tip of the iceberg.

Is the problem being addressed significant enough to warrant a change to Swift?
-----------------------------------------------------------------------------------------------------------

I think the problem warrants changes in tooling. I do not, however, think *this* is the change that needs to be made.

What I think we should do is:

- Warn when mutating a variable which shadows a property or global, or calling a function or method which shadows a method or global. These seem by far the most common potential mistakes.

- Consider introducing other shadowing warnings as well. I would not warn on *all* shadowing; `if let foo = foo`, initializer parameter names, and capture lists seem particularly common and likely to be correct. But many, and perhaps even most, instances of shadowing are incorrect, and I think that over time we can refine our warnings to avoid crying wolf.

- Change editor features to make it easier to identify properties. For example, Option-Clicking a variable in Xcode should tell you whether it’s local to the function, a property of the type, or a global. This may not strictly be in scope for swift-evolution, but it’s important to think about the toolchain as a whole.

- Make sure that linters using our parser can easily detect uses of implicit `self`. If the Swift project eventually writes its own linters or code-quality tools, this should definitely be an option in them.

Does this proposal fit well with the feel and direction of Swift?
---------------------------------------------------------------------------------

In some ways, it does. Clarity over brevity is an important Swift principle, and this proposal certainly does trade brevity for clarity.

But “clarity over brevity” only works up to a point. Look at implicitly-unwrapped optionals: clearly, regular optionals are clearer, and yet IUOs persist. Why?

Because there’s an underlying principle beneath “clarity over brevity”: Swift should make programming less painful. “Clarity over brevity” is a good rule of thumb because clarity often makes reading and debugging less painful. But beyond a certain point, you’re not getting much extra clarity, while the lack of brevity is inflicting pain. At that point, clarity needs to give way.

I think forcing `self.` on thousands of perfectly correct method calls and property accesses to prevent one or two from going to the wrong place is over that line. There are steps we can take which will help almost as much and be far less painful. I can’t think of an example of this causing a bug which doesn’t include some other code smell, so let’s go after those smells instead.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
-----------------------------------------------------------------------------------------------------------------------------------------------------------------

I have extensively used both languages which required `self` or an equivalent on all member references (Objective-C, Perl 5 and 6, Javascript) and languages which did not (Ruby, C#, a little bit of Java). With this proposal, Swift is similar to languages in the former category; without it, it’s similar to those in the latter.

In those languages where implicit `self` is forbidden, I find it rather annoying to be forced to write it. Objective-C and Perl 5 permit some form of cheating (direct ivar access and calling methods as subroutines); I don’t do this because in both cases it sacrifices correctness, but I’m always tempted to.

In those languages where implicit `self` is allowed, I use and enjoy this feature regularly. The only language in which it regularly causes me problems is Ruby, where local variables are declared by simply assigning them, and so `foo =` is always treated as declaring a new local variable even when `self.foo=` exists. This problem does not exist in Swift.

In short, my experience with other languages indicates that implicit self is not a serious problem and that the lack of it is a constant irritant.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
-----------------------------------------------------------------------------------------------------------------------------

I have followed the discussions on the list and given it some thought. I’ve also recently done some debugging of a related bug in some of my own Swift code. This is the result of more than a quick reading, but I wouldn’t say it’s an in-depth study.

--
Brent Royal-Gordon
Architechies