1476987 - Eliminate nsXPTCVariant::ptr

Status: RESOLVED FIXED

(Core :: XPConnect, enhancement)

Description

[Bobby Holley (:bholley)]

I started writing a patch for bug 1461789, but sounds like Nika is already on it.

I did notice in the process that we still have that annoying and unnecessary ptr field. It doesn't matter too much, but it bloats the stack footprint of XPConnect calls. I didn't fix it in years past because I didn't want to risk breaking the long tail of xptcall implementations, but we've now agreed that it's ok to break them.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Bug 1476987 - Eliminate nsXPTCVariant::ptr.

MozReview-Commit-ID: JCazltUuMBf

Comment 2

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e1b2e9bd30ecd2d6c379b82ef33ad7da3e1aba0

Comment 3

[Phabricator Automation]

Comment on attachment 8993397 [details]
Bug 1476987 - Eliminate nsXPTCVariant::ptr.

:Nika Layzell has approved the revision.

https://phabricator.services.mozilla.com/D2241

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08339a56f3ae
Eliminate nsXPTCVariant::ptr. r=nika

Comment 5

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/08339a56f3ae

Comment 6

[Mike Hommey [:glandium]]

This broke every unix that is not {arm,aarch64,x86,x64}-linux.

Comment 7

[Bobby Holley (:bholley)]

(In reply to Mike Hommey [:glandium] from comment #6)
> This broke every unix that is not {arm,aarch64,x86,x64}-linux.

Yeah, that's intentional. I'll comment in the other bug.

Comment 8

[Nicholas Nethercote [inactive]]

Would it be worth at least adding comments to the files for the broken platforms, saying something like "this code is now broken, see bug 1476987 for what needs fixing"?

Comment 9

[Bobby Holley (:bholley)]

(In reply to Nicholas Nethercote [:njn] from comment #8)
> Would it be worth at least adding comments to the files for the broken
> platforms, saying something like "this code is now broken, see bug 1476987
> for what needs fixing"?

IMO, bug 1479807 and the associated mailing list announcement should be sufficient for people to follow the bread crumbs (using a |git log| on the directory to see the changes to the other files).

You're certainly welcome to add those comments if you like, though if you do I'd prefer for you to point people to bug 1479807 instead of this bug, so that maintainers can get connected to the right channels to be notified in the future.