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

[Filip Pizlo]

On Jan 2, 2014, at 1:59 PM, Brent Fulgham <bfulgham at apple.com> wrote:

> Hi,
> 
> On Jan 2, 2014, at 1:12 PM, Geoffrey Garen <ggaren at apple.com> wrote:
> 
>>> +    auto newRenderer = textNode.createTextRenderer(style);
>>> +    ASSERT(newRenderer);
>>> 
>>> ….
>>> 
>>> +    parentRenderer->addChild(newRenderer.leakPtr(), nextRenderer);
>> 
>> I think this use of “auto” is net harmful. 
> 
> I disagree. I think the use of auto is an improvement, because it makes it less likely that we have something like the following:
> 
> int wrong = something.get64BitInt(); // potential truncation
> 
> There are plenty of cases where we change the signature of functions from one type to another, sometimes promoting sizes. I haven’t seen us do a very consistent job of combing through sources to make sure all receiving variables are properly sized.
> 
> When we don’t use ‘auto’, we run the risk of assigning to variables that the compiler “makes work” by casting. We only know this is happening when the compiler generates a warning (and we look for that warning).
> 
>> 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.
> 
> Don’t we have this same problem with all of our JavaScript, Python, and Ruby code? We don’t have type specifications in those languages, but we manage to build large software with them as well.

I can comment about the Ruby code.  It's often hard to make sense of that code because you don't have variable types to give you hints about what you're looking at.  For example, this treasure, from display-profiler-output:

                compilation.descriptions.each {
                    | description |
                    if description.origin.index {
                            | myBytecode |
                            bytecodes == myBytecode.bytecodes
                        }
                        myOrigins << description.origin
                    end
                }

It's true that "compilation" points to a Compilation.  But what does descriptions point to?  It turns out that it's an array of CompiledBytecode objects.  This is a relevant piece of information because it tells you what kinds of information each element in the array contains.  You can figure this out if you search for all of the places that the descriptions array gets populated.  But, there is no way to see this from looking at the declaration of "compilation" or "descriptions".  In the C++ code we would write prior to C++11, we would have likely done this as follows:

for (unsigned i = 0; i < compilation->descriptions().size(); ++i) {
    CompiledBytecode& description = compilation->descriptions()[i];
    ... // more stuff
}

I find this kind of thing easier to understand, when I have to return to such code after possibly a year, or more, of looking at it.  It's code like this that makes me think that 'auto'-everywhere is net harmful.

-Filip


> 
> -Brent
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

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