Closed Bug 776358 Opened 7 years ago Closed 4 years ago

RefPtr should implement an already_AddRefed() constructor


(Core :: MFBT, defect)

Not set





(Reporter: ekr, Unassigned)


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.
OS: Mac OS X → All
Hardware: x86 → All
I don't think it's a good idea to encourage mixing MFBT RefPtr and XPCOM smart pointers.
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.
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...)

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.
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1121489
You need to log in before you can comment on or make changes to this bug.