js> function f() { var a = 2; if (!!true) let a = 3; return a; } js> f() 3 js> f function f() { var a = 2; if (!!true) { let a = 3; } return a; } js> eval(uneval(f)) js> f() 2 I don't know where the scope of a let declaration in a statement without curly brackets should be.

Decompiler always braces, for simplicity -- obviously that changes the scope of let (let declarations, e.g. let a = 3, bind in the nearest enclosing block, or the function body if no such block). Contrast with bug 349634, where unnecessary-before-let-was-introduced braces are elided by the decompiler. /be

This turned out decently, after some noodling. /be

This could be beefed up to test, for all the table entries, that braces do make the let a = 3 block-local and therefore change the expected return value. /be

I fussed a bit, and eliminated a vacuous if (DONT_BRACE => braceOffset >= 0). I'm pretty happy with this now. /be

Couldn't resist fussing once more: why unroll the memmove with an explicit store of the '\n' over the space before the brace, when it can be included in the move for free? /be

D'oh, the test was broken. This is better, and gives visual results (should be testable against reference decompilations). /be

Fixed to cope with JS_DONT_PRETTY_PRINT option, which suppresses newlines. Thus the memmove must handle moving the newline down two chars in the pretty case. /be

Final fix, I missed early output from the test: function f1() { var n = 2, a = 2; if (!!true) let a = 3; else { foopy(); } return a; } Easily handled by js_printf'ing the "else" with the "\t} ", leaving the " {\n" to be js_printf'd separately with SET_MAYBE_BRACE(jp). Dangling else works, as ever: js> version(170) 0 js> function f(x){if (x) if (y) z; else let w } js> f function f(x) { if (x) { if (y) { z; } else let w; } } js> function f(x){if (x){ if (y) z;} else let w } js> f function f(x) { if (x) { if (y) { z; } } else let w; } js> function f(x){if (x){ if (y) let z;} else let w } js> f function f(x) { if (x) { if (y) let z; } else let w; } /be

This is important given let declarations for block scope in JS1.7. /be

Oops, I knew I forgot something that I thought of (like pink, best thoughts come in the shower, but no dive whiteboard or rubber memo-duck yet), then forgot: js> version(170) 0 js> function f(){var a = 2; if (x) {let a = 3; print(a)} return a} js> f function f() { var a = 2; if (x) let a = 3; print(a); return a; } With this patch, it's all good: js> function f(){var a = 2; if (x) {let a = 3; print(a)} return a} js> f function f() { var a = 2; if (x) { let a = 3; print(a); } return a; } js> function f(){var a = 2; if (x) {print(a);let a = 3} return a} js> f function f() { var a = 2; if (x) { print(a); let a = 3; } return a; } js> function f(){var a = 2; if (x) {let a = 3} return a} js> f function f() { var a = 2; if (x) { let a = 3; } return a; } js> function f(){var a = 2; if (x) let a = 3; return a} js> f function f() { var a = 2; if (x) let a = 3; return a; } Interdiff to the see the one crucial line. /be

Comment on attachment 237570 [details] [diff] [review] fix, v6 >+#if JS_HAS_BLOCK_SCOPE >+ if (jp->braceState == MAYBE_BRACE && >+ pc + 1 == endpc && >+ !strncmp(rval, var_prefix[SRC_DECL_LET], 4) && >+ rval[4] != '(') { >+ SetDontBrace(jp); >+ } >+#endif Comment on what this is doing and r=mrbkap.

This merged up to tip, to get the patch for bug 352026 that was just committed. /be

Fixed on trunk. /be

Comment on attachment 237788 [details] [diff] [review] fix, v6 with comment and updated to track trunk a=schrep for drivers.

Fixed on the 1.8 branch. /be

Checking in regress-351070-01.js; /cvsroot/mozilla/js/tests/js1_7/block/regress-351070-01.js,v <-- regress-351070-01.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/js/tests/js1_7/block/regress-351070-02.js,v done Checking in regress-351070-02.js; /cvsroot/mozilla/js/tests/js1_7/block/regress-351070-02.js,v <-- regress-351070-02.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/js/tests/js1_7/block/regress-351070-03.js,v done Checking in regress-351070-03.js; /cvsroot/mozilla/js/tests/js1_7/block/regress-351070-03.js,v <-- regress-351070-03.js initial revision: 1.1 done

verified fixed 1.8 20060914 windows/linux 1.9 20060914 windows/mac*/linux