Bug 159461 – Generate WebCore builtin wrapper files

• Component: Tools / Tests
• Status: RESOLVED FIXED

[youenn fablet]

We should be able to generate Source/WebCore/bindings/jsWebCoreJSBuiltin* files

[youenn fablet]

Created attachment 282879 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:79:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsWrapperHeaderGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:80:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsWrapperImplementationGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:82:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsInternalsWrapperHeaderGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:83:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsInternalsWrapperImplementationGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:41:  [BuiltinsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:45:  [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:50:  [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:69:  [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:72:  [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:95:  [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:103:  [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:117:  [BuiltinsWrapperHeaderGenerator.generate_accessors] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:129:  [BuiltinsWrapperHeaderGenerator.generate_members] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:42:  [BuiltinsInternalsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:46:  [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:51:  [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:73:  [BuiltinsInternalsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:41:  [BuiltinsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:45:  [BuiltinsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:50:  [BuiltinsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:59:  [BuiltinsWrapperImplementationGenerator.generate_section_for_object] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:62:  [BuiltinsWrapperImplementationGenerator.generate_section_for_object] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:42:  [BuiltinsInternalsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:46:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:51:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:53:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_primary_header_includes' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:80:  [BuiltinsInternalsWrapperImplementationGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
Total errors found: 27 in 23 files


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

[youenn fablet]

Created attachment 282888 [details]
Trying to fix win build

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:79:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsWrapperHeaderGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:80:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsWrapperImplementationGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:82:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsInternalsWrapperHeaderGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:83:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsInternalsWrapperImplementationGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:41:  [BuiltinsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:45:  [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:50:  [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:69:  [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:72:  [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:95:  [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:103:  [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:117:  [BuiltinsWrapperHeaderGenerator.generate_accessors] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:129:  [BuiltinsWrapperHeaderGenerator.generate_members] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:42:  [BuiltinsInternalsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:46:  [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:51:  [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:73:  [BuiltinsInternalsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:41:  [BuiltinsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:45:  [BuiltinsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:50:  [BuiltinsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:59:  [BuiltinsWrapperImplementationGenerator.generate_section_for_object] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:62:  [BuiltinsWrapperImplementationGenerator.generate_section_for_object] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:42:  [BuiltinsInternalsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:46:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:51:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:53:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_primary_header_includes' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:80:  [BuiltinsInternalsWrapperImplementationGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
Total errors found: 27 in 23 files


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

[Alex Christensen]

Could you please document what this change will allow you to do that you couldn't do before?

[youenn fablet]

(In reply to comment #5)
> Could you please document what this change will allow you to do that you
> couldn't do before?

This change allows developers to add new JS built-in files without messing up with the wrapper files, since they are now automatically generated.

I can beef up the change log if that helps, at landing time or in a later version of the patch.

[Alex Christensen]

That's what I figured.
This patch seems good, but has way too much python for me to be an effective reviewer.

[Blaze Burg]

Comment on attachment 282888 [details]
Trying to fix win build

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

> Source/JavaScriptCore/ChangeLog:3
> +        Generating WebCore wrapper files

This line seems out of place.

> Source/JavaScriptCore/ChangeLog:9
> +

Put the "why" here.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:76
> +        lines = ["class JSDOMGlobalObject;", "", "class JSBuiltinInternalFunctions {", "public:"]

Please split across multiple newlines so it's easier to read/see indentation.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:104
> +            if conditional_guard is not None:

Please use this method (copied from Source/JavaScriptCore/inspector/scripts/codegen/generator.py).

    @staticmethod
    def wrap_with_guard(guard, text):
        return '\n'.join([
            '#if %s' % guard,
            text,
            '#endif // %s' % guard,
        ])

You can put it on the generator base class.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:116
> +            if conditional_guard is not None:

See above.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:39
> +        self.internals = [object for object in model.objects if 'internal' in object.annotations]

I would write this as a call to filter() since we don't need to extract anything from `object`.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:103
> +                lines.append("#if %s" % conditional_guard)

See above.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:116
> +                lines.append("#if %s" % conditional_guard)

See above.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:127
> +            lines.append("    " + self.member_name(object) + ".init(globalObject);")

See above.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:135
> +            if conditional_guard is not None:

See above.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:159
> +                lines.append("#if %s" % conditional_guard)

See above.

> Source/JavaScriptCore/Scripts/generate-js-builtins.py:125
> +    cli_parser.add_option("--with-wrapper-files", action="store_true", help="Produce .h/.cpp wrapper files to ease integration of the builtins.")

I would rename this to --builtins-wrappers or just --wrappers.

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result:358
> + * Copyright (c) 2015 Canon Inc. All rights reserved.

Please update the copyright block template to have Apple copyright too.

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result:413
> +          globalObject.addStaticGlobals(staticGlobals, WTF_ARRAY_LENGTH(staticGlobals));

Nit: bad indent.

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-GuardedBuiltin-Separate.js-result:231
> +#ifndef WebCoreJSBuiltins_h

Please make it use #pragma once (in this patch or another one)

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result:327
> +

Nit: extra newline.

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result:385
> +    UNUSED_PARAM(vm);

This looks wrong. Shouldn't the argument be `v`?

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result:399
> +    UNUSED_PARAM(clientData);

Can we just omit generating clientData if it's not going to be used (presumably by staticGlobals)?

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:340
> +    JSC::VM& vm;

I think this should be m_vm to be consistent.

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:504
> +    UNUSED_PARAM(visitor);

This should be in an #else .. #endif, right? You might want to bake that into the helper formatting function that wraps code in a guard.

> Source/WebCore/CMakeLists.txt:3747
> +    list(APPEND WebCore_Generated_Builtins ${DERIVED_SOURCES_WEBCORE_DIR}/${_objectName}Builtins.h)

I think the variable name style we try to stick to is to uppercase everything but the project prefix (I.e., WebCore).

> Source/WebCore/CMakeLists.txt:3758
> +    COMMAND ${PYTHON_EXECUTABLE} ${JavaScriptCore_SCRIPTS_DIR}/generate-js-builtins.py --with-wrapper-files --framework WebCore --output-directory ${DERIVED_SOURCES_WEBCORE_DIR} ${WebCore_BUILTINS_SOURCES}

Should the script itself be able to infer whether to perform the effect of --with-wrapper-files based on the target framework?

[youenn fablet]

Thanks for the review.

Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:104
> > +            if conditional_guard is not None:
> 
> Please use this method (copied from
> Source/JavaScriptCore/inspector/scripts/codegen/generator.py).
> 
>     @staticmethod
>     def wrap_with_guard(guard, text):
>         return '\n'.join([
>             '#if %s' % guard,
>             text,
>             '#endif // %s' % guard,
>         ])
> 
> You can put it on the generator base class.

OK, I'll refactor that.

Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:39
> > +        self.internals = [object for object in model.objects if 'internal' in object.annotations]
> 
> I would write this as a call to filter() since we don't need to extract
> anything from `object`.

OK

> > Source/JavaScriptCore/Scripts/generate-js-builtins.py:125
> > +    cli_parser.add_option("--with-wrapper-files", action="store_true", help="Produce .h/.cpp wrapper files to ease integration of the builtins.")
> 
> I would rename this to --builtins-wrappers or just --wrappers.

OK

> > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-GuardedBuiltin-Separate.js-result:231
> > +#ifndef WebCoreJSBuiltins_h
> 
> Please make it use #pragma once (in this patch or another one)

You might like https://bugs.webkit.org/show_bug.cgi?id=159462, if you have some spare review time...

> > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result:385
> > +    UNUSED_PARAM(vm);
> 
> This looks wrong. Shouldn't the argument be `v`?

'v' is used but some bots may complain about vm being not used anywhere.

> > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result:399
> > +    UNUSED_PARAM(clientData);
> 
> Can we just omit generating clientData if it's not going to be used
> (presumably by staticGlobals)?

In real cases, clientData is used but in #if blocks.
Some bots may be compiling without all these #if blocks and will complain of clientData being not used.

> > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:340
> > +    JSC::VM& vm;
> 
> I think this should be m_vm to be consistent.

OK

> > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:504
> > +    UNUSED_PARAM(visitor);
> 
> This should be in an #else .. #endif, right? You might want to bake that
> into the helper formatting function that wraps code in a guard.

In real generated code, we have several #if blocks.
We could add something like #if!ENABLE(...) && !ENABLE(...) but I am not sure this is with the effort.

> > Source/WebCore/CMakeLists.txt:3747
> > +    list(APPEND WebCore_Generated_Builtins ${DERIVED_SOURCES_WEBCORE_DIR}/${_objectName}Builtins.h)
> 
> I think the variable name style we try to stick to is to uppercase
> everything but the project prefix (I.e., WebCore).

OK

> > Source/WebCore/CMakeLists.txt:3758
> > +    COMMAND ${PYTHON_EXECUTABLE} ${JavaScriptCore_SCRIPTS_DIR}/generate-js-builtins.py --with-wrapper-files --framework WebCore --output-directory ${DERIVED_SOURCES_WEBCORE_DIR} ${WebCore_BUILTINS_SOURCES}
> 
> Should the script itself be able to infer whether to perform the effect of
> --with-wrapper-files based on the target framework?

I was not sure about whether adding an option or using the framework.
I went with the option to be consistent with the combined option.
But I don't really mind either way.

[youenn fablet]

Created attachment 283001 [details]
Updated according review

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:42:  [BuiltinsInternalsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:46:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:51:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:53:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_primary_header_includes' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:80:  [BuiltinsInternalsWrapperImplementationGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:79:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsWrapperHeaderGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:80:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsWrapperImplementationGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:82:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsInternalsWrapperHeaderGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:83:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsInternalsWrapperImplementationGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:41:  [BuiltinsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:45:  [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:50:  [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:67:  [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:70:  [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:93:  [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:96:  [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:105:  [BuiltinsWrapperHeaderGenerator.generate_accessors] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:112:  [BuiltinsWrapperHeaderGenerator.generate_members] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:42:  [BuiltinsInternalsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:46:  [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:51:  [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:71:  [BuiltinsInternalsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:41:  [BuiltinsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:45:  [BuiltinsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:50:  [BuiltinsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:59:  [BuiltinsWrapperImplementationGenerator.generate_section_for_object] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:62:  [BuiltinsWrapperImplementationGenerator.generate_section_for_object] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
Total errors found: 27 in 24 files


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

[youenn fablet]

(In reply to comment #10)
> Created attachment 283001 [details]
> Updated according review

Patch is updated according comments.
I did not fixed some style comments for "empty classes" as the real code does not suffer from those.

I forgot to fix the option name but will do it. --wrappers is fine by me.

As for the copyright thing, I'll address it in another patch.

[Blaze Burg]

Comment on attachment 283001 [details]
Updated according review

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

I think it's almost ready. Let's try to clean up code generation a little more.

> Source/JavaScriptCore/ChangeLog:17
> +        * Scripts/builtins/builtins_generate_wrapper_implementation.py: Added  to generate WebCoreJSBuiltins.cpp.

Nit: extra space

> Source/JavaScriptCore/ChangeLog:25
> +

"Rebaseline builtins generator test results."

> Source/WebCore/ChangeLog:10
> +        Updating build system to handle new built-in generators without modifying WebCoreJSBuiltins* files..

Nit: extra '.'

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:105
> +            lines.append(BuiltinsGenerator.wrap_with_guard(object.annotations.get('conditional'), "    " + self.member_type(object) + "& " + self.accessor_name(object) + "() { return " + self.member_name(object) + "; }"))

(Applies throughout) Please put the expression to be wrapped into a string, like so. It is a lot easier to follow than using the overloaded + operator.

accessor_body = "%s& %s() { return %s; }" % (self.member_type(object), self.accessor_name(object), self.member_name(object))
lines.append(BuiltinsGenerator.wrap_with_guard(object.annotations.get('conditional'), accessor_body)

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:112
> +            lines.append(BuiltinsGenerator.wrap_with_guard(object.annotations.get('conditional'), "    " + self.member_type(object) + " " + self.member_name(object) + ";"))

See above.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:39
> +        self.internals = filter(lambda object:  'internal' in object.annotations, model.objects)

Nit: extra space after ':'

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:109
> +        for object in self.internals:

if len(self.internals) > 0:
 ... append each ...
else:
  ... append UNUSED_PARAM ...

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:113
> +        lines.append("void JSBuiltinInternalFunctions::initialize(JSDOMGlobalObject& globalObject)\n{")

Please don't put newline literals in `lines`. I know its tedious but let's be consistent.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:115
> +            lines.append(BuiltinsGenerator.wrap_with_guard(object.annotations.get('conditional'), "    " + self.member_name(object) + ".init(globalObject);"))

Same thing here, check len(self.internals).

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:121
> +"""#define DECLARE_GLOBAL_STATIC(name)\\

Please extract this into a separate variable. In general we don't wrap function arguments to newlines in Python code.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:130
> +    UNUSED_PARAM(clientData);

I think you can check if not len(self.internals) > 0 here.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:137
> +            lines.append(BuiltinsGenerator.wrap_with_guard(object.annotations.get('conditional'), "        " + self.member_type(object) + " " + self.member_name(object) + ";"))

See comment above.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:92
> +        lines = ["    explicit JSBuiltinFunctions(JSC::VM& v)", "        : m_vm(v)"]

Needs line wrap.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:94
> +            lines.append(BuiltinsGenerator.wrap_with_guard(object.annotations.get('conditional'), "        , " + self.member_name(object) + "(&m_vm)"))

Same comments as above: extract the formatted string into a local.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:41
> +        return self.model().framework.setting('namespace') + "JSBuiltins.cpp"

I would use format strings even for this. There are very few cases where using an overloaded + is more clear.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:60
> +            header_includes.append((["WebCore"], ("WebCore", object.object_name + "Builtins.cpp")))

... I think this is an example of where it's ok: only one level of dereference, so it's not too big of an expression to see at once.

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result:236
> +    explicit JSBuiltinFunctions(JSC::VM& v)

Argument name should be 'vm'.

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result:399
> +    if (sizeof(staticGlobals))

I don't think this conditional is necessary. We can statically determine the length and add the following line or not. The same is true for clientData. In fact, maybe the generator for this method should have a separate code path if there are no internals, that just prints UNUSED_PARAM(globalObject). I don't think there is anything meaningful to do in initialize() if there are no internals.

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:501
> +#define DECLARE_GLOBAL_STATIC(name)\

Nit: should have one space between things and a trailing continuation '\'

> Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:509
> +          globalObject.addStaticGlobals(staticGlobals, WTF_ARRAY_LENGTH(staticGlobals));

This generated code can be cleaned up as noted above in the case where there are no builtins.

[youenn fablet]

Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:130
> > +    UNUSED_PARAM(clientData);
> 
> I think you can check if not len(self.internals) > 0 here.

The issue here is not in the case where there is no internals.
The issue is that all internals may be guarded and guard being disabled.
In that case, compilers will issue warnings or errors because of unused parameters.

As I sad previously, we could wrap the UNUSED_PARAM in an "#if not any guard".
But I am not sure this is worth the effort.


> > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result:399
> > +    if (sizeof(staticGlobals))
> 
> I don't think this conditional is necessary. We can statically determine the
> length and add the following line or not. The same is true for clientData.
> In fact, maybe the generator for this method should have a separate code
> path if there are no internals, that just prints UNUSED_PARAM(globalObject).
> I don't think there is anything meaningful to do in initialize() if there
> are no internals.

This is the same principle as above.
I found it a bit better to have an if statement statically-resolvable, then an UNUSED_PARAM.
But I am fine using UNUSED_PARAM(globalObject) as is currently the case in the hand-written code.

> > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:509
> > +          globalObject.addStaticGlobals(staticGlobals, WTF_ARRAY_LENGTH(staticGlobals));
> 
> This generated code can be cleaned up as noted above in the case where there
> are no builtins.

I am not sure it is worth handling the no-builtin case.
In such case, the cpp file should not even be compiled.

[Blaze Burg]

(In reply to comment #14)
> Source/JavaScriptCore/Scripts/builtins/
> builtins_generate_internals_wrapper_implementation.py:130
> > > +    UNUSED_PARAM(clientData);
> > 
> > I think you can check if not len(self.internals) > 0 here.
> 
> The issue here is not in the case where there is no internals.
> The issue is that all internals may be guarded and guard being disabled.
> In that case, compilers will issue warnings or errors because of unused
> parameters.

Ah, ok, then I guess we might always need it.

> As I sad previously, we could wrap the UNUSED_PARAM in an "#if not any
> guard".
> But I am not sure this is worth the effort.

No, that's too annoying. I agree.

> > > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result:399
> > > +    if (sizeof(staticGlobals))
> > 
> > I don't think this conditional is necessary. We can statically determine the
> > length and add the following line or not. The same is true for clientData.
> > In fact, maybe the generator for this method should have a separate code
> > path if there are no internals, that just prints UNUSED_PARAM(globalObject).
> > I don't think there is anything meaningful to do in initialize() if there
> > are no internals.
> 
> This is the same principle as above.
> I found it a bit better to have an if statement statically-resolvable, then
> an UNUSED_PARAM.
> But I am fine using UNUSED_PARAM(globalObject) as is currently the case in
> the hand-written code.

I prefer an unconditional UNUSED_PARAM.

> > > Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result:509
> > > +          globalObject.addStaticGlobals(staticGlobals, WTF_ARRAY_LENGTH(staticGlobals));
> > 
> > This generated code can be cleaned up as noted above in the case where there
> > are no builtins.
> 
> I am not sure it is worth handling the no-builtin case.
> In such case, the cpp file should not even be compiled.

Ok.

Please address other comments and send through EWS again before landing.

[Blaze Burg]

Comment on attachment 283001 [details]
Updated according review

r=me

[youenn fablet]

Created attachment 283118 [details]
Updated according comments

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:42:  [BuiltinsInternalsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:46:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:51:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:53:  [BuiltinsInternalsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_primary_header_includes' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_implementation.py:80:  [BuiltinsInternalsWrapperImplementationGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:79:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsWrapperHeaderGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:80:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsWrapperImplementationGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:82:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsInternalsWrapperHeaderGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/generate-js-builtins.py:83:  [generate_bindings_for_builtins_files] Undefined variable 'BuiltinsInternalsWrapperImplementationGenerator'  [pylint/E0602] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:41:  [BuiltinsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:45:  [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:50:  [BuiltinsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:67:  [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:70:  [BuiltinsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:95:  [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:99:  [BuiltinsWrapperHeaderGenerator.generate_constructor] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:109:  [BuiltinsWrapperHeaderGenerator.generate_accessors] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_header.py:117:  [BuiltinsWrapperHeaderGenerator.generate_members] Instance of 'BuiltinsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:42:  [BuiltinsInternalsWrapperHeaderGenerator.output_filename] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:46:  [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:51:  [BuiltinsInternalsWrapperHeaderGenerator.generate_output] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_internals_wrapper_header.py:71:  [BuiltinsInternalsWrapperHeaderGenerator.generate_secondary_header_includes] Instance of 'BuiltinsInternalsWrapperHeaderGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:41:  [BuiltinsWrapperImplementationGenerator.output_filename] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:45:  [BuiltinsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:50:  [BuiltinsWrapperImplementationGenerator.generate_output] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'generate_license' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:59:  [BuiltinsWrapperImplementationGenerator.generate_section_for_object] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_wrapper_implementation.py:62:  [BuiltinsWrapperImplementationGenerator.generate_section_for_object] Instance of 'BuiltinsWrapperImplementationGenerator' has no 'generate_includes_from_entries' member  [pylint/E1101] [5]
Total errors found: 27 in 24 files


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

[youenn fablet]

Ossy, I plan to land this patch sometimes today.
Hopefully, it should not break any of your bots that disable all use of WebCore JS built-in internals (streams & webrtc).
Just in case, please let me know if you would prefer the landing to be postponed.

[Csaba Osztrogonác]

(In reply to comment #19)
> Ossy, I plan to land this patch sometimes today.
> Hopefully, it should not break any of your bots that disable all use of
> WebCore JS built-in internals (streams & webrtc).
> Just in case, please let me know if you would prefer the landing to be
> postponed.

ALL EWS are green, so I can't see any blocker issue, feel free to land it. ;)
Don't worry about non default configurations, we can fix them later
if they had some problems.

[WebKit Commit Bot]

Comment on attachment 283118 [details]
Updated according comments

Clearing flags on attachment: 283118

Committed r202975: <http://trac.webkit.org/changeset/202975>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.