1398677 - nsWrapperCache.h: insufficient ifdef for 64 bit architectures

Status: RESOLVED DUPLICATE of bug 1376277

(Core :: DOM: Core & HTML, defect)

Description

[Martin Husemann]

Attachment: nsWrapperCache.h.patch

nsWrapperCache.h does not allow building on 64bit architectures white-listed in a simple ifdef:

In file included from /usr/pkgobj/www/firefox/work/build/dist/include/mozilla/dom/EventTarget.h:12:0,
                 from /usr/pkgobj/www/firefox/work/firefox-55.0.3/dom/base/nsPIDOMWindow.h:15,
                 from /usr/pkgobj/www/firefox/work/build/dist/include/mozilla/DOMEventTargetHelper.h:13,
                 from /usr/pkgobj/www/firefox/work/firefox-55.0.3/dom/presentation/PresentationAvailability.h:10,
                 from /usr/pkgobj/www/firefox/work/firefox-55.0.3/dom/presentation/AvailabilityCollection.cpp:10,
                 from /usr/pkgobj/www/firefox/work/build/dom/presentation/Unified_cpp_dom_presentation0.cpp:2:
/usr/pkgobj/www/firefox/work/firefox-55.0.3/dom/base/nsWrapperCache.h:48:1: error: static assertion failed: Only support 32-bit and 64-bit
 static_assert(sizeof(void*) == 4, "Only support 32-bit and 64-bit");
 ^

Simple patch to fix this attached.

Comment 1

[Martin Husemann]

that should of course read: "architectures NOT white-listed"
and the simple fix is to check for the very generic __LP64__.

Comment 2

[Martin Husemann]

The

 defined(__x86_64__) || defined(__aarch64__) 

could probably be removed with that change, unless there is some very wiered arch64 compiler on windows that requires this.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8906495 [details] [diff] [review]
nsWrapperCache.h.patch

This looks reasonable to me, but the patch is not formatted properly.  It needs:

1. To modify the exact file, not file ".orig"
2. It needs author information added
3. It needs a commit message in the form "Bug 1398677 <description> r=bkelly"

See here for more information:

https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Can you update the patch and resubmit it for review again?  Sorry for the hassle.  Thanks!

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Martin, I am assigning this to you as you are already working on this. Thank you for reporting and providing a patch.

Comment 5

[Jan Beich]

(In reply to Martin Husemann from comment #2)
> defined(__x86_64__) || defined(__aarch64__) could probably be removed

Already fixed as you've suggested in Firefox 56 by bug 1376277.