408957 - let declaration must be direct child of block, top-level implicit block, or switch body block

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

See http://bugs.ecmascript.org/ticket/280 -- this is much saner than what we did in JS1.7, it will simplify the decompiler as well as the compiler. We should do it for Mozilla 1.9 / Firefox 3 to prepare for JS2.

/be

Comment 1

[Brendan Eich [:brendan]]

Attachment: fix

Want to get this in and shake the trees for anything it might break (I would be surprised, but not stunned...).

/be

Comment 2

[Brendan Eich [:brendan]]

Besides tracking JS2/ES4, this reduces code footprint and attack surface (consider the decompiler's track record). It also will close a bunch of bugs Jesse filed, on the limitations of the decompiler's let-in-non-block-triggered magic unbracer. Jesse, can you id those bugs?

/be

Comment 3

[Jesse Ruderman]

Yay!  I bet this will fix at least bug 352422, bug 352786, and bug 376410.

Comment 4

[Jesse Ruderman]

Yep, this patch makes the testcases for those three bugs (plus bug 352907) be compilation errors, so there's no decompilation to get wrong.

Comment 5

[Brendan Eich [:brendan]]

Fixed:

js/src/js.msg 3.82
js/src/jsemit.h 3.87
js/src/jsopcode.c 3.278
js/src/jsparse.c 3.321
js/src/jsscan.c 3.144

/be

Comment 6

[Reed Loden [:reed]]

(In reply to comment #1)
> Want to get this in and shake the trees for anything it might break (I would be
> surprised, but not stunned...).

Glad you got this in now, as there are definite regressions from it (such as bug 410894). :)

Comment 7

[Bob Clary [:bc] (inactive)]

The following now fail with SyntaxError: let declaration not directly within block:

js1_7/block/regress-349298.js
js1_7/block/regress-350730.js
js1_7/block/regress-352092.js
js1_7/block/regress-352185.js
js1_7/decompilation/regress-351070-01.js
js1_7/decompilation/regress-351070-03.js
js1_7/decompilation/regress-352217.js
js1_7/decompilation/regress-352268.js
js1_7/decompilation/regress-352732.js
js1_7/decompilation/regress-356247.js
js1_7/extensions/regress-351070-02.js

what is the best way to handle this? rewrite using eval to catch the SyntaxError on the trunk or rewrite to put let in explicit {} or just note them as known errors on the trunk.

Comment 8

[Brendan Eich [:brendan]]

bclary, not sure wht you prefer -- we should test for SyntaxError (message detail is not important, but exception type is), so I favor try{eval(s)}catch(e){...}.

/be

Comment 9

[Benjamin Smedberg]

Is the pattern

for each (let v in a) {
}

legal now?

We have many uses of let in the tree which follow this pattern, see http://mxr.mozilla.org/mozilla/search?string=let+&find=\.js%24&findi=&filter=&tree=mozilla

Comment 10

[Brendan Eich [:brendan]]

(In reply to comment #9)
> Is the pattern
> 
> for each (let v in a) {
> }
> 
> legal now?

Yes, that is not going away of course:

js> function f(){for(let i=0;i<10;i++)print(i)}
js> f()
0
1
2
3
4
5
6
7
8
9
js> function g(){for (let i in this)print(i)}        
js> g()
f
g
js> function h(){for each (let i in this)print(i)}
js> h()
function f() {
    for (let i = 0; i < 10; i++) {
        print(i);
    }
}
function g() {
    for (let i in this) {
        print(i);
    }
}
function h() {
    for each (let i in this) {
        print(i);
    }
}

Sorry for not noting the for and for-in exceptions, but testing allays doubt. Still not sure what the specific download.js errors were, but that is for bug   	410894...

/be

Comment 11

[Bob Clary [:bc] (inactive)]

This does not cause a syntax error:

js> (function() { for(let x in []) let {} = [1]; })
function () {
    for (let x in []) {
        let [] = [1];
    }
}

Comment 12

[Brendan Eich [:brendan]]

(In reply to comment #11)
> This does not cause a syntax error:
> 
> js> (function() { for(let x in []) let {} = [1]; })
> function () {
>     for (let x in []) {
>         let [] = [1];
>     }
> }

Followup bug blocking this one to me please. Thanks,

/be

Comment 13

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_7/block/regress-349298.js,v
new revision: 1.4; previous revision: 1.3

/cvsroot/mozilla/js/tests/js1_7/block/regress-350730.js,v
new revision: 1.4; previous revision: 1.3

/cvsroot/mozilla/js/tests/js1_7/block/regress-352092.js,v
new revision: 1.3; previous revision: 1.2

/cvsroot/mozilla/js/tests/js1_7/block/regress-352185.js,v
new revision: 1.3; previous revision: 1.2

/cvsroot/mozilla/js/tests/js1_7/decompilation/regress-351070-01.js,v
new revision: 1.3; previous revision: 1.2

/cvsroot/mozilla/js/tests/js1_7/decompilation/regress-351070-03.js,v
new revision: 1.3; previous revision: 1.2

/cvsroot/mozilla/js/tests/js1_7/decompilation/regress-352217.js,v
new revision: 1.3; previous revision: 1.2

/cvsroot/mozilla/js/tests/js1_7/decompilation/regress-352268.js,v
new revision: 1.3; previous revision: 1.2

/cvsroot/mozilla/js/tests/js1_7/decompilation/regress-352732.js,v
new revision: 1.3; previous revision: 1.2

/cvsroot/mozilla/js/tests/js1_7/decompilation/regress-356247.js,v
new revision: 1.3; previous revision: 1.2

/cvsroot/mozilla/js/tests/js1_7/extensions/regress-351070-02.js,v
new revision: 1.3; previous revision: 1.2

Comment 14

[Bob Clary [:bc] (inactive)]

account for modified failure message in 1.8 branch

/cvsroot/mozilla/js/tests/public-failures.txt,v  <--  public-failures.txt
new revision: 1.19; previous revision: 1.18

Comment 15

[Bob Clary [:bc] (inactive)]

v