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
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
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
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
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
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.,
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
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
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 =
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
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.
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
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.
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
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.
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
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
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
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
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
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
21 matches
Mail list logo