With regard to what #2 is doing, I threw the following into a file with
nsRefPtr, and got clang to dump the AST:
struct Foo {void AddRef() {} void Release() {}};
nsRefPtr<Foo> bar;
void foo() {
Foo* x = true ? nullptr : bar;
}
`-FunctionDecl 0x103805750 <line:943:1, line:945:1> line:943:6 foo 'void
(void)'
`-CompoundStmt 0x103807708 <col:12, line:945:1>
`-DeclStmt 0x1038076f0 <line:944:3, col:32>
`-VarDecl 0x103805800 <col:3, col:29> col:8 x 'struct Foo *' cinit
`-ExprWithCleanups 0x1038076d8 <col:12, col:29> 'struct Foo *'
`-ImplicitCastExpr 0x1038076c0 <col:12, col:29> 'struct Foo *'
<UserDefinedConversion>
`-CXXMemberCallExpr 0x103807698 <col:12, col:29> 'struct Foo *'
`-MemberExpr 0x103807668 <col:12, col:29> '<bound member
function type>' .operator Foo * 0x1038048a0
`-ImplicitCastExpr 0x103807650 <col:12, col:29> 'const
class nsRefPtr<struct Foo>' <NoOp>
`-ConditionalOperator 0x103807618 <col:12, col:29>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>'
|-CXXBoolLiteralExpr 0x103805858 <col:12> '_Bool' true
|-CXXBindTemporaryExpr 0x1038074a8 <col:19>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' (CXXTemporary
0x1038074a0)
| `-CXXConstructExpr 0x103807468 <col:19>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' 'void (nsRefPtr<struct
Foo> &&)' elidable
| `-MaterializeTemporaryExpr 0x103807450 <col:19>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' xvalue
| `-CXXBindTemporaryExpr 0x103807358 <col:19>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' (CXXTemporary
0x103807350)
| `-CXXConstructExpr 0x103807318 <col:19>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' 'void (nsRefPtr<struct
Foo> &&)' elidable
| `-MaterializeTemporaryExpr 0x103807300
<col:19> 'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' xvalue
| `-CXXBindTemporaryExpr 0x103806f38 <col:19>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' (CXXTemporary
0x103806f30)
| `-ImplicitCastExpr 0x103806f10 <col:19>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' <ConstructorConversion>
| `-CXXConstructExpr 0x103806ed8 <col:19>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' 'void (struct Foo *)'
| `-ImplicitCastExpr 0x103806ec0
<col:19> 'struct Foo *' <NullToPointer>
| `-CXXNullPtrLiteralExpr 0x103805870
<col:19> 'nullptr_t'
`-CXXBindTemporaryExpr 0x1038075f8 <col:29>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' (CXXTemporary
0x1038075f0)
`-CXXConstructExpr 0x1038075b8 <col:29>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' 'void (const
nsRefPtr<struct Foo> &)'
`-ImplicitCastExpr 0x1038075a0 <col:29> 'const
nsRefPtr<struct Foo>':'const class nsRefPtr<struct Foo>' lvalue <NoOp>
`-DeclRefExpr 0x103805888 <col:29>
'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>' lvalue Var 0x103084da0
'bar' 'nsRefPtr<struct Foo>':'class nsRefPtr<struct Foo>'
In summary, the nsRefPtr is copied into a temporary in it's 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.
The problem here appears to be that when deciding the type of the ternary,
c++ chooses nsRefPtr<Foo>, rather than Foo*. Adding the get() makes C++
choose the correct type for the ternary, and avoids the cast of the rvalue
reference.
On Wed, Jul 1, 2015 at 3:48 PM, Nathan Froyd <nfr...@mozilla.com> wrote:
> On Wed, Jul 1, 2015 at 7:39 AM, Aryeh Gregor <a...@aryeh.name> wrote:
>
> > On Wed, Jul 1, 2015 at 6:24 AM, Joshua Cranmer [image: 🐧] <
> pidgeo...@gmail.com>
> > wrote:
> > > You'd get the same benefit, I think, by making operator T*() && =
> > delete;, syntax which is accepted on gcc 4.8.1+, clang, and MSVC 2015
> IIRC.
> >
> > I once tried this and found it had problematic side effects that made
> > deploying it difficult in practice. I think one involved the ternary
> > operator. Unfortunately, I can't remember exactly what they were. :(
> > Anyone who's interested can try for themselves by just adding that
> > line to nsCOMPtr.h, recompiling, and seeing what breaks.
> >
>
> I tried this, fixed a few compilation errors, then decided this wasn't
> worth it just yet and threw my work into a bug. Comments appreciated:
>
> https://bugzilla.mozilla.org/show_bug.cgi?id=1179451
>
> -Nathan
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
