Bug 158893

Summary: It should be easy to add a private global helper function for builtins
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 158960    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
mark.lam: review+, commit-queue: commit-queue-
Benchmark results
none
Patch for landing
none
Patch for landing v2 none

Keith Miller
Reported 2016-06-17 16:55:51 PDT
It should be easy to add a private global helper function for builtins
Attachments
Patch (117.74 KB, patch)
2016-06-20 09:50 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch (118.20 KB, patch)
2016-06-20 10:52 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch (120.13 KB, patch)
2016-06-20 11:13 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch (165.61 KB, patch)
2016-06-20 12:52 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch (172.47 KB, patch)
2016-06-20 13:58 PDT, Keith Miller 		mark.lam: review+
commit-queue: commit-queue-
DetailsFormatted DiffDiff
Benchmark results (78.46 KB, text/plain)
2016-06-20 14:07 PDT, Keith Miller 		no flags
Details
Patch for landing (158.40 KB, patch)
2016-06-20 15:10 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch for landing v2 (163.42 KB, patch)
2016-06-21 10:24 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2016-06-20 09:50:11 PDT
Created attachment 281651 [details] Patch
Keith Miller
Comment 2 2016-06-20 10:52:23 PDT
Created attachment 281654 [details] Patch
WebKit Commit Bot
Comment 3 2016-06-20 10:54:05 PDT
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
Comment 4 2016-06-20 10:54:19 PDT
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
Comment 5 2016-06-20 11:13:58 PDT
Created attachment 281656 [details] Patch
Mark Lam
Comment 6 2016-06-20 11:14:36 PDT
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
Comment 7 2016-06-20 11:15:44 PDT
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 8 2016-06-20 11:34:38 PDT
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
Comment 9 2016-06-20 11:50:51 PDT
(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
Comment 10 2016-06-20 11:51:26 PDT
> 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
Comment 11 2016-06-20 12:52:24 PDT
Created attachment 281666 [details] Patch
WebKit Commit Bot
Comment 12 2016-06-20 12:53:18 PDT
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
Comment 13 2016-06-20 13:58:11 PDT
Created attachment 281674 [details] Patch
WebKit Commit Bot
Comment 14 2016-06-20 14:00:42 PDT
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
Comment 15 2016-06-20 14:07:44 PDT
Created attachment 281676 [details] Benchmark results
Mark Lam
Comment 16 2016-06-20 14:34:32 PDT
Comment on attachment 281674 [details] Patch r=me
WebKit Commit Bot
Comment 17 2016-06-20 14:47:22 PDT
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
Comment 18 2016-06-20 15:10:09 PDT
Created attachment 281680 [details] Patch for landing
WebKit Commit Bot
Comment 19 2016-06-20 15:38:55 PDT
Comment on attachment 281680 [details] Patch for landing Clearing flags on attachment: 281680 Committed r202248: <http://trac.webkit.org/changeset/202248>
WebKit Commit Bot
Comment 20 2016-06-20 15:38:59 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 21 2016-06-20 17:24:12 PDT
Re-opened since this is blocked by bug 158960
Keith Miller
Comment 22 2016-06-21 10:24:12 PDT
Created attachment 281757 [details] Patch for landing v2
WebKit Commit Bot
Comment 23 2016-06-21 10:54:44 PDT
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
Comment 24 2016-06-21 10:54:49 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.