Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Tue, Jul 7, 2015 at 6:10 PM, Michael Layzell mich...@thelayzells.com wrote: No, I never checked if it happens on an optimized build, but as C++ follows an as-if principal, which means that code has to execute as if those temporaries had been created. Unfortunately, AddRef and Release are virtual, which, I'm pretty sure, means that the compiler can't optimize them out as they may have arbitrary side effects :(. So the temporaries are probably eliminated, but the calls to AddRef and Release are probably not. I could be wrong on this though. This is true except in special cases where copy elision is allowed, in which case the temporary may be removed entirely, with its constructor and destructor. According to 12.8.32 of http://www.open-std.org/Jtc1/sc22/wg21/docs/papers/2011/n3242.pdf, one of those cases is when a temporary class object that has not been bound to a reference (12.2) would be copied/moved to a class object with the same cv-unqualified type, the copy/move operation can be omitted by constructing the temporary object directly into the target of the omitted copy/move I'm not an expert in C++-ese, but this looks like exactly that, so compilers should be allowed to remove the AddRef/Release. Whether they do or not is a separate question -- I'd hope so, in such a simple-looking case. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
I brought this up on IRC, but I think this is worth taking to the list: Replacing TemporaryRef with already_AddRefed has already caused at least one leak because already_AddRefed does not call Release() on the pointer it holds if nothing takes ownership of that pointer before its destructor runs. TemporaryRef did do that, and this seems like a strictly safer approach. My question is: why doesn’t already_AddRefed call Release() in its destructor? The fact that it doesn’t seems to me to be a profoundly unintuitive footgun. I don’t think we can trust reviewers to catch this, either. The fact that Foo() is declared as: already_AddRefedBar Foo(); won’t necessarily be obvious from a call site, where the reviewer will just see: Foo(); That call will leak, and if we don’t happen to hit that code path in our tests, we may not find out about it until after shipping it. I’d prefer that already_AddRefed just call Release(), but if there are performance arguments against that, then we should be checking for this pattern with static analysis. I don’t think the situation as it stands should be allowed to remain for long. - Seth On Jun 30, 2015, at 12:02 PM, Nathan Froyd nfr...@mozilla.com wrote: Bug 1161627 has landed on inbound, which converts all uses of mozilla::TemporaryRef to already_AddRefed and removes TemporaryRef. (already_AddRefed moved to MFBT several months ago, in case you were wondering about the spreading of XPCOM concepts.) TemporaryRef was added for easier porting of code from other engines, but as it has not been used much for that purpose and it led to somewhat less efficient code in places, it was deemed a failed experiment. -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
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Tue, Jul 7, 2015 at 9:59 AM, Seth Fowler mfow...@mozilla.com wrote: I brought this up on IRC, but I think this is worth taking to the list: Replacing TemporaryRef with already_AddRefed has already caused at least one leak because already_AddRefed does not call Release() on the pointer it holds if nothing takes ownership of that pointer before its destructor runs. TemporaryRef did do that, and this seems like a strictly safer approach. My question is: why doesn’t already_AddRefed call Release() in its destructor? The fact that it doesn’t seems to me to be a profoundly unintuitive footgun. We have a version of already_AddRefed that automatically releases the value it holds when it's destroyed; it's called nsCOMPtr. I don’t think we can trust reviewers to catch this, either. The fact that Foo() is declared as: already_AddRefedBar Foo(); won’t necessarily be obvious from a call site, where the reviewer will just see: Foo(); Foo should be MOZ_WARN_UNUSED_RESULT (or whatever that annotation is) then, as should anything that returns an already_AddRefed. That call will leak, and if we don’t happen to hit that code path in our tests, we may not find out about it until after shipping it. Ensuring that code that is untested works is in general a hard thing to do. I'd prefer that people wrote tests. (yes, there may be edge cases in which this isn't possible) I’d prefer that already_AddRefed just call Release(), but if there are performance arguments against that, then we should be checking for this pattern with static analysis. I don’t think the situation as it stands should be allowed to remain for long. There is currently a dynamic analysis (in the form of a fatal assertion) in the already_AddRefed dtor that I added last year. - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
We're already working on a static analysis in https://bugzilla.mozilla.org/show_bug.cgi?id=1180993 which will prevent code like that from passing try. Hopefully that will help with your concerns. On 2015-07-07 12:59 PM, Seth Fowler wrote: I brought this up on IRC, but I think this is worth taking to the list: Replacing TemporaryRef with already_AddRefed has already caused at least one leak because already_AddRefed does not call Release() on the pointer it holds if nothing takes ownership of that pointer before its destructor runs. TemporaryRef did do that, and this seems like a strictly safer approach. My question is: why doesn’t already_AddRefed call Release() in its destructor? The fact that it doesn’t seems to me to be a profoundly unintuitive footgun. I don’t think we can trust reviewers to catch this, either. The fact that Foo() is declared as: already_AddRefedBar Foo(); won’t necessarily be obvious from a call site, where the reviewer will just see: Foo(); That call will leak, and if we don’t happen to hit that code path in our tests, we may not find out about it until after shipping it. I’d prefer that already_AddRefed just call Release(), but if there are performance arguments against that, then we should be checking for this pattern with static analysis. I don’t think the situation as it stands should be allowed to remain for long. - Seth On Jun 30, 2015, at 12:02 PM, Nathan Froyd nfr...@mozilla.com wrote: Bug 1161627 has landed on inbound, which converts all uses of mozilla::TemporaryRef to already_AddRefed and removes TemporaryRef. (already_AddRefed moved to MFBT several months ago, in case you were wondering about the spreading of XPCOM concepts.) TemporaryRef was added for easier porting of code from other engines, but as it has not been used much for that purpose and it led to somewhat less efficient code in places, it was deemed a failed experiment. -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 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On 2015-07-07 6:37 AM, Aryeh Gregor wrote: Did you check whether this actually occurs in an optimized build? C++ allows temporaries to be optimized away under some circumstances, e.g., when returning a local variable. It would make a lot of sense to me if it allowed the temporary created by a ternary operator to be optimized away. No, I never checked if it happens on an optimized build, but as C++ follows an as-if principal, which means that code has to execute as if those temporaries had been created. Unfortunately, AddRef and Release are virtual, which, I'm pretty sure, means that the compiler can't optimize them out as they may have arbitrary side effects :(. So the temporaries are probably eliminated, but the calls to AddRef and Release are probably not. I could be wrong on this though. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Fri, Jul 3, 2015 at 6:22 PM, Michael Layzell mich...@thelayzells.com wrote: So the ternary actually causes an unnecessary AddRef/Release pair, neat. Did you check whether this actually occurs in an optimized build? C++ allows temporaries to be optimized away under some circumstances, e.g., when returning a local variable. It would make a lot of sense to me if it allowed the temporary created by a ternary operator to be optimized away. The problem here appears to be that when deciding the type of the ternary, c++ chooses nsRefPtrFoo, rather than Foo*. Adding the get() makes C++ choose the correct type for the ternary, and avoids the cast of the rvalue reference. I think it's pretty clear that nsRefPtrFoo must be the type of the ternary expression. I would be rather surprised if the type of the ternary depended on what you did with the result! ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Fwd: mozilla::TemporaryRef is gone; please use already_AddRefed
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? -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Fwd: mozilla::TemporaryRef is gone; please use already_AddRefed
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
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Fri, Jul 3, 2015 at 3:56 PM, Neil n...@parkwaycc.co.uk wrote: Aryeh Gregor wrote: we still want a new type for function parameters that accepts implicit conversions from nsRefPtr/nsCOMPtr, to use instead of raw pointers. Sure, but that won't stop someone from writing ArgFoo foo = ReturnFoo2(); will it? No, but that's an extremely obvious bug that should be caught in review. If you want to be sure, you could do static analysis of some kind, but I don't expect it's necessary. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Fwd: mozilla::TemporaryRef is gone; please use already_AddRefed
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() {}};
nsRefPtrFoo 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 nsRefPtrstruct Foo' NoOp
`-ConditionalOperator 0x103807618 col:12, col:29
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo'
|-CXXBoolLiteralExpr 0x103805858 col:12 '_Bool' true
|-CXXBindTemporaryExpr 0x1038074a8 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' (CXXTemporary
0x1038074a0)
| `-CXXConstructExpr 0x103807468 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' 'void (nsRefPtrstruct
Foo )' elidable
| `-MaterializeTemporaryExpr 0x103807450 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' xvalue
| `-CXXBindTemporaryExpr 0x103807358 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' (CXXTemporary
0x103807350)
| `-CXXConstructExpr 0x103807318 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' 'void (nsRefPtrstruct
Foo )' elidable
| `-MaterializeTemporaryExpr 0x103807300
col:19 'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' xvalue
| `-CXXBindTemporaryExpr 0x103806f38 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' (CXXTemporary
0x103806f30)
| `-ImplicitCastExpr 0x103806f10 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' ConstructorConversion
| `-CXXConstructExpr 0x103806ed8 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' 'void (struct Foo *)'
| `-ImplicitCastExpr 0x103806ec0
col:19 'struct Foo *' NullToPointer
| `-CXXNullPtrLiteralExpr 0x103805870
col:19 'nullptr_t'
`-CXXBindTemporaryExpr 0x1038075f8 col:29
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' (CXXTemporary
0x1038075f0)
`-CXXConstructExpr 0x1038075b8 col:29
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' 'void (const
nsRefPtrstruct Foo )'
`-ImplicitCastExpr 0x1038075a0 col:29 'const
nsRefPtrstruct Foo':'const class nsRefPtrstruct Foo' lvalue NoOp
`-DeclRefExpr 0x103805888 col:29
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' lvalue Var 0x103084da0
'bar' 'nsRefPtrstruct Foo':'class nsRefPtrstruct 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 nsRefPtrFoo, 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
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Fri, Jul 3, 2015 at 8:56 AM, Neil n...@parkwaycc.co.uk wrote: Sure, but that won't stop someone from writing ArgFoo foo = ReturnFoo2(); will it? Something like that is fairly trivial for a static analysis system like our clang plugin to catch, if we want to create a type like that. Restricting certain types to only exist in temporaries and parameter declarations would not be too hard. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
Nathan Froyd wrote: I guess we could fix this in current m-c to be smarter about the parameter passing, perhaps by declaring that ParameterStoragensCOMPtrT is StoreCopyPassByConstLRefnsCOMPtrT? (Similarly for nsRefPtrT.) So what's StorensRefPtrPassByPtr for? -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
Aryeh Gregor wrote: we still want a new type for function parameters that accepts implicit conversions from nsRefPtr/nsCOMPtr, to use instead of raw pointers. Sure, but that won't stop someone from writing ArgFoo foo = ReturnFoo2(); will it? -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Thu, Jul 2, 2015 at 3:50 PM, Neil n...@parkwaycc.co.uk wrote:
Would you mind reminding me what the failure case this avoids is?
already_AddRefedFoo
ReturnFoo1()
{
nsRefPtrFoo foo = new Foo();
return foo.forget();
}
nsRefPtrFoo
ReturnFoo2()
{
return new Foo();
}
// This doesn't compile
Foo* foo = ReturnFoo1();
// This happily compiles and causes use-after-free
Foo* foo = ReturnFoo2();
Naturally, the hard-to-catch case is when the function returns
something that usually has a refcount above one, so it works fine, but
sometimes will return something with a refcount of exactly one, which
causes a sec-critical arbitrary code execution on Firefox stable.
It's worth pointing out that if we only remove the implicit conversion
to T* for rvalues, you still won't be able to directly pass the
returned value to a function that wants T*, even though this is
perfectly safe (because the destructor for the temporary won't be
called until the function call returns, IIUC). So we still want a new
type for function parameters that accepts implicit conversions from
nsRefPtr/nsCOMPtr, to use instead of raw pointers. But you can't pass
an already_AddRefed directly to such a function right now anyway, so
this isn't actually a disadvantage relative to the status quo.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Thu, Jul 2, 2015 at 9:21 AM, Neil n...@parkwaycc.co.uk wrote: Nathan Froyd wrote: 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 1. It's because QueryInterface has to addref, so we can't directly pass the returned pointer. (But otherwise it's effectively the same as 4.) Sure. This also comes up when passing the results of nsCOMPtrT-returning functions to other things, or returning them as raw pointers. This might point to a deficiency in APIs, of course. 2. Ugh, standard MSVC optimisation doesn't compile an nsCOMPtr in a ternary at all well. (If my x86_64 is up to scratch then I think it creates an internal flag so it remembers which arm of the ternary was constructed.) Hooray for forcing people to write the efficient code, then? :) 3. Why isn't the const nsCOMPtrT converting to a T*? (There's too much indirection for my liking, so I can't work out which storage class is getting applied to the argument.) Because for something like: nsCOMPtrnsIRunnable event = NS_NewRunnableMethodWithArgnsCOMPtrnsIDOMHTMLInputElement (this, nsFormFillController::MaybeStartControllingInput, focusedInput); the nsCOMPtrnsIDOMHTMLInputElement gets the storage class StoreCopyPassByValuensCOMPtrT, which means that the argument gets passed as a (copied!) nsCOMPtr. And that's a temporary, so the implicit conversion to T* is disallowed. I guess we could fix this in current m-c to be smarter about the parameter passing, perhaps by declaring that ParameterStoragensCOMPtrT is StoreCopyPassByConstLRefnsCOMPtrT? (Similarly for nsRefPtrT.) If people want something else, they can explicitly ask for it. 4. Maybe I'm confused, but isn't this the pattern we're trying to avoid so that we can remove already_AddRefed and use moves instead? That's a good point, I didn't consider that when just trying to make things compile. In the particular case I remember fixing (calling nsIDocument::GetBaseURI), the result was stored as a T* (!), so we would have had to construct something explicitly anyway. -Nathan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
Nathan Froyd wrote: 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 1. It's because QueryInterface has to addref, so we can't directly pass the returned pointer. (But otherwise it's effectively the same as 4.) 2. Ugh, standard MSVC optimisation doesn't compile an nsCOMPtr in a ternary at all well. (If my x86_64 is up to scratch then I think it creates an internal flag so it remembers which arm of the ternary was constructed.) 3. Why isn't the const nsCOMPtrT converting to a T*? (There's too much indirection for my liking, so I can't work out which storage class is getting applied to the argument.) 4. Maybe I'm confused, but isn't this the pattern we're trying to avoid so that we can remove already_AddRefed and use moves instead? -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
Jeff Muizelaar wrote: I believe this is predicated on removing the implicit conversion from nsRefPtrT to T* Would you mind reminding me what the failure case this avoids is? -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Wed, Jul 1, 2015 at 6:24 AM, Joshua Cranmer 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. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
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 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
mozilla::TemporaryRef is gone; please use already_AddRefed
Bug 1161627 has landed on inbound, which converts all uses of mozilla::TemporaryRef to already_AddRefed and removes TemporaryRef. (already_AddRefed moved to MFBT several months ago, in case you were wondering about the spreading of XPCOM concepts.) TemporaryRef was added for easier porting of code from other engines, but as it has not been used much for that purpose and it led to somewhat less efficient code in places, it was deemed a failed experiment. -Nathan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
Will it ever be possible to eliminate TemporaryRef and use moves instead? Rob -- oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo owohooo osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o‘oRoaocoao,o’o oioso oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo owohooo osoaoyoso,o o‘oYooouo ofolo!o’o owoiololo oboeo oiono odoaonogoeoro ooofo otohoeo ofoioroeo ooofo ohoeololo. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
