On 09/12/2016 11:00 AM, Boris Zbarsky wrote: > On 9/12/16 1:53 PM, Daniel Holbert wrote: >> (I believe we have the "already_AddRefed as a parameter" pattern in our >> code in order to avoid an extra addref/release when ownership is being >> transferred into a function. > > And assertions if the function does not actually take the reference... >> But, RefPtr/nsCOMPtr with move references >> would let us do that just as well > > Not the assertions part, right?
Right -- this sort of code with Move() and a RefPtr<>&& param would be missing those assertions. I don't see that as a *huge* downfall, though. Those assertions have a few benefits for already_AddRefed: (1) They guard against leaks from unused already_AddRefed. (2) They guard against logic errors where we should've taken the value but forgot to do so. (And it enforces that callees must take the value, even if they have some error handling cases where they'd prefer not to.) So I think (1) is irrelevant here, because with Move() the object will always be stored in a refcounted pointer of some sort. I think (2) is semi-relevant. It brings up one potential concern with Move: since the callee might not take the value (intentionally or unintentionally), the caller must *refrain* from caring about the state of its Move()'d variable, between the Move() operation and any reassignment/cleanup. (This seems like something that static analysis could check for us & could guarantee is a non-issue. And it seems like a standard part of the Move bargain that reviewers should know to watch for, as soon as they're familiar with Move.) I guess my bottom-line question is: which pattern should I prefer, for new code that wants to trivially transfer ownership of a refcounted object into a function or a constructor? (In my current case, I'm looking at a constructor much like the nsDOMMutationObserver example that I linked to, with a parameter that gets directly assigned in an init list -- so, there's no actual risk of logic preventing it from getting taken.) I'm OK with continuing to prefer already_AddRefed, if I should; though I was initially hoping that this is one spot where we could get rid of it (with the upsides being brevity, consistency in type-of-smart-pointer, and fewer mozilla-specific quirks). Thanks, ~Daniel _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
