Bug 158893 – It should be easy to add a private global helper function for builtins

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

It should be easy to add a private global helper function for builtins

[Keith Miller]

Created attachment 281651 [details]
Patch

[Keith Miller]

Created attachment 281654 [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`)

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:157:  [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:162:  [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
Total errors found: 2 in 37 files


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

[Keith Miller]

Created attachment 281656 [details]
Patch

[Mark Lam]

Comment on attachment 281654 [details]
Patch

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

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:3
> +# Copyright (c) 2014, 2015, 2016 Apple Inc. All rights reserved.

You can abbreviate this as 2014-2016.

> Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_header.py:3
> +# Copyright (c) 2014, 2015, 2016 Apple Inc. All rights reserved.

Ditto.

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:157:  [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:162:  [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
Total errors found: 2 in 38 files


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

[Blaze Burg]

Comment on attachment 281656 [details]
Patch

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

Generator changes look good, except I would have expected some tests to need rebaselining. Did you do run-builtins-generator-tests --reset-results?

> Source/JavaScriptCore/ChangeLog:13
> +        on the glabol object. The name of the property will be the same as

typo: 'global'

[Keith Miller]

(In reply to comment #8)
> Comment on attachment 281656 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281656&action=review
> 
> Generator changes look good, except I would have expected some tests to need
> rebaselining. Did you do run-builtins-generator-tests --reset-results?

Nope I totally missed that. It looks like they have not been rebaselined in a long time . Since there are changes that don't appear to be part of my patch. I'll include the rebaselined tests in my build fix.

[Keith Miller]

> You can abbreviate this as 2014-2016.

Fixed.

> > Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_header.py:3
> > +# Copyright (c) 2014, 2015, 2016 Apple Inc. All rights reserved.
> 
> Ditto.

It looks like there shouldn't even be a copyright change in that file. I'll remove.

[Keith Miller]

Created attachment 281666 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:157:  [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:162:  [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
Total errors found: 2 in 50 files


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

[Keith Miller]

Created attachment 281674 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:157:  [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_combined_header.py:162:  [BuiltinsCombinedHeaderGenerator.generate_section_for_global_private_code_name_macro] Instance of 'BuiltinsCombinedHeaderGenerator' has no 'model' member  [pylint/E1101] [5]
Total errors found: 3 in 52 files


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

[Keith Miller]

Created attachment 281676 [details]
Benchmark results

[Mark Lam]

Comment on attachment 281674 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 281674 [details]
Patch

Rejecting attachment 281674 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 281674, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/1538359

[Keith Miller]

Created attachment 281680 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 281680 [details]
Patch for landing

Clearing flags on attachment: 281680

Committed r202248: <http://trac.webkit.org/changeset/202248>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 158960

[Keith Miller]

Created attachment 281757 [details]
Patch for landing v2

[WebKit Commit Bot]

Comment on attachment 281757 [details]
Patch for landing v2

Clearing flags on attachment: 281757

Committed r202280: <http://trac.webkit.org/changeset/202280>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.