If create_hash_table script knew whether a function is JSBuiltin or not, we could do the following: - Simplify lut generated code (removing #if JSC_BUILTIN_EXIST...) - Simplify JS built-in generated code (removal JSC_BUILTIN_EXIST macros generation) - Include "JSCBuiltins.h" in the lut generated code only if the table contains a JS built-in function. This could also simplify the build system by removing the "-b" option of the create_hash_table script.
Created attachment 265038 [details] Patch
This patch modifies the JS builtins code generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-builtins-generator-tests --reset-results`)
This patch does not yet simplify the build system. This can be done in a follow-up bug, removing -b (and -i as well?).
Comment on attachment 265038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265038&action=review Good cleanup work! > Source/JavaScriptCore/ChangeLog:14 > + A further patch should simplify the build system by removing create_has_table -b option. Nit: create_hash_table > Source/JavaScriptCore/runtime/ArrayConstructor.cpp:54 > + of JSBuiltin DontEnum|Function 0 I would have preferred something that's an invalid C++ function name, like @builtin or something. But this is fine, I think.
Created attachment 265149 [details] Patch for landing
> Good cleanup work! Thanks :) > > Source/JavaScriptCore/ChangeLog:14 > > + A further patch should simplify the build system by removing create_has_table -b option. > > Nit: create_hash_table OK > > Source/JavaScriptCore/runtime/ArrayConstructor.cpp:54 > > + of JSBuiltin DontEnum|Function 0 > > I would have preferred something that's an invalid C++ function name, like > @builtin or something. But this is fine, I think. Yes, it could be done. I kept JSBuiltin, as it is the same as the IDL keyword. Also, in lut.h files, most of the function have names starting with lower case letters. It might be good to make the naming more consistent.
Comment on attachment 265149 [details] Patch for landing Clearing flags on attachment: 265149 Committed r192204: <http://trac.webkit.org/changeset/192204>
All reviewed patches have been landed. Closing bug.