On Thu, Jul 2, 2015 at 9:21 AM, Neil <[email protected]> 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 nsCOMPtr<T>-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 nsCOMPtr<T>& 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:
nsCOMPtr<nsIRunnable> event =
NS_NewRunnableMethodWithArg<nsCOMPtr<nsIDOMHTMLInputElement>>
(this, &nsFormFillController::MaybeStartControllingInput,
focusedInput);
the nsCOMPtr<nsIDOMHTMLInputElement> gets the storage class
StoreCopyPassByValue<nsCOMPtr<T>>, 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 ParameterStorage<nsCOMPtr<T>> is
StoreCopyPassByConstLRef<nsCOMPtr<T>>? (Similarly for nsRefPtr<T>.) 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
[email protected]
https://lists.mozilla.org/listinfo/dev-platform
