On Mon, Aug 15, 2016 at 3:55 PM, Aryeh Gregor <[email protected]> wrote:
> IMO, it makes sense to move ahead with this.
One more point I forgot to (re-?)mention: with proper move
constructors, it's not just already_AddRefed<T> return values that
should be changed to RefPtr<T>, but also T* return values, in the case
where the returning function already had a RefPtr<T> lying around.
This will *reduce* addref/release in common cases, like:
T*
Foo()
{
RefPtr<T> t = DoStuff();
return t;
}
RefPtr<T> t = Foo();
Currently this does a release when Foo() returns, and a new addref
when assigning to the new variable. If Foo() instead returned
RefPtr<T> with proper move constructors, these would be removed. You
also could not assign the return value of Foo() to a raw pointer
(without .get()), which improves security against use-after-free
without any extra addref/release. (The release is just handled by the
caller instead of callee.)
It could inadvertently introduce extra addref/release if a function
that doesn't hold a reference on some code paths has its return type
changed to RefPtr, like:
T* Bar();
T*
Foo()
{
return Bar();
}
Foo()->Method();
If Foo()'s return type was changed to RefPtr<T>, this would silently
add an addref/release. I don't know how to prevent this. Would
making the relevant constructor explicit do it? If so, would it cause
issues in other places?
Also -- now I'm remembering more of the issues here -- this would also
require .get() to pass a function's return value directly as a
function parameter, unless bug 1194215 is fixed (see also bug
1194195). It's not *so* common to do this, but it's common enough
that we don't want to require .get(), I think.
So this isn't such a no-brainer, but I think it is where we ideally want to be.
_______________________________________________
dev-platform mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-platform
