On Tue, Aug 12, 2014 at 6:14 PM, L. David Baron <[email protected]> wrote: > Bug 967364 did already add assertions to guarantee this.
Yes, if I'm not mistaken, already_AddRefed these days is quite safe. > Two thoughts: > > (1) I think it introduces a somewhat major coding style risk, in > that it makes it possible to accidentally assign the value of a > function that returns a reference into a raw pointer, which isn't > possible with already_AddRefed. In other words, today, you can > write: > T* t = GetT() > and know that as long as the code is following conventions > correctly and using already_AddRefed when a reference is returned, > this won't compile if an nsCOMPtr is required. If we change to > returning nsCOMPtr instead of returning already_AddRefed, this > won't be the case, and the code will end up having a dangling > pointer to a potentially-deleted object. For refcounted types, isn't a raw pointer in a local variable a red flag to reviewers to begin with? If GetT() returns a raw pointer today, like nsINode::GetFirstChild() or something, storing the result in a raw pointer is a potential use-after-free, and that definitely has happened already. Reviewers need to make sure that refcounted types aren't ever kept in raw pointers in local variables, unless perhaps it's very clear from the code that nothing can possibly call Release() (although it still makes me nervous). > (2) If we agree it's a good idea from a coding style perspective, > it's probably worth having a look that the generated code is > equally efficient, given how widely used it is in our codebase. The implementation includes a simple test-case that verifies that there are no unexpected addrefs when relying on move constructors. Beyond that (actual assembly instructions) I don't know. _______________________________________________ dev-platform mailing list [email protected] https://lists.mozilla.org/listinfo/dev-platform
