A new type of error 'auto' seems to cause, now seen on m-i, is
auto foo = new SomeRefCountedFoo();
That hides that foo is a raw pointer but we should be using nsRefPtr.
So please, consider again when about to use auto. It usually doesn't make the
code easier to read,
and it occasionally just leads to errors. In this case it clearly made the code
harder to read so that
whoever reviewed that patch didn't catch the issue.
(So far the only safe case I've seen is using 'auto' on left side when doing
*_cast<> on the right side.
And personally I think even in that case auto doesn't help with readability,
but I can live with use of auto there.)
-Olli
On 06/02/2015 10:58 PM, smaug wrote:
Hi all,
there was some discussion in #developers about use of 'auto' earlier today.
Some people seem to like it, and some, like I, don't.
The reasons why I try to avoid using it and usually ask to replace it with the
actual type when I'm
reviewing a patch using it are:
- It makes the code harder to read
* one needs to explicitly check what kind of type is assigned to the variable
to see how the variable is supposed to be used. Very important for example
when dealing with refcounted objects, and even more important when dealing
with raw pointers.
- It makes the code possibly error prone if the type is later changed.
* Say, you have a method nsRefPtr<Foo> Foo(); (I know, silly example, but
you get the point)
Now auto foo = Foo(); makes sure foo is kept alive.
But then someone decides to change the return value to Foo*.
Everything still compiles just fine, but use of foo becomes risky
and may lead to UAF.
Perhaps my mind is too much on reviewer's side, and less on the code writer's.
So, I'd like to understand why people think 'auto' is a good thing to use.
(bz mentioned it having some use inside bindings' codegenerator, and sure, I
can see that being rather
valid case.)
-Olli
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
