I suppose that the general problem is that we are using an nsRefPtr
lvalue in one side of the branch, and an nsRefPtr xvalue on the other
(as the nullptr is being cast to an nsRefPtr, which has to live in a
temporary making it an xvalue). This is reasonable in a situation where
we actually want an nsRefPtr, as when the value is used somewhere, it
will be moved, and we won't produce any extra addref/releases. The
problem, instead, comes from the operator T*() &&, which allows us to
extract one of these references from an xvalue. I think that every use
of this conversion will frequently be a source of an unnecessary (or
dangerous) addref/release pair.
If we remove it, I don't think that the static analysis will be
necessary, as the pattern of creating an unnecessary temporary, just to
extract a value from it, should be much more visibly wrong.
Foo* x = true ? nullptr : bar;
would look like
Foo* x = (true ? nullptr : bar).get();
which is much more obviously inefficient.
On 2015-07-06 8:49 AM, Neil wrote:
Michael Layzell wrote:
In summary, the nsRefPtr is copied into a temporary in its side of
the conditional. The nullptr is cast to a struct Foo*, which is
constructed into a nsRefPtr, which is bound to a temporary, and then
moved around a bit between temporaries. The resulting temporary
xvalue then has the raw pointer extracted from it (which uses the
rvalue-reference cast call - the part which is causing the problem),
which is stored locally after the temporaries are destroyed. The
temporaries which are created in the conditional are then destroyed,
which causes a Release(), but that is OK because there was an AddRef
due to the extra copy in the nsRefPtr branch.
So the ternary actually causes an unnecessary AddRef/Release pair, neat.
"Neat" as in "Can we please have a static analysis to detect this
please"?
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
