Summary: | create_hash_table should know whether a function is JSBuiltin or not. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
Component: | Tools / Tests | Assignee: | youenn fablet <youennf> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bburg, commit-queue, darin, lforschler | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=150482 | ||||||||
Attachments: |
|
Description
youenn fablet
2015-11-09 02:54:18 PST
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. |