Bug 50000 - [Qt] Remove redundant JAVASCRIPTCORE_JIT variable
Summary: [Qt] Remove redundant JAVASCRIPTCORE_JIT variable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-11-23 18:25 PST by Laszlo Gombos
Modified: 2012-10-09 00:23 PDT (History)
9 users (show)

See Also:


Attachments
proposed patch (1.45 KB, patch)
2010-11-23 18:31 PST, Laszlo Gombos
no flags Details | Formatted Diff | Diff
proposed patch (1.13 KB, patch)
2012-10-08 14:33 PDT, Laszlo Gombos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2010-11-23 18:25:13 PST
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.
Comment 1 Laszlo Gombos 2010-11-23 18:31:47 PST
Created attachment 74715 [details]
proposed patch
Comment 2 Andreas Kling 2010-11-24 02:07:10 PST
JAVASCRIPTCORE_JIT is used by the Qt build system, we need to make some changes on that side too if we're removing this.
Comment 3 Csaba Osztrogonác 2010-11-24 04:18:27 PST
(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.
Comment 4 Laszlo Gombos 2012-09-25 13:46:00 PDT
There might be an opportunity to fix this in Qt5 if there is an interest. Simon, what do you think ?
Comment 5 Simon Hausmann 2012-09-26 00:04:25 PDT
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.
Comment 6 Laszlo Gombos 2012-10-08 14:33:33 PDT
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.
Comment 7 WebKit Review Bot 2012-10-09 00:22:56 PDT
Comment on attachment 167612 [details]
proposed patch

Clearing flags on attachment: 167612

Committed r130728: <http://trac.webkit.org/changeset/130728>
Comment 8 WebKit Review Bot 2012-10-09 00:23:00 PDT
All reviewed patches have been landed.  Closing bug.