Closed
Bug 350991
Opened 18 years ago
Closed 18 years ago
Decompilation of function with "let" expression in for's initial-expression does not compile
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.8.1
People
(Reporter: jruderman, Assigned: brendan)
References
Details
(Keywords: testcase, verified1.8.1)
Attachments
(1 file, 1 obsolete file)
1.36 KB,
patch
|
mrbkap
:
review+
mtschrep
:
approval1.8.1+
|
Details | Diff | Splinter Review |
> 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.
Reporter | ||
Updated•18 years ago
|
Summary: Decompilation of function with "let" in for's initial-expression does not compile → Decompilation of function with "let" expression in for's initial-expression does not compile
Assignee | ||
Comment 1•18 years ago
|
||
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
Assignee | ||
Updated•18 years ago
|
Flags: blocking1.8.1?
OS: Mac OS X 10.4 → All
Priority: -- → P1
Hardware: Macintosh → All
Target Milestone: --- → mozilla1.8.1
Updated•18 years ago
|
Attachment #236572 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 2•18 years ago
|
||
Fixed on trunk. /be
Assignee | ||
Updated•18 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•18 years ago
|
Attachment #236572 -
Flags: approval1.8.1?
Comment 3•18 years ago
|
||
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.
Assignee | ||
Updated•18 years ago
|
Attachment #236572 -
Flags: approval1.8.1?
Assignee | ||
Comment 4•18 years ago
|
||
Thanks for catching that -- I should have seen it. /be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 5•18 years ago
|
||
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
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 6•18 years ago
|
||
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
Attachment #236572 -
Attachment is obsolete: true
Attachment #236589 -
Flags: review?(mrbkap)
Updated•18 years ago
|
Attachment #236589 -
Flags: review?(mrbkap) → review+
Assignee | ||
Updated•18 years ago
|
Attachment #236589 -
Flags: approval1.8.1?
Assignee | ||
Comment 7•18 years ago
|
||
Fixed on trunk, for good. /be
Status: ASSIGNED → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
Comment 8•18 years ago
|
||
Checking in regress-350991.js; /cvsroot/mozilla/js/tests/js1_7/block/regress-350991.js,v <-- regress-350991.js initial revision: 1.1
Flags: in-testsuite+
Comment 10•18 years ago
|
||
Brendan - quick risk/reward thoughts for 1.8 branch?
Assignee | ||
Comment 11•18 years ago
|
||
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•18 years ago
|
||
Comment on attachment 236589 [details] [diff] [review] much simpler, and correct, fix a=schrep for drivers given comment 11
Attachment #236589 -
Flags: approval1.8.1? → approval1.8.1+
Comment 14•18 years ago
|
||
verified fixed 1.8 20060906 windows/mac*/linux
Keywords: fixed1.8.1 → verified1.8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•