<div dir="ltr">On Thu, Jan 2, 2014 at 11:12 PM, Geoffrey Garen <span dir="ltr"><<a href="mailto:ggaren@apple.com" target="_blank">ggaren@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi folks.<br>
<br>
Recently, I’ve seen patches go by using the C++11 “auto” keyword. For example, let me pick on Andreas:<br>
<br>
> <<a href="http://trac.webkit.org/changeset/161143" target="_blank">http://trac.webkit.org/changeset/161143</a>><br>
><br>
> + auto newRenderer = textNode.createTextRenderer(style);<br>
> + ASSERT(newRenderer);<br>
><br>
> ….<br>
><br>
> + parentRenderer->addChild(newRenderer.leakPtr(), nextRenderer);<br>
<br>
I think this use of “auto” is net harmful.<br></blockquote><div> </div><div><div>I agree that in a case where a smart pointer of unclear type (when the new renderer smart pointers are deployed everywhere the situation might be different) is returned it might be better not to use "auto". </div>
<div><br></div><div>However I also feel the "harm" here is debatable enough that people working on/reviewing the code should make decisions instead of there being a project level dictate. It is a new thing, the appropriate usage hasn't yet settled and people should be allowed to experiment.</div>
</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">I think an appropriate style guideline for “auto” would say something like:<br>
<br>
- Use “auto" to declare a disgusting templated iterator type in a loop<br>
- Use “auto… ->" to define a template-dependent return type in a class template<br>
- In all other cases, use an explicit type declaration<br></blockquote><div><br></div><div>I think this is a too limiting set of rules. I have found that in many cases "auto" improves code readability by having less gunk obscuring the logic. It has also made refactoring easier.</div>
<div><br></div><div>I'm not sure how much rules we really need beyond "use 'auto' when it improves code". I gather from this thread that especially people working on JSC are not comfortable using it so they shouldn't.</div>
<div><br></div><div>If we start making rules I'd add the following:<br></div><div><br></div><div>- Use "auto" when the type name is already mentioned on a line:</div><div><br></div><div>auto& cell = toRenderTableCell(*renderer); // right</div>
<div>RenderTableCell& cell = toRenderTableCell(*renderer); // wrong</div><div><br></div><div>for (auto& source : descendantsOfType<HTMLSourceElement>(*this)) // right</div><div>for (const HTMLSourceElement& source : descendantsOfType<HTMLSourceElement>(*this)) // wrong<br>
</div><div><br></div><div>auto properties = std::make_unique<PropertiesVector>(); //right</div><div>std::unique_ptr<PropertiesVector> properties = std::make_unique<PropertiesVector>(); //wrong</div><div>
</div><div>This rule is already widely deployed and I think the code readability has improved.</div><div><br></div><div>- Use "auto" when the type is irrelevant. This covers things like iterators and adapter classes:</div>
<div><br></div><div>auto sourceDescendants = descendantsOfType<HTMLSourceElement >(*this));</div><div><br></div><div>It might also cover smart pointer usage like your example and multiline expansions of setSize(foo->size()).</div>
<div><br></div><div>- Use "auto" when type is obvious for people with basic familiarity with a subsystem:</div><div><br></div><div>auto& style = renderer.style();</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Thoughts?</blockquote><div><br></div><div><br></div><div> antti</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Thanks,<br>
Geoff<br>
_______________________________________________<br>
webkit-dev mailing list<br>
<a href="mailto:webkit-dev@lists.webkit.org">webkit-dev@lists.webkit.org</a><br>
<a href="https://lists.webkit.org/mailman/listinfo/webkit-dev" target="_blank">https://lists.webkit.org/mailman/listinfo/webkit-dev</a><br>
</blockquote></div><br></div></div>