Skip to content

Commit

Permalink
Bug 1685482 - Part 2: Disallow identifiers named "async" in for-of lo…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
anba committed Jan 14, 2021
1 parent 35f5586 commit f742f7e
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 8 deletions.
2 changes: 1 addition & 1 deletion js/public/friend/ErrorNumbers.msg
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ MSG_DEF(JSMSG_BAD_DUP_ARGS, 0, JSEXN_SYNTAXERR, "duplicate argument n
MSG_DEF(JSMSG_BAD_FOR_EACH_LOOP, 0, JSEXN_SYNTAXERR, "invalid for each loop")
MSG_DEF(JSMSG_BAD_FOR_LEFTSIDE, 0, JSEXN_SYNTAXERR, "invalid for-in/of left-hand side")
MSG_DEF(JSMSG_LEXICAL_DECL_DEFINES_LET,0, JSEXN_SYNTAXERR, "a lexical declaration can't define a 'let' binding")
MSG_DEF(JSMSG_LET_STARTING_FOROF_LHS, 0, JSEXN_SYNTAXERR, "an expression X in 'for (X of Y)' must not start with 'let'")
MSG_DEF(JSMSG_BAD_STARTING_FOROF_LHS, 1, JSEXN_SYNTAXERR, "an expression X in 'for (X of Y)' must not start with '{0}'")
MSG_DEF(JSMSG_BAD_INCOP_OPERAND, 0, JSEXN_SYNTAXERR, "invalid increment/decrement operand")
MSG_DEF(JSMSG_BAD_LEFTSIDE_OF_ASS, 0, JSEXN_SYNTAXERR, "invalid assignment left-hand side")
MSG_DEF(JSMSG_BAD_METHOD_DEF, 0, JSEXN_SYNTAXERR, "bad method definition")
Expand Down
35 changes: 30 additions & 5 deletions js/src/frontend/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6120,8 +6120,9 @@ bool GeneralParser<ParseHandler, Unit>::matchInOrOf(bool* isForInp,

template <class ParseHandler, typename Unit>
bool GeneralParser<ParseHandler, Unit>::forHeadStart(
YieldHandling yieldHandling, ParseNodeKind* forHeadKind,
Node* forInitialPart, Maybe<ParseContext::Scope>& forLoopLexicalScope,
YieldHandling yieldHandling, IteratorKind iterKind,
ParseNodeKind* forHeadKind, Node* forInitialPart,
Maybe<ParseContext::Scope>& forLoopLexicalScope,
Node* forInOrOfExpression) {
MOZ_ASSERT(anyChars.isCurrentTokenType(TokenKind::LeftParen));

Expand Down Expand Up @@ -6155,8 +6156,13 @@ bool GeneralParser<ParseHandler, Unit>::forHeadStart(
// For-in loop backwards compatibility requires that |let| starting a
// for-loop that's not a (new to ES6) for-of loop, in non-strict mode code,
// parse as an identifier. (|let| in for-of is always a declaration.)
//
// For-of loops can'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 [])|.
bool parsingLexicalDeclaration = false;
bool letIsIdentifier = false;
bool startsWithForOf = false;
if (tt == TokenKind::Const) {
parsingLexicalDeclaration = true;
tokenStream.consumeKnownToken(tt, TokenStream::SlashIsRegExp);
Expand All @@ -6177,6 +6183,18 @@ bool GeneralParser<ParseHandler, Unit>::forHeadStart(
anyChars.ungetToken();
letIsIdentifier = true;
}
} else if (tt == TokenKind::Async && iterKind == IteratorKind::Sync) {
tokenStream.consumeKnownToken(TokenKind::Async, TokenStream::SlashIsRegExp);

TokenKind next;
if (!tokenStream.peekToken(&next)) {
return false;
}

if (next == TokenKind::Of) {
startsWithForOf = true;
}
anyChars.ungetToken();
}

if (parsingLexicalDeclaration) {
Expand Down Expand Up @@ -6242,7 +6260,14 @@ bool GeneralParser<ParseHandler, Unit>::forHeadStart(
//
// See ES6 13.7.
if (isForOf && letIsIdentifier) {
errorAt(exprOffset, JSMSG_LET_STARTING_FOROF_LHS);
errorAt(exprOffset, JSMSG_BAD_STARTING_FOROF_LHS, "let");
return false;
}

// In a for-of loop, the LeftHandSideExpression isn't allowed to be an
// identifier named "async" per the [lookahead ≠ async of] restriction.
if (isForOf && startsWithForOf) {
errorAt(exprOffset, JSMSG_BAD_STARTING_FOROF_LHS, "async of");
return false;
}

Expand Down Expand Up @@ -6356,8 +6381,8 @@ typename ParseHandler::Node GeneralParser<ParseHandler, Unit>::forStatement(
//
// In either case the subsequent token can be consistently accessed using
// TokenStream::SlashIsDiv semantics.
if (!forHeadStart(yieldHandling, &headKind, &startNode, forLoopLexicalScope,
&iteratedExpr)) {
if (!forHeadStart(yieldHandling, iterKind, &headKind, &startNode,
forLoopLexicalScope, &iteratedExpr)) {
return null();
}

Expand Down
5 changes: 3 additions & 2 deletions js/src/frontend/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@
#include "frontend/ErrorReporter.h"
#include "frontend/FullParseHandler.h"
#include "frontend/FunctionSyntaxKind.h" // FunctionSyntaxKind
#include "frontend/IteratorKind.h"
#include "frontend/NameAnalysisTypes.h"
#include "frontend/NameCollections.h"
#include "frontend/ParseContext.h"
Expand Down Expand Up @@ -1043,8 +1044,8 @@ class MOZ_STACK_CLASS GeneralParser : public PerHandlerParser<ParseHandler> {
BinaryNodeType whileStatement(YieldHandling yieldHandling);

Node forStatement(YieldHandling yieldHandling);
bool forHeadStart(YieldHandling yieldHandling, ParseNodeKind* forHeadKind,
Node* forInitialPart,
bool forHeadStart(YieldHandling yieldHandling, IteratorKind iterKind,
ParseNodeKind* forHeadKind, Node* forInitialPart,
mozilla::Maybe<ParseContext::Scope>& forLetImpliedScope,
Node* forInOrOfExpression);
Node expressionAfterForInOrOf(ParseNodeKind forHeadKind,
Expand Down
116 changes: 116 additions & 0 deletions js/src/tests/non262/statements/for-of-async-of-starting-lhs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
if (typeof getRealmConfiguration === "undefined") {
var getRealmConfiguration = SpecialPowers.Cu.getJSTestingFunctions().getRealmConfiguration;
}

const IsTopLevelAwaitEnabled = getRealmConfiguration().topLevelAwait;

const AsyncFunction = async function(){}.constructor;

function assertNoError(f, msg) {
try {
f();
} catch (e) {
assertEq(true, false, `${msg}: ${e}`);
}
}

function assertSyntaxError(code) {
assertThrowsInstanceOf(function () { Function(code); }, SyntaxError, "Function:" + code);
assertThrowsInstanceOf(function () { AsyncFunction(code); }, SyntaxError, "AsyncFunction:" + code);

if (typeof parseModule === "function") {
assertThrowsInstanceOf(function () { parseModule(code); }, SyntaxError, "Module:" + code);
}
}

function assertNoSyntaxError(code) {
assertNoError(function () { Function(code); }, "Function:" + code);
assertNoError(function () { AsyncFunction(code); }, "AsyncFunction:" + code);

if (typeof parseModule === "function") {
assertNoError(function () { parseModule(code); }, "Module:" + code);
}
}

function assertNoSyntaxErrorAsyncContext(code) {
assertNoError(function () { AsyncFunction(code); }, "AsyncFunction:" + code);

if (typeof parseModule === "function" && IsTopLevelAwaitEnabled) {
assertNoError(function () { parseModule(code); }, "Module:" + code);
}
}

const invalidTestCases = [
// for-in loop: LHS can't start with an async arrow function.
"for ( async of => {} in [] ) ;",
"for ( async o\\u0066 => {} in [] ) ;",

// for-of loop: LHS can't start with an async arrow function.
"for ( async of => {} of [] ) ;",
"for ( async o\\u0066 => {} of [] ) ;",

// for-of loop: LHS can't start with an identifier named "async".
"for ( async of [] ) ;",

// for-await-of loop: LHS can't start with an async arrow function.
"for await ( async of => {} of [] ) ;",
"for await ( async o\\u0066 => {} of [] ) ;",
];

for (let source of invalidTestCases) {
assertSyntaxError(source);

// Also test when the tokens are separated by newline characters.
assertSyntaxError(source.split(" ").join("\n"));
}

// for-loop: async arrow functions are allowed in C-style for-loops.
assertNoSyntaxError("for ( async of => {} ; ; ) ;")

const validTestCases = [
// for-loop: LHS can start with an identifier named "async".
"for ( async ; ; ) ;",
"for ( \\u0061sync ; ; ) ;",

// for-in loop: LHS can start with an identifier named "async".
"for ( async in [] ) ;",
"for ( \\u0061sync in [] ) ;",

// for-in loop: LHS can start with an property assignment starting with "async".
"for ( async . prop in [] ) ;",
"for ( async [ 0 ] in [] ) ;",

// for-of loop: LHS can start with an identifier named "async" when escape characters are used.
"for ( \\u0061sync of [] ) ;",

// for-of loop: LHS can start with an property assignment starting with "async".
"for ( async . prop of [] ) ;",
"for ( async [ 0 ] of [] ) ;",
];

for (let source of validTestCases) {
assertNoSyntaxError(source);

// Also test when the tokens are separated by newline characters.
assertNoSyntaxError(source.split(" ").join("\n"));
}

const validTestCasesAsync = [
// for-await-of loop: LHS can start with an identifier named "async".
// "for await ( async of [] ) ;",
"for await ( \\u0061sync of [] ) ;",

// for-await-of loop: LHS can start with an property assignment starting with "async".
"for await ( async . prop of [] ) ;",
"for await ( async [ 0 ] of [] ) ;",
];

for (let source of validTestCasesAsync) {
assertNoSyntaxErrorAsyncContext(source);

// Also test when the tokens are separated by newline characters.
assertNoSyntaxErrorAsyncContext(source.split(" ").join("\n"));
}

if (typeof reportCompare === "function")
reportCompare(true, true);

0 comments on commit f742f7e

Please sign in to comment.