Bug 50000 – [Qt] Remove redundant JAVASCRIPTCORE_JIT variable

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Laszlo Gombos]

I find JAVASCRIPTCORE_JIT build option redundant as we have a standard ENABLE_X variable for it which works quite well. I propose to remove it.

[Laszlo Gombos]

Created attachment 74715 [details]
proposed patch

[Andreas Kling]

JAVASCRIPTCORE_JIT is used by the Qt build system, we need to make some changes on that side too if we're removing this.

[Csaba Osztrogonác]

(In reply to comment #2)
> JAVASCRIPTCORE_JIT is used by the Qt build system, we need to make some changes on that side too if we're removing this.

Good point. We don't want to break building trunk WebKit inside Qt,
so we shouldn't land it now. I propose to fix it in Qt and after 
the new Qt release land this fix.

[Laszlo Gombos]

There might be an opportunity to fix this in Qt5 if there is an interest. Simon, what do you think ?

[Simon Hausmann]

Comment on attachment 74715 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74715&action=review

> WebKit.pri:90
> -    contains(JAVASCRIPTCORE_JIT,yes): error("'JAVASCRIPTCORE_JIT=yes' not supported with valgrind")
> +    contains(DEFINES, ENABLE_JIT=1): error("'ENABLE_JIT=1' not supported with valgrind, setting ENABLE_JIT=0")

This check is not correct, with --smc-check=all-non-file valgrind appears to work just fine.

> common.pri:-4
> -contains(JAVASCRIPTCORE_JIT,yes): DEFINES+=ENABLE_JIT=1
> -contains(JAVASCRIPTCORE_JIT,no): DEFINES+=ENABLE_JIT=0

Yeah, let's get rid of this variable. I believe we've already removed the corresponding "configure option" from qt's configure.

[Laszlo Gombos]

Created attachment 167612 [details]
proposed patch

As Simon indicated valgrind might work with JIT enabled. Potentially in valgrind.prf the following line would make more sense

-DEFINES += ENABLE_JIT=0
+enable?(jit): DEFINES += ENABLE_JIT=0

I am happy to make that change or perhaps it could be in a follow-up patch.

[WebKit Review Bot]

Comment on attachment 167612 [details]
proposed patch

Clearing flags on attachment: 167612

Committed r130728: <http://trac.webkit.org/changeset/130728>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.