patch.
Created attachment 263985 [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`)
Attachment 263985 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_implementation.py:64: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_primary_header_includes' member [pylint/E1101] [5] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Just a quick note that I used @optional in streams built-in js, but maybe this is a poor name. WebIDL is using "Conditional" keyword. Generated code is using "ENABLE" macro.
(In reply to comment #4) > Just a quick note that I used @optional in streams built-in js, but maybe > this is a poor name. > WebIDL is using "Conditional" keyword. > Generated code is using "ENABLE" macro. In other code generators, it is along the lines of // @conditional=ENABLE(STREAMS) so that files can be conditionalized on PLATFORM(), USE(), etc. It is easy to change this.
(In reply to comment #5) > (In reply to comment #4) > > Just a quick note that I used @optional in streams built-in js, but maybe > > this is a poor name. > > WebIDL is using "Conditional" keyword. > > Generated code is using "ENABLE" macro. > > In other code generators, it is along the lines of > > // @conditional=ENABLE(STREAMS) > > so that files can be conditionalized on PLATFORM(), USE(), etc. > It is easy to change this. Sounds good to me
Comment on attachment 263985 [details] Patch r=me, annotation looks nice ;)
(In reply to comment #7) > Comment on attachment 263985 [details] > Patch > > r=me, annotation looks nice ;) Before landing, as discussed above, I'll change @optional to @conditional and include ENABLE() in all the existing guards. I'll put up another patch for EWS to catch any mistakes.
(In reply to comment #8) > (In reply to comment #7) > > Comment on attachment 263985 [details] > > Patch > > > > r=me, annotation looks nice ;) > > Before landing, as discussed above, I'll change @optional to @conditional > and include ENABLE() in all the existing guards. I'll put up another patch > for EWS to catch any mistakes. @conditional looks preferable! So your revised patch will include the existing stream JS code, right? (replacing @optional to @conditional)
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > Comment on attachment 263985 [details] > > > Patch > > > > > > r=me, annotation looks nice ;) > > > > Before landing, as discussed above, I'll change @optional to @conditional > > and include ENABLE() in all the existing guards. I'll put up another patch > > for EWS to catch any mistakes. > > @conditional looks preferable! > So your revised patch will include the existing stream JS code, right? > (replacing @optional to @conditional) Yes.
Created attachment 264119 [details] Proposed Fix
Attachment 264119 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_implementation.py:64: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_primary_header_includes' member [pylint/E1101] [5] Total errors found: 1 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
LGTM. Just wondering whether we also want to test "@conditional=ENABLE(STREAMS_API) || USE(CF)" annotation?
(In reply to comment #13) > LGTM. > Just wondering whether we also want to test > "@conditional=ENABLE(STREAMS_API) || USE(CF)" annotation? Sure. The regex should accept any non '=' character as the annotation's value. But would be good to verify.
Created attachment 264197 [details] Proposed Fix
Attachment 264197 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/Scripts/builtins/builtins_generate_separate_implementation.py:64: [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_primary_header_includes' member [pylint/E1101] [5] Total errors found: 1 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 264197 [details] Proposed Fix Setting cq+ with additional changes as discussed in bug.
Comment on attachment 264197 [details] Proposed Fix Rejecting attachment 264197 [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-02', 'validate-changelog', '--check-oops', '--non-interactive', 264197, '--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/349230
Committed r191687: <http://trac.webkit.org/changeset/191687>