<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Jan 2, 2014, at 1:12 PM, Geoffrey Garen <<a href="mailto:ggaren@apple.com">ggaren@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">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><blockquote type="cite"><<a href="http://trac.webkit.org/changeset/161143">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></blockquote><br>I think this use of “auto” is net harmful.<span class="Apple-converted-space"> </span><br><br>Upsides:<br><br>- Less typing<br><br>Downsides:<br><br>- I don’t know the type of ’newRenderer’ at a glance<br>- Navigating to newRenderer’s class definition is a two-step process: (1) Go to the definition of createTextRenderer and see what it returns; (2) Go to the definition of (1).<br><br>I think the downsides outweigh the upsides here because reading code is more common than writing code, and because it’s not *that* much typing to write out "RenderPtr< RenderText>”. In this particular code, I think it’s especially bad to disguise the type of a pointer in the render tree, since we’re in the middle of a transition to a new type of pointer, and so it’s important to know if you’re looking at code that does the new thing or the old thing.<br></div></blockquote><div dir="auto"><br></div><div dir="auto">I agree.</div><div dir="auto"><br></div><div dir="auto">Code is read more than it is written. In many cases, the types of variables - even nasty templated ones - convey useful information that makes the code easier to read. I think that "auto" is only a good idea if the presence of the full type would be either completely redundant or would make the code hard to read by virtue of being unusually long (>80 chars, like some intense HashMap instantiations). Crucially, while the C++ compiler has a smart type inference engine that can see a lot of code all at once, a typical human reader lacks such powers and so "auto" results in a net loss of information.</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>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</div></blockquote><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">- In all other cases, use an explicit type declaration</div></blockquote><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>Thoughts?<br></div></blockquote><div><br></div><div>I like this. I would like to see this be added to the style guide.</div><div><br></div><div>-Filip</div><div><br></div><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><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">https://lists.webkit.org/mailman/listinfo/webkit-dev</a></div></blockquote></div><br></body></html>