Closed
Bug 1439659
Opened 7 years ago
Closed 5 years ago
MOZ_LITTLE_ENDIAN and MOZ_BIG_ENDIAN should be function-like macros
Categories
(Core :: MFBT, enhancement, P3)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla73
Tracking | Status | |
---|---|---|
firefox73 | --- | fixed |
People
(Reporter: jorendorff, Assigned: Waldo)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
We have a lot of code that does stuff like
#if MOZ_LITTLE_ENDIAN
...
#else
...
#endif
without first doing #include "mozilla/EndianUtils.h" directly. The code only works because of unified builds or because the file includes EndianUtils.h indirectly.
When you disturb the house of cards, the symptom is you get is... well, just imagine it. *One* Gecko source file thinks you're on a big-endian machine. Mirror universe. You the programmer aren't expecting that at all. It's maddening to track down. I literally manually rm -rf'd my objdir and rebuilt before I could believe what the debugger was telling me.
If we changed these macros to be function-like, the symptom would be: compiler errors. For these particular macros, I think it's worth doing.
Comment 1•7 years ago
|
||
+0x01000000
Updated•5 years ago
|
Blocks: big-endian
Assignee | ||
Comment 2•5 years ago
|
||
Updated•5 years ago
|
Assignee: nobody → jwalden
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•5 years ago
|
||
Perhaps worth noting, when you have function-macros like this it is, um, "less possible" to accidentally use #ifdef
with a macro designed to be used with #if
. As apparently happened a few places previously, as you can see in certain places in the patch's changes.
Unfortunately it's only "less possible" because at least with my clang, the result I got was a compiler warning about extra tokens at end of those lines. Still, a warning is better than the nothing that was happening before.
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/6b57e33999e9
Make |MOZ_{LITTLE,BIG}_ENDIAN| into function macros so that invoking them inside |#if| conditions when they haven't been defined yet is a compile error. r=froydnj
Comment 5•5 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox73:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Assignee | ||
Comment 6•5 years ago
|
||
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/7bebdc108aa3
Fix inadvertent typo (that would have been immediately obvious as compile error in a big-endian build, because of this bug's change, happily). r=froydnj
Comment 8•5 years ago
|
||
bugherder |
You need to log in
before you can comment on or make changes to this bug.
Description
•