Bug 170979 - test262: test262/test/language/expressions/generators/yield-as-label.js
Summary: test262: test262/test/language/expressions/generators/yield-as-label.js
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
: 171048 (view as bug list)
Depends on: 171212
Blocks: 171190
  Show dependency treegraph
 
Reported: 2017-04-18 23:12 PDT by Joseph Pecoraro
Modified: 2017-04-24 09:06 PDT (History)
8 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (68.65 KB, patch)
2017-04-19 00:59 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (70.19 KB, patch)
2017-04-19 01:27 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (74.53 KB, patch)
2017-04-20 01:38 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (94.62 KB, patch)
2017-04-21 11:43 PDT, Joseph Pecoraro
joepeck: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Proposed Fix (113.92 KB, patch)
2017-04-21 21:06 PDT, Joseph Pecoraro
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (947.76 KB, application/zip)
2017-04-21 22:17 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-elcapitan (1.76 MB, application/zip)
2017-04-21 22:42 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (27.23 MB, application/zip)
2017-04-21 22:54 PDT, Build Bot
no flags Details
[PATCH] Proposed Fix (117.72 KB, patch)
2017-04-21 23:19 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (117.73 KB, patch)
2017-04-23 22:51 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2017-04-18 23:12:17 PDT
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.
Comment 1 Joseph Pecoraro 2017-04-19 00:38:31 PDT
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)
Comment 2 Joseph Pecoraro 2017-04-19 00:59:54 PDT
Created attachment 307470 [details]
[PATCH] Proposed Fix

If you want I can probably split this up into 2 patches. (Can't => Cannot) and (yield).
Comment 3 Joseph Pecoraro 2017-04-19 01:27:34 PDT
Created attachment 307471 [details]
[PATCH] Proposed Fix

Rebaseline a few other tests.
Comment 4 Joseph Pecoraro 2017-04-19 19:43:09 PDT
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.
Comment 5 Joseph Pecoraro 2017-04-20 01:38:47 PDT
Created attachment 307579 [details]
[PATCH] Proposed Fix

Wrote far more exhaustive tests and discovered another bug! (Bug: 171048)
Comment 6 Joseph Pecoraro 2017-04-21 11:34:10 PDT
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.
Comment 7 Joseph Pecoraro 2017-04-21 11:34:55 PDT
*** Bug 171048 has been marked as a duplicate of this bug. ***
Comment 8 Joseph Pecoraro 2017-04-21 11:43:49 PDT
Created attachment 307760 [details]
[PATCH] Proposed Fix

Getting pretty bigger now. This now covers a bunch of couple issues with keywords.
Comment 9 Build Bot 2017-04-21 12:20:55 PDT
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 10 Joseph Pecoraro 2017-04-21 12:24:15 PDT
Comment on attachment 307760 [details]
[PATCH] Proposed Fix

Probably more rebaselines needed.
Comment 11 Joseph Pecoraro 2017-04-21 21:06:31 PDT
Created attachment 307878 [details]
[PATCH] Proposed Fix
Comment 12 Build Bot 2017-04-21 21:07:52 PDT
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 13 Build Bot 2017-04-21 22:17:29 PDT
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
Comment 14 Build Bot 2017-04-21 22:17:30 PDT
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 15 Build Bot 2017-04-21 22:42:05 PDT
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
Comment 16 Build Bot 2017-04-21 22:42:07 PDT
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 17 Build Bot 2017-04-21 22:54:33 PDT
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
Comment 18 Build Bot 2017-04-21 22:54:35 PDT
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
Comment 19 Joseph Pecoraro 2017-04-21 23:19:22 PDT
Created attachment 307893 [details]
[PATCH] Proposed Fix
Comment 20 Build Bot 2017-04-21 23:21:05 PDT
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.
Comment 21 Joseph Pecoraro 2017-04-23 22:41:22 PDT
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.
Comment 22 Joseph Pecoraro 2017-04-23 22:45:42 PDT
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.
Comment 23 Joseph Pecoraro 2017-04-23 22:51:54 PDT
Created attachment 307958 [details]
[PATCH] Proposed Fix

Correctly putting this patch on this bug. This addresses the style bot warnings.
Comment 24 WebKit Commit Bot 2017-04-24 09:06:38 PDT
Comment on attachment 307958 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 307958

Committed r215682: <http://trac.webkit.org/changeset/215682>
Comment 25 WebKit Commit Bot 2017-04-24 09:06:40 PDT
All reviewed patches have been landed.  Closing bug.