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

VERIFIED FIXED in mozilla1.9beta3

Status

()

defect
P1
normal
VERIFIED FIXED
12 years ago
11 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

Trunk
mozilla1.9beta3
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
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
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
(Assignee)

Comment 1

12 years ago
Posted patch fixSplinter Review
Want to get this in and shake the trees for anything it might break (I would be surprised, but not stunned...).

/be
Attachment #295157 - Flags: review?(mrbkap)
(Assignee)

Comment 2

12 years ago
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
Flags: blocking1.9+

Comment 3

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

Comment 4

12 years ago
Yep, this patch makes the testcases for those three bugs (plus bug 352907) be compilation errors, so there's no decompilation to get wrong.
Attachment #295157 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 5

12 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(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

12 years ago
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.
(Assignee)

Comment 8

12 years ago
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
(Assignee)

Updated

12 years ago
Depends on: 410894

Comment 9

12 years ago
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
(Assignee)

Comment 10

12 years ago
(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

Updated

12 years ago
Depends on: 410900

Comment 11

12 years ago
This does not cause a syntax error:

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

Comment 12

12 years ago
(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

12 years ago
/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
Flags: in-testsuite+
Flags: in-litmus-

Updated

12 years ago
Depends on: 410981

Comment 14

12 years ago
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
(Assignee)

Updated

12 years ago
Blocks: 411279

Updated

12 years ago

Updated

12 years ago
No longer blocks: 411279
Depends on: 411279
(Assignee)

Updated

12 years ago
Summary: let declaration must be direct child of block or top-level implicit block → let declaration must be direct child of block, top-level implicit block, or switch body block

Comment 15

11 years ago
v
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.