Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grammar ambiguity in for ( async of #2034

Closed
bakkot opened this issue Jun 5, 2020 · 4 comments · Fixed by #2256
Closed

Grammar ambiguity in for ( async of #2034

bakkot opened this issue Jun 5, 2020 · 4 comments · Fixed by #2256
Labels
has consensus This has committee consensus.

Comments

@bakkot
Copy link
Contributor

bakkot commented Jun 5, 2020

The token sequence for ( async of can begin two different parses: either for (async of => {};;); or for (async of foo);.

@waldemarhorwat raised this issue at the December 2019 meeting. During that discussion we decided to resolve this by using a lookahead restriction to disallow for-of loops which start with the token async.

@michaelficarra michaelficarra added the has consensus This has committee consensus. label Jun 5, 2020
@Fenzland
Copy link

Fenzland commented Jun 8, 2020

why allow keyword like async, of and even undefined as variable name?

@ljharb
Copy link
Member

ljharb commented Jun 8, 2020

Because they’ve always been allowed, and it wouldn’t be web compatible to disallow them now.

pull bot pushed a commit to ashu8912/v8 that referenced this issue Feb 10, 2021
Implements tc39/ecma262#2034

Currently the token sequence `for (async of` is ambiguous. It can be the
prefix for either `(async of => {};;);` or `for (async of foo);`. This
CL disallows the token sequence.

Note that `for await (async of` is still allowed, since there is no
C-style `for await (;;)`, and thus no ambiguity.

Bug: v8:11412
Change-Id: I3fede83a69420996baa2bc8b6c1cff000535d990
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2683221
Commit-Queue: Shu-yu Guo <[email protected]>
Reviewed-by: Marja Hölttä <[email protected]>
Cr-Commit-Position: refs/heads/master@{#72607}
@strager
Copy link

strager commented Feb 19, 2021

For reference, the following code prints 42 in Node.js version v12.20.0 and in Chromium version 88.0.4324.150, but emits a SyntaxError in Firefox 84.0.1 and in macOS Safari version 12.1.2 (14607.3.9):

var async;
for (async of [42]) {}
console.log(async);

strager added a commit to quick-lint/quick-lint-js that referenced this issue Feb 19, 2021
For a while, the code 'for (async of []);' is ambiguous in the JS
standard. It's going to be made unambiguously a SyntaxError. Make
quick-lint-js report an error for this code, treating 'async' as a
variable name, and gracefully continue.

Many other forms, such as 'for (async.prop of []);' and
'for (async of => {}; cond; update);' are unambiguous, and shouldn't
result in any errors. This commit adds tests for these similar cases.

Discussion on the ambiguity and its resolution:
tc39/ecma262#2034
strager added a commit to quick-lint/quick-lint-js that referenced this issue Feb 20, 2021
For a while, the code 'for (async of []);' is ambiguous in the JS
standard. It's going to be made unambiguously a SyntaxError. Make
quick-lint-js report an error for this code, treating 'async' as a
variable name, and gracefully continue.

Many other forms, such as 'for (async.prop of []);' and
'for (async of => {}; cond; update);' are unambiguous, and shouldn't
result in any errors. This commit adds tests for these similar cases.

Discussion on the ambiguity and its resolution:
tc39/ecma262#2034
strager added a commit to quick-lint/quick-lint-js that referenced this issue Feb 20, 2021
For a while, the code 'for (async of []);' is ambiguous in the JS
standard. It's going to be made unambiguously a SyntaxError. Make
quick-lint-js report an error for this code, treating 'async' as a
variable name, and gracefully continue.

Many other forms, such as 'for (async.prop of []);' and
'for (async of => {}; cond; update);' are unambiguous, and shouldn't
result in any errors. This commit adds tests for these similar cases.

Discussion on the ambiguity and its resolution:
tc39/ecma262#2034
@syg
Copy link
Contributor

syg commented Feb 22, 2021

@strager A fix was recently merged (https://chromium-review.googlesource.com/c/v8/v8/+/2683221), and Chrome 90 / V8 9.0 will correctly disallow for ( async of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants