Closed Bug 1476987 Opened Last year Closed Last year

Eliminate nsXPTCVariant::ptr

Categories

(Core :: XPConnect, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
MozReview-Commit-ID: JCazltUuMBf
Comment on attachment 8993397 [details]
Bug 1476987 - Eliminate nsXPTCVariant::ptr.

:Nika Layzell has approved the revision.

https://phabricator.services.mozilla.com/D2241
Attachment #8993397 - Flags: review+
Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/08339a56f3ae
Eliminate nsXPTCVariant::ptr. r=nika
https://hg.mozilla.org/mozilla-central/rev/08339a56f3ae
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1478560
This broke every unix that is not {arm,aarch64,x86,x64}-linux.
(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.
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"?
Flags: needinfo?(bobbyholley)
(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.
Flags: needinfo?(bobbyholley)
You need to log in before you can comment on or make changes to this bug.