776358 - RefPtr should implement an already_AddRefed() constructor

Status: RESOLVED DUPLICATE of bug 1121489

(Core :: MFBT, defect)

Description

[Eric Rescorla (:ekr)]

Note that nsRefPtr has this:


      template <typename I>
      nsRefPtr( const already_AddRefed<I>& aSmartPtr )
            : mRawPtr(aSmartPtr.mRawPtr)
          // construct from |dont_AddRef(expr)|
        {
        }


But RefPtr does not appear to.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I don't think it's a good idea to encourage mixing MFBT RefPtr and XPCOM smart pointers.

Comment 2

[Eric Rescorla (:ekr)]

Well, if that's not desirable then they shouldn't use the same reference increment/decrement operators.

Since they do, I think the ship has already sailed on that.

Comment 3

[Jeff Walden [:Waldo]]

Eric, could you explain your first paragraph of comment 2?  I don't understand it.  (I'm inclined to agree with comment 1 right now, tho.  Which doesn't mean maybe we shouldn't have something like already-addrefed in mfbt...)

Comment 4

[Eric Rescorla (:ekr)]

Sure.

My general principle here is that if things are invariants, then they should be enforced by the compiler where possible. Hence, if we don't want people mixing MFBT RefPtr and XPCOM smart pointers, then RefPtr<T> shouldn't use AddRef() and Release(), but rather some other functions, even if they do the same thing as AddRef() and Release(). E.g., RefPtrAddRef(), RefPtrRelease(). That way the compiler would stop you from using the MFBT and XPCOM smart pointers on the same objects (or at least it would be a lot harder. I could imagine further template trickery to make it effectively impossible). It's the fact that the different varieties of ref-counted pointers use the same interface that encourages mixing, not the existence/non-existence of already_AddRefed.

Conversely, if the interfaces are to be compatible, then each of the ref-counted pointer classes should be complete, which in this case means having an already_AddRefed constructor variant.