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

03 янв. 2014 г., в 4:12, Antti Koivisto <[email protected]> написал(а):

> 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

_______________________________________________
webkit-dev mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to