[webkit-dev] usage of auto

[Darin Adler]

We probably need to step away from mandating style for a while until we have more consensus.

I’m sad that we are so far away from that right now. I’ve found greatly increased use of auto during coding and refactoring that I am doing feels like it’s improving clarity quite at bit. From the point of view of project norms I’m a little disappointed that so many people see it so dramatically differently!

> On Jan 12, 2017, at 9:24 AM, Filip Pizlo <fpizlo at apple.com> wrote:
> 
>> On Jan 12, 2017, at 08:54, Brady Eidson <beidson at apple.com <mailto:beidson at apple.com>> wrote:
>> 
>>> My take-away from this discussion so far is that there is actually very little consensus on usage of auto, which means there’s probably very little room for actual style guideline rules.
>>> 
>>> I think there are two very limited rules that are probably not objectionable to anybody.
>>> 
>>> 1 - If you are using auto for a raw pointer type, you should use auto*
>>> 2 - If you are using auto in a range-based for loop for values that aren’t pointers, you should use (const) auto&
> 
> In some cases you need a copy for the code to be correct. I understand why & is often better for performance but there is a significant and dangerous behavioral difference.
> 
> I agree with encouraging people to use auto& because it's usually ok, but I disagree with mandating it because it's sometimes wrong. 


Lets talk about that specific point for a moment. Consider these two cases:

1) Iterating over a collection that is not being modified and operating on the objects inside the collection in a straightforward way. This idiom is incredibly common. Iterating over an entire collection is a pattern that occurs constantly in WebKit code, and we lean more toward the for loop than using lambdas for that kind of operation. In these cases, copying the items as we iterate is not valuable, often bad for efficiency, and a very easy mistake to make because of how auto works. I have fixed at least 20 examples, maybe as many as 100, of copying a RefPtr by using a for loop and so churning the reference count for no benefit. Using “auto” creates this problem in the first place, since it makes the copy of the RefPtr subtle, missed by many programmers. And “auto&” is a pretty good solution, I think better than writing RefPtr<T>&. To me “auto&” is the way to say “operate on the members of this collection in place”, without the distraction of stating anything else about the iteration, such as the type of the iterator, for example, which you would see if the begin/end was written out without the use of a modern for loop or auto. The biggest downside of “auto&” as a solution is that the incorrect “auto” and the correct “auto&” are so subtly different that it’s easy to fail to spot incorrect code. The future version of C++ where you can do a non-copying for loop omitting any mention of the type will be even better, because “no type at all” is not easily confusable with the incorrect “auto”. In that new feature, the type is “auto&&” where not specified. I look forward to when we can use that feature.

2) The occasional for loops where copying each item in the collection is part of what we want to do. There are multiple options here. We could name the type in the for loop and count on people’s understanding that means copying. Or if we want to be more explicit, we could have the for loop use “auto&” and write the copying out explicitly as a separate statement. That would also work for moving. Less terse than doing the copying as a side effect of how the for loop itself is written, but perhaps much clearer because of how explicit it is. Since this is a tiny fraction of for loops I haven’t had to consider this style question often.

I personally don’t like using auto& when iterating a collection that contains a scalar such as an int or bool with a short name. But there is a change using auto& when iterating a collection containing a complex type that we want to copy might be a good style idea. Maybe even a good enough idea that  we some day would want to mandate it!

— Darin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-dev/attachments/20170112/1b9e6d17/attachment.html>