Bug 151016 – create_hash_table should know whether a function is JSBuiltin or not.

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[youenn fablet]

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.

[youenn fablet]

Created attachment 265038 [details]
Patch

[WebKit Commit Bot]

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`)

[youenn fablet]

This patch does not yet simplify the build system.
This can be done in a follow-up bug, removing -b (and -i as well?).

[Blaze Burg]

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.

[youenn fablet]

Created attachment 265149 [details]
Patch for landing

[youenn fablet]

> 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.

[WebKit Commit Bot]

Comment on attachment 265149 [details]
Patch for landing

Clearing flags on attachment: 265149

Committed r192204: <http://trac.webkit.org/changeset/192204>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.