352217 - Need space between "else" and "let"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

js> uneval(function() { if(g) h; else let x; })
(function () {if (g) {h;} elselet x;})

Regression from bug 351070?

Comment 1

[Brendan Eich [:brendan]]

I should read my test output more carefully:

$ ./Darwin_DBG.OBJ/js < maybebrace.js| fgrep '{let'
(function f() {var n = 2, a = 2;do {let a = 3;} while (false);return a;})
(function f() {var n = 2, a = 2;with (this) {let a = 3;}return a;})

Neither do-while nor with decompilaton cases call Decompile recursively on just the body statement, frustrating the maybe=>don't-brace logic under JSVAL_POP{,V}.

/be

Comment 2

[Brendan Eich [:brendan]]

The "elselet" bug is easy to fix, the do-while and with cases a bit harder.  More soon, I hope.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: patch that fixes the elselet problem

I would like to develop this to fix related problems in the maybe-brace code.

/be

Comment 4

[Brendan Eich [:brendan]]

Jesse, could you do me a big favor and link the other bugs that depend on brace removal due to let to this one, or to a new tracking one if you prefer?  Thanks,

/be

Comment 5

[Brendan Eich [:brendan]]

Attachment: fix elselet and several related probs

See the dependent bugs.

Jesse, this should cure all ills except for with, do-while, and if-else-if statements that govern unbraced let statements.  Let me know if something's amiss.

/be

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 238031 [details] [diff] [review]
fix elselet and several related probs

Have to retest more before attaching.

/be

Comment 7

[Brendan Eich [:brendan]]

Attachment: better fix

The js_printf code can't be too smart.  It's up to js_printf callers to clear maybe-brace state as needed.  That means all the mandatory-brace control flow structures (try, catch, finally, switch), plus explicit braced blocks (annotated with SRC_BRACE).

This does not fix bug 352268, that (if-else-if) is a separate headache.

/be

Comment 8

[Jesse Ruderman]

I split out two bugs:

Bug 352421 for "do..while"
Bug 352422 for "with"

Comment 9

[Brendan Eich [:brendan]]

Fixed on trunk:

Checking in jsopcode.c;
/cvsroot/mozilla/js/src/jsopcode.c,v  <--  jsopcode.c
new revision: 3.179; previous revision: 3.178
done

/be

Comment 10

[Mike Schroepfer]

Comment on attachment 238042 [details] [diff] [review]
better fix

a=schrep for 181drivers

Comment 11

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch.

/be

Comment 12

[Bob Clary [:bc] (inactive)]

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

Comment 13

[Bob Clary [:bc] (inactive)]

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