Closed Bug 1376277 Opened 3 years ago Closed 3 years ago

nsWrapperCache.h: add support for 64bits sparc build

Categories

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

52 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: petr.sumbera, Assigned: petr.sumbera)

References

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; SunOS i86pc; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170612021003

Steps to reproduce:

11:20.89 In file included from /scratch/firefox/obj-sparc64-sun-solaris2.12/dist/include/mozilla/dom/TimeRanges.h:14:0,
11:20.90                  from /scratch/firefox/obj-sparc64-sun-solaris2.12/dist/include/TimeUnits.h:14,
11:20.90                  from /scratch/firefox/obj-sparc64-sun-solaris2.12/dist/include/MediaData.h:12,
11:20.90                  from /scratch/firefox/media/libstagefright/binding/Adts.cpp:6,
11:20.90                  from /scratch/firefox/obj-sparc64-sun-solaris2.12/media/libstagefright/Unified_cpp_media_libstagefright0.cpp:2:
11:20.90 /scratch/firefox/obj-sparc64-sun-solaris2.12/dist/include/nsWrapperCache.h:48:1: error: static assertion failed: Only support 32-bit and 64-bit
11:20.90  static_assert(sizeof(void*) == 4, "Only support 32-bit and 64-bit");
11:20.91  ^
Attached patch Bug1376277.patch (obsolete) — Splinter Review
Attachment #8881237 - Flags: review?(nfroyd)
Component: Untriaged → DOM
Product: Firefox → Core
Comment on attachment 8881237 [details] [diff] [review]
Bug1376277.patch

Review of attachment 8881237 [details] [diff] [review]:
-----------------------------------------------------------------

Does 64-bit SPARC also define __LP64__?  It would be better to use that check instead of constantly adding new cases for every new 64-bit CPU architecture (PowerPC, MIPS, etc. etc.)

If yes, then r=me with the condition modified to:

#if defined(_M_X64) || defined(__LP64__)

assuming that still builds on x86-64.  If not, then r=me.
Attachment #8881237 - Flags: review?(nfroyd) → review+
Attached patch Bug1376277.patchSplinter Review
Attachment #8881237 - Attachment is obsolete: true
Attachment #8881362 - Flags: review?(nfroyd)
Attachment #8881362 - Flags: review?(nfroyd) → review+
Keywords: checkin-needed
Assignee: nobody → petr.sumbera
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/036ce4d8ca09
nsWrapperCache.h: add support for 64bits sparc build. r=glandium
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/036ce4d8ca09
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(In reply to Nathan Froyd [:froydnj] from comment #2)
> Comment on attachment 8881237 [details] [diff] [review]
> Bug1376277.patch
> 
> Review of attachment 8881237 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Does 64-bit SPARC also define __LP64__?  It would be better to use that
> check instead of constantly adding new cases for every new 64-bit CPU
> architecture (PowerPC, MIPS, etc. etc.)
> 
> If yes, then r=me with the condition modified to:
> 
> #if defined(_M_X64) || defined(__LP64__)
> 
> assuming that still builds on x86-64.  If not, then r=me.

Should just have been HAVE_64BIT_BUILD. Which I was surprised I didn't mention when I saw r=glandium in the commit message, although I didn't remember having reviewed that code, and it turns out I didn't...
Duplicate of this bug: 1398677
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.