350991 - Decompilation of function with "let" expression in for's initial-expression does not compile

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

> function () { for ((let (y) 3); ;) { } }
function () { for (let (y) 3;;) { } }

> function () { for (let (y) 3;;) { } }
SyntaxError on line 1: missing variable name

According to http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Statements:for the first thing in a for loop can be an expression *or* a variable declaration, but not any statement.  So I guess the reason the second doesn't compile is that the compiler sees the "let" and thinks "this must be a variable declaration rather than an expression", but then it fails to compile as a variable declaration because it's written as a "let" expression rather than a "let" statement.

I can imagine this being fixed in one of two ways:
(1) Make the decompiler not drop the parens when decompiling the first.
(2) Make the compiler accept the second, since it seems to me that the second should be legal.

Comment 1

[Brendan Eich [:brendan]]

Attachment: fix

This is important not only for decompiler hygiene, but for JS1.7 semantics, which should match the ES4 wiki (which is being clarified on this point).

/be

Comment 2

[Brendan Eich [:brendan]]

Fixed on trunk.

/be

Comment 3

[Jeff Walden [:Waldo]]

js> function() { let x = 5; while (x-- > 0) { for (let x = x, q = 5;;); } }
Segmentation fault

I'm 99% sure it was the checkin for this bug that did it.

Comment 4

[Brendan Eich [:brendan]]

Thanks for catching that -- I should have seen it.

/be

Comment 5

[Brendan Eich [:brendan]]

I backed out.  The problem, of course, is that for (let i = 0; ...) is not the same as let i; for (i = 0; ...).  But re-using Statement to process the let decl or expr in the first part of a for loop will morph the nearest enclosing block into a lexical scope.

/be

Comment 6

[Brendan Eich [:brendan]]

Attachment: much simpler, and correct, fix

The grammatical type of the thing at the front of a for loop head is expression, not statement, so the whole original patch that tried to use Statement was misconceived.  This may be as confusing for users as for implementors.  The three let forms are not anywhere near the same in meaning.  At least the one that looks like var resembles var otherwise, and the let(head)... block and expression forms look new and different.

/be

Comment 7

[Brendan Eich [:brendan]]

Fixed on trunk, for good.

/be

Comment 8

[Bob Clary [:bc] (inactive)]

Checking in regress-350991.js;
/cvsroot/mozilla/js/tests/js1_7/block/regress-350991.js,v  <--  regress-350991.js
initial revision: 1.1

Comment 9

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060903 windows/mac*/linux

Comment 10

[Mike Schroepfer]

Brendan - quick risk/reward thoughts for 1.8 branch?

Comment 11

[Brendan Eich [:brendan]]

Comment on attachment 236589 [details] [diff] [review]
much simpler, and correct, fix

Sorry, I should have said: this is essentially a two-line addition to handle an unhandled case that is required in ES4/JS2, and that ought to be in JS1.7 along with the rest of 'let'.  It reuses LetBlock, which is used in exactly the same way in the general 'let x...' case, and just needs to handled here in the start of a for loop head.  The risk is as close to zero as any, since it calls existing code in a consistent way. The reward is JS1.7 not being broken in this 'for (let (...) ...; ...)' case.

/be

Comment 12

[Mike Schroepfer]

Comment on attachment 236589 [details] [diff] [review]
much simpler, and correct, fix

a=schrep for drivers given comment 11

Comment 13

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch.

/be

Comment 14

[Bob Clary [:bc] (inactive)]

verified fixed 1.8 20060906 windows/mac*/linux