953410 - JS_ION define is inconsistently used

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Martin Husemann]

Attachment: Add #ifdef JS_ION around includes of IonCode.h and usages

Builds without JS_ION fail due to inconsistent usage of the define. This is all very messy, one way to fix it is to #ifdef a lot places (see attached patch), another option would be to make js/src/jit/IonCode.h compilable without JS_ION.

Comment 1

[Landry Breuil (:gaston)]

Comment on attachment 8351703 [details] [diff] [review]
Add #ifdef JS_ION around includes of IonCode.h and usages

Do you see the same build failure as on bug #953210 ? If so the two bugs could be merged..

(i'm not sure commenting out needGC=true in jscompartment.h will work out :)

Comment 2

[Martin Husemann]

Yes, without this patch I see the same problem as bug #953210.
The commenting of needGC = true may be unrelated, but I could not find a definition for needGC and it made the compile fail.

Comment 3

[Brian Hackett [Laid off!]]

Comment on attachment 8351703 [details] [diff] [review]
Add #ifdef JS_ION around includes of IonCode.h and usages

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

Generally it'd be good to avoid sprinkling #ifdef JS_ION around the codebase except where absolutely necessary.  Things are in kind of bad shape right now wrt this idea but this patch makes things worse.  This build failure is a recent regression from bug 939614 and modifying the header added by that bug makes --disable-ion builds work for me, patch in a minute.

Comment 4

[Brian Hackett [Laid off!]]

Attachment: change JitOptions.h

Comment 6

[Hannes Verschore [:h4writer]]

(In reply to Brian Hackett (:bhackett) from comment #4)
> Created attachment 8354265 [details] [diff] [review]
> change JitOptions.h

I can't find the place where we would need JitOptions.h when JS_ION is false?

Comment 7

[Brian Hackett [Laid off!]]

Try compiling with a --disable-ion build.

Comment 8

[Hannes Verschore [:h4writer]]

Comment on attachment 8354265 [details] [diff] [review]
change JitOptions.h

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

We don't need to compile jitOptions.h for --disable-ion builds. It looks like I forgot to add #ifdef JS_ION to IonOptimizationLevels.h. I'll publish patch.

Comment 9

[Hannes Verschore [:h4writer]]

Attachment: Add JS_ION to IonOptimizationLevels.h

This fixes the compiler issues when compiling with --disable-ion

Comment 10

[Brian Hackett [Laid off!]]

Comment on attachment 8355219 [details] [diff] [review]
Add JS_ION to IonOptimizationLevels.h

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

Whatever.

Comment 11

[Hannes Verschore [:h4writer]]

Thanks for the quick review!

https://hg.mozilla.org/integration/mozilla-inbound/rev/e11a69d3d4ba

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e11a69d3d4ba