[webkit-dev] When to use "auto"? (I usually consider it harmful)

[Alexey Proskuryakov]

I'm not Geoff, yet I'd like to offer my perspective on the specific examples you provided.

03 янв. 2014 г., в 4:12, Antti Koivisto <koivisto at iki.fi> написал(а):

> auto& cell = toRenderTableCell(*renderer); // right
> RenderTableCell& cell = toRenderTableCell(*renderer);  // wrong

I can't agree with this example. With auto, I don't know if it's a raw pointer, a reference, or a smart pointer (we have toXXX functions that return all of those). I also cannot Cmd+click on anything to go to the class definition, and when I Cmd-click on toRenderTableCell, I presumably get into some unreadable macro.

Overall, auto makes the code very opaque here.

> for (auto& source : descendantsOfType<HTMLSourceElement>(*this)) // right
> for (const HTMLSourceElement& source : descendantsOfType<HTMLSourceElement>(*this)) // wrong

I think that this is borderline OK, and may even be allowed by the latest proposal, although the win is so small that maybe we shouldn't? I don't really see how the first line is better than the second one in any way.

> auto properties = std::make_unique<PropertiesVector>();  //right
> std::unique_ptr<PropertiesVector> properties = std::make_unique<PropertiesVector>(); //wrong

Agreed. I think that this is also allowed by Geoff's proposal.

> This rule is already widely deployed and I think the code readability has improved.
> 
> - Use "auto" when the type is irrelevant. This covers things like iterators and adapter classes:
> 
> auto sourceDescendants = descendantsOfType<HTMLSourceElement >(*this));

So do we need auto or auto& here? I would know how to answer this question immediately if it was an explicit type, but I don't know how to answer it with auto in this particular case.

> It might also cover smart pointer usage like your example and multiline expansions of setSize(foo->size()).
> 
> - Use "auto" when type is obvious for people with basic familiarity with a subsystem:
> 
> auto& style = renderer.style();

I don't agree that "basic familiarly with subsystem" is a good criterion. One shouldn't need even basic familiarity with style system to see whether code leaks, introduces refcount thrashing, or copies too much. Using auto tends to make such mistakes much more opaque.

Raising artificial barriers between subsystems would be really unfortunate, as people tend to work on bugs in many areas.

> Thoughts?

The latest set of rules proposed by Geoff looks pretty good to me. It would be great to make it a formal requirement. We can always add to it later if things get annoying.

- WBR, Alexey Proskuryakov

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-dev/attachments/20140103/feecab70/attachment.html>