| Summary: | test262: test262/test/language/expressions/generators/yield-as-label.js | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||||||||||||||||||
| Component: | JavaScriptCore | Assignee: | Joseph Pecoraro <joepeck> | ||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||
| Severity: | Normal | CC: | buildbot, commit-queue, joepeck, keith_miller, mark.lam, msaboff, rniwa, saam | ||||||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||
| Bug Depends on: | 171212 | ||||||||||||||||||||||||
| Bug Blocks: | 171190 | ||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||
While the label case turned out to be easy, checking surrounding code turned out to be hard. The particular interesting case is GeneratorDeclaration versus GeneratorExpression: https://tc39.github.io/ecma262/#sec-generator-function-definitions 14.4 Generator Function Definitions > GeneratorMethod[Yield, Await]: > * PropertyName[?Yield, ?Await] ( UniqueFormalParameters[+Yield, ~Await] ) { GeneratorBody } Here the function name 'yield' is allowed because PropertyName doesn't prevent it. > GeneratorDeclaration[Yield, Await, Default]: > function * BindingIdentifier[?Yield, ?Await] ( FormalParameters[+Yield, ~Await] ) { GeneratorBody } > [+Default] function * (FormalParameters[+Yield, ~Await]) { GeneratorBody } Here the function name 'yield' is allowed because BindingIdentifier[?Yield]. > GeneratorExpression: > function* BindingIdentifier[+Yield, ~Await]opt ( FormalParameters[+Yield, ~Await] ) { GeneratorBody } Here the function name 'yield' is not allowed because BindingIdentifier[+Yield]. -- In all cases: - the name yield is not allowed in parameters or GeneratorBody. - yield is not allowed in nested cases (because we would be +Yield from the start) Created attachment 307470 [details]
[PATCH] Proposed Fix
If you want I can probably split this up into 2 patches. (Can't => Cannot) and (yield).
Created attachment 307471 [details]
[PATCH] Proposed Fix
Rebaseline a few other tests.
Comment on attachment 307471 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=307471&action=review > JSTests/stress/yield-reserved-word.js:80 > +// GeneratorDeclaration inside a generator now has +Yield. > +checkClassicSyntaxError(` > +function* foo() { function* yield(){} } > +`, `SyntaxError: Cannot use the keyword 'yield' as a generator function name.`); Clearing review because I want to add more tests like this for nested functions to ensure that yield inside of a (Function / ArrowFunction / AsyncFunction) nested inside of a Generator function is allowed. I'll also use that opportunity to break this up into a couple patches to avoid the clutter it currently has. Created attachment 307579 [details]
[PATCH] Proposed Fix
Wrote far more exhaustive tests and discovered another bug! (Bug: 171048)
Comment on attachment 307579 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=307579&action=review > JSTests/stress/yield-reserved-word.js:158 > +// FIXME: <https://webkit.org/b/171048> Should not be able to use a keyword in object literal shorthand syntax > +// checkClassicSyntaxError(` > +// function *get() { var o = { yield }; } > +// `, `SyntaxError: ...); I'm going to address this here. Since I have the patch for it and it modified other related error messages. *** Bug 171048 has been marked as a duplicate of this bug. *** Created attachment 307760 [details]
[PATCH] Proposed Fix
Getting pretty bigger now. This now covers a bunch of couple issues with keywords.
Comment on attachment 307760 [details] [PATCH] Proposed Fix Attachment 307760 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/3577585 New failing tests: jsc-layout-tests.yaml/js/script-tests/parser-syntax-check.js.layout-no-cjit stress/catch-parameter-syntax.js.ftl-no-cjit-validate-sampling-profiler stress/catch-parameter-syntax.js.ftl-no-cjit-no-inline-validate stress/catch-parameter-syntax.js.ftl-no-cjit-b3o1 stress/yield-named-variable-generator.js.ftl-no-cjit-small-pool stress/yield-named-variable-generator.js.dfg-eager stress/catch-parameter-syntax.js.default jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/parser-syntax-check.js.layout-ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout stress/yield-named-variable-generator.js.ftl-no-cjit-validate-sampling-profiler stress/yield-named-variable-generator.js.no-llint stress/yield-named-variable.js.ftl-no-cjit-b3o1 stress/yield-named-variable.js.dfg-eager-no-cjit-validate stress/catch-parameter-syntax.js.no-llint stress/yield-named-variable.js.default stress/catch-parameter-syntax.js.ftl-no-cjit-small-pool stress/yield-named-variable.js.no-cjit-collect-continuously stress/yield-named-variable.js.ftl-no-cjit-no-inline-validate jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-dfg-eager-no-cjit stress/catch-parameter-syntax.js.no-cjit-validate-phases stress/catch-parameter-syntax.js.ftl-eager-no-cjit-b3o1 stress/yield-named-variable.js.ftl-no-cjit-validate-sampling-profiler stress/catch-parameter-syntax.js.ftl-eager-no-cjit stress/catch-parameter-syntax.js.no-ftl stress/yield-named-variable-generator.js.ftl-no-cjit-no-inline-validate stress/yield-named-variable.js.no-cjit-validate-phases stress/yield-named-variable.js.ftl-eager-no-cjit-b3o1 stress/yield-named-variable-generator.js.default jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-no-llint stress/yield-named-variable.js.ftl-no-cjit-small-pool stress/yield-named-variable.js.no-llint stress/yield-named-variable-generator.js.dfg-maximal-flush-validate-no-cjit stress/catch-parameter-syntax.js.ftl-no-cjit-no-put-stack-validate jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-ftl-eager-no-cjit ChakraCore.yaml/ChakraCore/test/strict/23.reservedWords_sm.js.default jsc-layout-tests.yaml/js/script-tests/parser-syntax-check.js.layout-no-ftl jsc-layout-tests.yaml/js/script-tests/parser-syntax-check.js.layout-ftl-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/parser-syntax-check.js.layout-no-llint jsc-layout-tests.yaml/js/script-tests/parser-syntax-check.js.layout-dfg-eager-no-cjit stress/catch-parameter-syntax.js.no-cjit-collect-continuously stress/yield-named-variable-generator.js.ftl-no-cjit-no-put-stack-validate jsc-layout-tests.yaml/js/script-tests/parser-syntax-check.js.layout jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-no-ftl stress/yield-named-variable-generator.js.dfg-eager-no-cjit-validate stress/yield-named-variable-generator.js.ftl-eager-no-cjit stress/yield-named-variable-generator.js.ftl-eager stress/yield-named-variable-generator.js.no-cjit-collect-continuously stress/yield-named-variable.js.dfg-eager stress/yield-named-variable-generator.js.ftl-no-cjit-b3o1 stress/catch-parameter-syntax.js.dfg-maximal-flush-validate-no-cjit stress/yield-named-variable.js.no-ftl jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-no-cjit stress/yield-named-variable-generator.js.no-ftl stress/yield-named-variable.js.ftl-eager-no-cjit stress/yield-named-variable.js.ftl-eager stress/catch-parameter-syntax.js.ftl-eager stress/catch-parameter-syntax.js.dfg-eager-no-cjit-validate stress/yield-named-variable-generator.js.ftl-eager-no-cjit-b3o1 stress/yield-named-variable.js.dfg-maximal-flush-validate-no-cjit stress/yield-named-variable-generator.js.no-cjit-validate-phases stress/catch-parameter-syntax.js.dfg-eager stress/yield-named-variable.js.ftl-no-cjit-no-put-stack-validate Comment on attachment 307760 [details]
[PATCH] Proposed Fix
Probably more rebaselines needed.
Created attachment 307878 [details]
[PATCH] Proposed Fix
Attachment 307878 [details] did not pass style-queue:
ERROR: JSTests/ChangeLog:12: Line contains tab character. [whitespace/tab] [5]
ERROR: JSTests/ChangeLog:13: Line contains tab character. [whitespace/tab] [5]
ERROR: JSTests/ChangeLog:14: Line contains tab character. [whitespace/tab] [5]
ERROR: JSTests/ChangeLog:12: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
ERROR: JSTests/ChangeLog:13: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
ERROR: JSTests/ChangeLog:14: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
ERROR: JSTests/ChangeLog:20: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 7 in 20 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 307878 [details] [PATCH] Proposed Fix Attachment 307878 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3582060 New failing tests: js/dom/reserved-words-as-property.html Created attachment 307884 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 307878 [details] [PATCH] Proposed Fix Attachment 307878 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3582172 New failing tests: js/dom/reserved-words-as-property.html Created attachment 307888 [details]
Archive of layout-test-results from ews116 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 307878 [details] [PATCH] Proposed Fix Attachment 307878 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3582141 New failing tests: js/dom/reserved-words-as-property.html Created attachment 307889 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 307893 [details]
[PATCH] Proposed Fix
Attachment 307893 [details] did not pass style-queue:
ERROR: JSTests/ChangeLog:12: Line contains tab character. [whitespace/tab] [5]
ERROR: JSTests/ChangeLog:13: Line contains tab character. [whitespace/tab] [5]
ERROR: JSTests/ChangeLog:14: Line contains tab character. [whitespace/tab] [5]
ERROR: JSTests/ChangeLog:12: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
ERROR: JSTests/ChangeLog:13: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
ERROR: JSTests/ChangeLog:14: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
ERROR: JSTests/ChangeLog:20: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 7 in 20 files
If any of these errors are false positives, please file a bug against check-webkit-style.
What the heck happened here. Commit queue somehow landed this patch but I don't see any record of it getting reviewed... I'm going to roll out and investigate what happened. Okay it appears I put what was a new version of a patch intended for this bug on bug 171191. Saam seems to have reviewed the patch on that bug but I'm not sure that was intentional given what the bug was titled and the original patch I had on it. I'm still going to roll out just to be safe. Created attachment 307958 [details]
[PATCH] Proposed Fix
Correctly putting this patch on this bug. This addresses the style bot warnings.
Comment on attachment 307958 [details] [PATCH] Proposed Fix Clearing flags on attachment: 307958 Committed r215682: <http://trac.webkit.org/changeset/215682> All reviewed patches have been landed. Closing bug. |
test262/test/language/expressions/generators/yield-as-label.js Test: <script> function* foo() { yield: 1; } </script> <script> "use strict"; yield: 1; </script> Expected: SyntaxError: Cannot use yield as a label in a general function Actual: No error. Notes: - Chrome: Uncaught SyntaxError: Unexpected token : Uncaught SyntaxError: Unexpected strict mode reserved word - Firefox: SyntaxError: yield is a reserved identifier SyntaxError: yield is a reserved identifier Spec: https://tc39.github.io/ecma262/#sec-identifiers-static-semantics-early-errors 12.1.1 Static Semantics: Early Errors > IdentifierReference : yield > BindingIdentifier : yield > LabelIdentifier : yield > > It is a Syntax Error if the code matched by this production is contained in strict mode code. > > IdentifierReference : Identifier > BindingIdentifier : Identifier > LabelIdentifier : Identifier > > It is a Syntax Error if this production has a [Yield] parameter and StringValue of Identifier is "yield". > It is a Syntax Error if this production has an [Await] parameter and StringValue of Identifier is "await". The second set of cases preventing the use of the keyword inside of generators which have the [Yield] parameter.