349596 - Labeled "if(0)..." disappears but label remains

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

Steps to reproduce:
  javascript:function() { L: if (0) return 5 }

Result:
  function () { L: }
  
Expected:
  (1) Decompile to what I had originally, or
  (2) Decompile to a simpler useless statement/block, such as "L: if (0) { }" or "L: null", or
  (3) Also remove the label, since it is useless given that nothing can break/continue to it.

This is similar to bug 349489, which is already fixed.

Comment 1

[Brendan Eich [:brendan]]

This could be a problem in the constant folder, which runs right after the parse phase.  It's therefore not like bug 349489, which was a bug in the useless expression eliminator, which runs from the code generator -- a later phase than the constant folding phase.  But it is actually a bug in the decompiler:

js> function f(){L:;}
js> f
function f() {
L:
}

Old bug, taking.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

Turns out that the easiest, and the singular, place to fix this is in the constant folder.  There is no point elaborating the compiled code with a special "smoke signal" just for empty labeled statements, which are rare in JS (compared to C, where you sometimes need a goto target at the bottom of a loop, e.g.).

By turning all empty labeled statements into labeled empty blocks, we save on code for special or redundant case logic, at the cost of an extra nop.  This fixes both the verbatim L:; input case, and equivalent L:if(0)foo(); constant-folded cases.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: alterna-fix that doesn't require the constant folding phase

This may be a bit more code, but on second thought I'd rather not mandate the constant folding phase.

/be

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 234887 [details] [diff] [review]
alterna-fix that doesn't require the constant folding phase

Checking in jsparse.c;
/cvsroot/mozilla/js/src/jsparse.c,v  <--  jsparse.c
new revision: 3.206; previous revision: 3.205
done

Comment 5

[Brendan Eich [:brendan]]

Fixed on trunk.

/be

Comment 6

[Mike Schroepfer]

Comment on attachment 234887 [details] [diff] [review]
alterna-fix that doesn't require the constant folding phase

a=schrep for drivers.

Comment 7

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch.

/be

Comment 8

[Brendan Eich [:brendan]]

Attachment: hand-merged 1.8 branch version of the alterna-fix

Comment 9

[Bob Clary [:bc] (inactive)]

Checking in regress-349596.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-349596.js,v  <--  regress-349596.js
initial revision: 1.1

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed 1.8, 1.9 20060824 windows/mac*/linux