On 04/10/2015 09:09 PM, Seth Fowler wrote:
On Apr 10, 2015, at 8:46 AM, Ehsan Akhgari <ehsan.akhg...@gmail.com> wrote: I would like to propose that we should ban the usage of refcounted objects inside lambdas in Gecko. Here is the reason: Consider the following code: nsINode* myNode; TakeLambda([&]() { myNode->Foo(); }); There is nothing that guarantees that the lambda passed to TakeLambda is executed before myNode is destroyed somehow.I agree that this pattern is bad, but I don’t think that means that we should ban lambda capture of refcounted objects. This alternative formulation would work just fine today, AFAIK:nsCOMPtr<nsINode> myNode; TakeLambda([=]() { myNode->Foo(); });This captures by value, so we end up with a copy of myNode in the lambda, with the refcount incremented appropriately. Once we have C++14 support everywhere, we can also do this:nsCOMPtr<nsINode> myNode; TakeLambda([myNode = Move(myNode)]() { myNode->Foo(); });To capture by move (and avoid the cost of a refcount increment). Using either of these approaches is enough to make refcounted objects safe to use in lambda capture expressions. Lambdas will be much less useful if they can’t capture refcounted objects, so I’m strongly against banning that.
I'd say that is rather painful for reviewers, since both Move() (I prefer .swap()) and lambda hide what is actually happening to the refcnt. So easy to forget to use nsCOMPtr explicitly there. We should emphasize easy-to-read-and-understand code over fast-to-write. -Olli
- Seth
_______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
