Also clean up some crap. This blocks separate compilation for JSC builtins.
Created attachment 263910 [details] Proposed Fix Might need a few rounds of EWS to get the CMake changes correct.
Comment on attachment 263910 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=263910&action=review :D r=me with very small nit. > Source/JavaScriptCore/create_hash_table:325 > if ($values[$i]{"type"} eq "Function") { I think making this as `$values[$i]{"type"} eq "Function" && $includeBuiltin` is preferable since it maintains consistency between the upper one and lower one. Or defining a variable like `my $mayBeBuildinFunction = $values[$i]{"type"} eq "Function" && $includeBuiltin` and using it may be nice.
I am not very familiar with lut use and generation. Sorry if some of the points below are not useful. The current way of using JS built-ins through lut information in CPP files seems a bit complex to me. Wouldn't it be easier if the lut info was more explicit about whether a function is implemented as built-in or not? That would be clearer to the reader. This would remove the need for JSC_BUILTIN_EXISTS. This may help the script become smarter. In the scope of that particular bug, it seems easier for me to include "XXXBuiltins.h" in a CPP file than to update the two build systems when a lut info starts to use built-ins.
(In reply to comment #3) > I am not very familiar with lut use and generation. Sorry if some of the > points below are not useful. > > The current way of using JS built-ins through lut information in CPP files > seems a bit complex to me. > Wouldn't it be easier if the lut info was more explicit about whether a > function is implemented as built-in or not? > That would be clearer to the reader. This would remove the need for > JSC_BUILTIN_EXISTS. This may help the script become smarter. That could work. > In the scope of that particular bug, it seems easier for me to include > "XXXBuiltins.h" in a CPP file than to update the two build systems when a > lut info starts to use built-ins. If XXX is JSC, then touching any builtin.js file will cause all files that include a .lut.h (and have a builtin) to rebuild. If XXX is the specific builtin, then the generator needs to know which inputs have builtins otherwise it will include a non-existent header for those without. If the lut input format is changed to include builtin/native, then we can remove the guards and includes altogether. But I think this is an easier first step towards separate compilation than that.
> If XXX is the specific > builtin, then the generator needs to know which inputs have builtins > otherwise it will include a non-existent header for those without. Or the generator can include no header at all, leaving that work to the the cpp file that includes the .lut.h. No big difference really, except maybe that the explicit include is a hint for cautious readers that built-ins are in action.
I am investigating the CMake build failures, but haven't figured it out yet. It looks like this change caused NativeErrorPrototype.cpp to not see a definition for class ErrorPrototype. I didn't change the headers However all the .lut.h files seem to generated correctly. There might be some trouble with the new create_hash_tables invocation.
Created attachment 263983 [details] Fix CMake Required a bit of hackery to pass flags to a macro in CMake. But it seems to work now.
Attachment 263983 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/CMakeLists.txt:1029: No space before ")" [whitespace/parentheses] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r191537: <http://trac.webkit.org/changeset/191537>
Comment on attachment 263983 [details] Fix CMake Landed manually.