Bug 50000

Summary: [Qt] Remove redundant JAVASCRIPTCORE_JIT variable
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, benjamin, hausmann, jturcotte, kling, ossy, rafael.lobo, vestbo, webkit.review.bot
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
proposed patch
none
proposed patch none

Laszlo Gombos
Reported 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.
Attachments
proposed patch (1.45 KB, patch)
2010-11-23 18:31 PST, Laszlo Gombos 		no flags
DetailsFormatted DiffDiff
proposed patch (1.13 KB, patch)
2012-10-08 14:33 PDT, Laszlo Gombos 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Laszlo Gombos
Comment 1 2010-11-23 18:31:47 PST
Created attachment 74715 [details] proposed patch
Andreas Kling
Comment 2 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.
Csaba Osztrogonác
Comment 3 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.
Laszlo Gombos
Comment 4 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 ?
Simon Hausmann
Comment 5 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.
Laszlo Gombos
Comment 6 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.
WebKit Review Bot
Comment 7 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>
WebKit Review Bot
Comment 8 2012-10-09 00:23:00 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.