Created attachment 810705 [details] [diff] [review] Patch (v1) https://tbpl.mozilla.org/?tree=Try&rev=3ccdd7d72da1
Attachment #810705 - Flags: review?(luke)
Comment on attachment 810705 [details] [diff] [review] Patch (v1) Thanks! One nit throughout the patch: can you indent everywhere you put #idefs like so: #ifdef MOZ_INSTRUMENTS # include "devtools/Instruments.h" #endif
Attachment #810705 - Flags: review?(luke) → review+
(In reply to comment #2) > Thanks! One nit throughout the patch: can you indent everywhere you put #idefs > like so: > > #ifdef MOZ_INSTRUMENTS > # include "devtools/Instruments.h" > #endif Sure, will do before landing.
This broke no-ion builds: <https://tbpl.mozilla.org/php/getParsedLog.php?id=28432259&tree=Mozilla-Inbound> I tried to fix this locally, but I don't think I know enough about what should and should not be in no-ion builds. Luke, any idea who can fix those builds?
It's weird that this didn't show up in the try push. You should be able to repro the compile errors locally by just configuring and building a js shell with --disable-ion.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Note: I am seeing a lot of build errors with --disable-ion.
Depends on: 921584
(In reply to Luke Wagner [:luke] from comment #6) > It's weird that this didn't show up in the try push. You should be able to > repro the compile errors locally by just configuring and building a js shell > with --disable-ion. Yeah I can *reproduce* the errors just fine. I just don't know how to fix them, because it actually requires one to know what code should be compiled in no-ion mode and which code should not... :/
Created attachment 811901 [details] [diff] [review] Fix --disable-ion build I don't know why ForkJoin.h needs MIR.h and Clang doesn't know either (tried both debug/opt builds).
Attachment #811901 - Flags: review?(luke)
Backed out for compilation failures on ion android builds: https://tbpl.mozilla.org/php/getParsedLog.php?id=28560698&tree=Mozilla-Inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/210167f25955
Sorry for that. We have to forward-declare MDefinition, don't know how I missed that and why it only breaks Android. I sent an updated patch to Try. On the bright side, it did fix the No-Ion build...
Let's try this again with a forward declaration of MDefinition: https://hg.mozilla.org/integration/mozilla-inbound/rev/e43f1bac03c1 Linux/Windows/Android/Android NoIon are all green on Try: https://tbpl.mozilla.org/?tree=Try&showall=1&rev=1a0bf267064e
You need to log in before you can comment on or make changes to this bug.