Normative: forbid for-of loops with variable named async #2256
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bakkot
added
has consensus
devsnek
reviewed
Dec 26, 2020
michaelficarra
approved these changes
Jan 6, 2021
syg
approved these changes
Jan 7, 2021
bakkot
added
ready to merge
|
(Actually I guess we should hold off on this until there are tests.) |
bakkot
added
has test262 tests
and removed
needs test262 tests
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Jan 19, 2021
…ops. r=yulia
`for-of` loops mustn't start with the token sequence `async of`, because that
leads to a shift-reduce conflict when parsing `for (async of => {};;)` or
`for (async of [])`. This restriction doesn't apply to `for-await-of` loops,
because `async` in `for await (async of ...)` is always parsed as an identifier.
Parsing `for (async of ...)` already results in a SyntaxError, but that happens
because `assignExpr()` always tries to parse the sequence `async [no LineTerminator] of`
as the start of an async arrow function. That means `forHeadStart()` still needs
to handle the case when `async` and `of` are separated by a line terminator.
Part 3 will update the parser to allow `for await (async of ...)`.
Spec change: tc39/ecma262#2256
Depends on D100994
Differential Revision: https://phabricator.services.mozilla.com/D100995
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Jan 25, 2021
…ops. r=yulia
`for-of` loops mustn't start with the token sequence `async of`, because that
leads to a shift-reduce conflict when parsing `for (async of => {};;)` or
`for (async of [])`. This restriction doesn't apply to `for-await-of` loops,
because `async` in `for await (async of ...)` is always parsed as an identifier.
Parsing `for (async of ...)` already results in a SyntaxError, but that happens
because `assignExpr()` always tries to parse the sequence `async [no LineTerminator] of`
as the start of an async arrow function. That means `forHeadStart()` still needs
to handle the case when `async` and `of` are separated by a line terminator.
Part 3 will update the parser to allow `for await (async of ...)`.
Spec change: tc39/ecma262#2256
Depends on D100994
Differential Revision: https://phabricator.services.mozilla.com/D100995
UltraBlame original commit: c260009506931dfe9e8c75906cd200e3ff687b79
jamienicol
pushed a commit
to jamienicol/gecko
that referenced
this pull request
Jan 29, 2021
…ops. r=yulia
`for-of` loops mustn't start with the token sequence `async of`, because that
leads to a shift-reduce conflict when parsing `for (async of => {};;)` or
`for (async of [])`. This restriction doesn't apply to `for-await-of` loops,
because `async` in `for await (async of ...)` is always parsed as an identifier.
Parsing `for (async of ...)` already results in a SyntaxError, but that happens
because `assignExpr()` always tries to parse the sequence `async [no LineTerminator] of`
as the start of an async arrow function. That means `forHeadStart()` still needs
to handle the case when `async` and `of` are separated by a line terminator.
Part 3 will update the parser to allow `for await (async of ...)`.
Spec change: tc39/ecma262#2256
Depends on D100994
Differential Revision: https://phabricator.services.mozilla.com/D100995
bakkot
added
es2021
ready to merge
ljharb
removed
the
ready to merge
bakkot
added
the
ready to merge
1 task
This was referenced May 21, 2021
This was referenced May 30, 2021
This was referenced Jun 6, 2021
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2034, raised by @waldemarhorwat a year ago (sorry for the delay). See the linked slides for context. This makes
for (async of [1,2,3]);a syntax error, as it is in all major engines except V8.We said at the time that we could resolve this with a single token of lookahead, presumably by just preventing for-of loops where the first token is
async. Unfortunately,let async = {x: 0}; for (async.x of [1,2,3]);is a legal program with no ambiguities and we probably shouldn't forbid it. So I've used a two token lookahead here. We use two tokens of lookahead in some other contexts; I don't think there's any issue with extending that to this case, especially now that #2254 has fixed up the definition.We should add a few tests asserting thatEdit: done.let async = {x: 0}; for (async.x of [1,2,3]);andfor (async of => 0; false;);are legal andfor (async of [1,2,3]);is a syntax error.