Builtins generator should put WebCore-only wrappers in the per-builtin header

• Component: Tools / Tests
• Status: RESOLVED FIXED

[BJ Burg]

No point in making this a separate header.

[BJ Burg]

Created attachment 263989 [details]
Proposed Fix

[WebKit Commit Bot]

Attachment 263989 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_header.py:71:  [BuiltinsSeparateHeaderGenerator.generate_output] Instance of 'BuiltinsSeparateHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[BJ Burg]

Created attachment 263991 [details]
Proposed Fix

[WebKit Commit Bot]

Attachment 263991 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_header.py:71:  [BuiltinsSeparateHeaderGenerator.generate_output] Instance of 'BuiltinsSeparateHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Comment on attachment 263991 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=263991&action=review

Thanks for moving that forward. Obviously, I like it.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_header.py:86
> +                ("WebCore", "%sBuiltins.h" % self.object.object_name),

We probably do not need that include.

> Source/JavaScriptCore/Scripts/builtins/builtins_templates.py:138
>      """class ${objectName}BuiltinsWrapper : private JSC::WeakHandleOwner {

${objectName}BuiltinsWrapper could be renamed ${objectName}Builtins later on.
Hopefully, only Source/WebCore/bindings/js/WebCoreJSBuiltins.h and WebCoreJSBuiltinInternals.h should need to be updated.

We might also want to clean a little bit  ${objectName}BuiltinFunctions in the future.

[BJ Burg]

Comment on attachment 263991 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=263991&action=review

>> Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_header.py:86
>> +                ("WebCore", "%sBuiltins.h" % self.object.object_name),
> 
> We probably do not need that include.

OK.

>> Source/JavaScriptCore/Scripts/builtins/builtins_templates.py:138
>>      """class ${objectName}BuiltinsWrapper : private JSC::WeakHandleOwner {
> 
> ${objectName}BuiltinsWrapper could be renamed ${objectName}Builtins later on.
> Hopefully, only Source/WebCore/bindings/js/WebCoreJSBuiltins.h and WebCoreJSBuiltinInternals.h should need to be updated.
> 
> We might also want to clean a little bit  ${objectName}BuiltinFunctions in the future.

Sure, this can be a followup.

[BJ Burg]

Youenn, can you review this patch? I don't want it to get stale.

[BJ Burg]

Committed r191842: <http://trac.webkit.org/changeset/191842>