1232674 - for-of with let as identifier parses as for-head and skips "of"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jay Freeman (saurik)]

js> let = {}; for (let.x of;;);

This is an infinite loop, while it should clearly be a SyntaxError ;P.

In Parser::forStatement(), peekShouldParseLetDeclaration() is used to determine if a "let" at the front of a for loop should be considered an identifier, and is stored into letIsIdentifier. In this case, an expression is then parsed using expr(), leaving the input on the following token (presumably "in", "of", or ";").

After this first segment is parsed, matchInOrOf() is used to determine if the next token is "in" or "of". If there is such a token, then that token is consumed. If there is not, then the token (which is presumably a ";") is returned to the input stream using ungetToken() and will be checked by the later code for for-head.

If there was an "of", then the ECMAScript6 grammar required that, before the expression was parsed (as this restriction is not as severe if the token was "in" or ";"), the expression could not begin with "let". This is checked using "else if (isForOf && !letIsIdentifier)". In this case, headKind defaults to PNK_FORHEAD.

The code for PNK_FORHEAD normally would be looking at the token directly after the left-hand-side expression, which it verifies is a semi-colon. As the "of" was already consumed from the token stream, it is no longer available to flag a syntax error. I did not look into why pn1 is still semi-valid, but it seems to be ;P.

My personal read of the ECMAScript 6 grammar is that this path of "degrade a for-of into a for-head in the case of a retroactive let identifier" is incorrect: this case should simply be a syntax error (though arguably that would happen at this point anyway if this code ungetToken()'d the "of", as it would not match ";").

(I will go further, and question the idea that the ECMAScript 6 grammar requires an implementation to somehow retroactively use different lookahead depending on a token that occurs arbitrarily later in the token stream, but that is a discussion for a different issue tracker ;P. I'm talking about this with someone currently.)

Comment 1

[Jeff Walden [:Waldo]]

Wow, that's awful.  :-)  I'm not sure whether I'm surprised or not.  There is a great deal of ruin in for-loop parsing (particularly in SpiderMonkey), but I'd have thought by now we'd sussed out the bad-syntax bogosities.

Happily, and coincidentally, my current incomplete patchwork for bug 449811 and bug 10669480 fixes this.

[jwalden@the-great-waldo-search src]$ dbg/js/src/js
js> let = {}; for (let.x of;;);
typein:1:15 SyntaxError: 'let' can't appear as an identifier at the beginning of X in 'for (X of, Y)':
typein:1:15 let = {}; for (let.x of;;);
typein:1:15 ...............^

So this can/should wait to be fixed til that patchwork has landed.

Comment 2

[Jeff Walden [:Waldo]]

Bug 1069480, that is.

Comment 3

[Jeff Walden [:Waldo]]

This is a syntax error now that bug 1233249 is fixed.