On 2014-08-13, 8:14 AM, Aryeh Gregor wrote:
On Wed, Aug 13, 2014 at 9:42 AM, Jeff Walden <jwalden+...@mit.edu> wrote:
So at risk of adding yet another flavor of thing: why not introduce an
already_AddRefed<T> sort of struct that *does* own an addref, *will* release on
destruction if not nulled out, and does *not* explicitly convert to T*? Give nsCOMPtr move
constructors and move assignment operators from that class, that null out this other
pointer when called, to transfer ownership at no cost. (At risk of overplaying my hand, I
think I've just described what WebBlink calls PassRefPtr, except for that pertaining to
concrete classes whereas nsCOMPtr<T> is for abstract T classes.)
I recognize this introduces yet another way to do things. But it enables
incremental transition to a completely safe system that prohibits assignment of
a method call to a raw pointer, and yet never has problems with leaks. If we
work hard at enforcing this on the way in, it can quickly become second nature,
I think.
I think this can be improved upon a bit further: just change
already_AddRefed to behave more similarly to nsCOMPtr, but still not
convert to T* implicitly. So for instance:
* Change ~already_AddRefed to just release the pointer instead of
asserting. This means it would be perfectly okay to ignore the result
of a function that returns already_AddRefed.
What goal would this achieve? I don't understand why it's OK to ignore
the return value of a function which returns an "already AddRef'ed"
object from a conceptual perspective.
* Make an already_AddRefed constructor from T* (which would addref).
This allows you to return a raw pointer from a function with
already_AddRefed return type, and it will do the right thing without
you having to define an nsCOMPtr temporary.
already_AddRefed already has a constructor from T* which would not
addref. Essentially this will mean not supporting dont_AddRef() any
more. Why is that a good idea?
* Make an already_AddRefed constructor from nsCOMPtr/nsRefPtr (which
would addref if the source is an lvalue, and steal the value if the
source is an rvalue). This allows you to return a local nsCOMPtr or
nsRefPtr directly, without having to call .forget(). If we want to be
extra careful to avoid accidental addrefs, we could allow construction
only from rvalues, but I'm not sure this is a good idea.
Hmm, so this would allow you to save typing ".forget()" when returning a
COM/ref ptr? I actually think that the .forget() there is super helpful
because it makes it clear that there is an ownership transfer going on.
It's more cumbersome to type, but code is read more often than it's
written and all. :-)
* Allow conversion of already_AddRefed to bool, and comparison to T*.
This I can get behind, but I'm not sure how useful it is without the
other parts of your proposal.
This has most of the benefits of my original proposal, without the
drawback David and Ehsan object to. It has the advantage of not
having to change the return type of a few thousand functions. It has
the disadvantage that you could still not pass an already_AddRefed
directly to a function that wants T*, which is unfortunate because
it's perfectly safe to do so. I can't think of any way to avoid that
except defining new types for refcounted parameters to use instead of
raw pointers (which would have other uses as well).
David, Ehsan, what do you think?
Honestly I'm struggling to decide which problems we _should_ solve here.
I almost always prefer more explicit syntax (for example, having to
type .forget()) when it comes to things such as ownership transfers. It
makes it easier to spot things when reading the code, and it makes the
author more conscious of what they're doing. I also think that you're
essentially trying to eliminate the concept of a helper object that acts
as a pipe of sorts to deliver the information that there is an ownership
transfer happening by making already_AddRefed and nsCOM/RefPtr
indistinguishable. Neither of these seem like improvements to me. I'm
curious to know what others think about this.
Cheers,
Ehsan
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
