On 08/02/2015 01:47 PM, Xidorn Quan wrote:
On Sun, Aug 2, 2015 at 7:57 PM, Kyle Huey <m...@kylehuey.com> wrote:
On Sun, Aug 2, 2015 at 2:56 AM, Hubert Figuière <h...@mozilla.com> wrote:
On 02/08/15 04:55 AM, smaug wrote:
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.

Shouldn't we, instead, ensure that SomeRefCountedFoo() returns a nsRefPtr?

How do you do that with a constructor?

Probably we should generally avoid using constructor directly for
those cases. Instead, use helper functions like MakeUnique() or
MakeAndAddRef(), which is much safer.

MakeAndAddRef would have the same problem as MakeUnique. Doesn't really tell 
what type is returned.
And when you're dealing with lifetime management issues, you really want to 
know what kind of type you're playing with.


I would just limit use of 'auto' to those cases which are safe for certain, 
given that
it helps with readability in rather rare cases (this is of course very 
subjective).
So, *_cast<> and certain forms of iteration, but only in cases when iteration 
is known to not call
any may-change-the-view-of-the-world methods - so no calling to JS, or flushing 
layout or dispatching dom events or...


-Olli
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to