On Sun, Aug 2, 2015 at 3:47 AM, Xidorn Quan <quanxunz...@gmail.com> 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.
We used to have NS_NewFoo() objects all over the codebase. A lot of those were removed as part of deCOMtamination efforts. Personally I think being able to directly call the constructor has made for much easier to read code. Definitely worth the risk of refcount errors. I wonder if we could do static analysis to check of the result of "new X", where X has an AddRef member function, is assigned into a raw pointer. / Jonas _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
