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.
Created attachment 74715 [details] proposed patch
JAVASCRIPTCORE_JIT is used by the Qt build system, we need to make some changes on that side too if we're removing this.
(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.
There might be an opportunity to fix this in Qt5 if there is an interest. Simon, what do you think ?
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.
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 on attachment 167612 [details] proposed patch Clearing flags on attachment: 167612 Committed r130728: <http://trac.webkit.org/changeset/130728>
All reviewed patches have been landed. Closing bug.