352079 - Incorrect decompilation with ++ on an lvalue-return expression

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

./js -v 170

js> function() { f(let (y = 3) 4)++; }

function () {
    f(let (3 = 3) 4)++;
}

Comment 1

[Jesse Ruderman]

If you remove the "let", a different kind of incorrect decompilation occurs:

js> function() { f(4)++; }             

function () {
    (f(4))[()]++;
}

This bug also happens with pre-increment:

js> function() { ++f(4); }

function () {
    ++(f(4))[()];
}

Comment 2

[Jesse Ruderman]

Same with the delete operator:

js> function() { delete(p(3)) }   

function () {
    delete (p(3))[()];
}

Comment 3

[Brendan Eich [:brendan]]

Regression from fix for bug 350238 (itself a fix to a previous inadequate fix). The decompiler needs to track the extra stack slot required for postfix property and element increment forms.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: fix

JSOP_SETCALL is not an assignment op in precedence, it's a left-hand side expression operator.

/be

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 237860 [details] [diff] [review]
fix

This fix is good, but I don't see how it fixes the let bustage. Am I missing something?

Comment 6

[Brendan Eich [:brendan]]

The code for the comment 0 test function is:

js> dis(function() { f(let (y = 3) 4)++; })
flags: LAMBDA
main:
00000:  name "f"
00003:  pushobj
00004:  enterblock depth 3 {y: 0}
00007:  uint16 3
00010:  setlocal 3
00013:  pop
00014:  uint16 4
00017:  leaveblockexpr 1
00020:  setcall 1
00023:  eleminc
00024:  pop
00025:  stop

Source notes:
  0:    13 [  13] xdelta  
  1:    13 [   0] decl     offset 3
  3:    20 [   7] pcbase   offset 20
  5:    23 [   3] pcbase   offset 23

The fix to JSOP_SETCALL's precedence in this patch fixes this case too.  The let expression is just a parameter to the function call lvalue.

Fixed on trunk.

/be

Comment 7

[Jesse Ruderman]

The problem in comment 0 remains.

Comment 8

[Brendan Eich [:brendan]]

Oh, right -- separate bug, harder to fix.  Can you spin it out?  Thanks,

/be

Comment 9

[Jesse Ruderman]

Ok, I split that out into bug 352272.  I'm resummarizing this bug to what was fixed here.

Comment 10

[Brendan Eich [:brendan]]

Attachment: forgot the JSOP_POPV precedence fix

In all the patch-juggling.  This is checked in now:

Checking in jsopcode.tbl;
/cvsroot/mozilla/js/src/jsopcode.tbl,v  <--  jsopcode.tbl
new revision: 3.75; previous revision: 3.74
done

The jsopcode.c rev was:

revision 3.74
date: 2006/09/12 02:07:32;  author: brendan%mozilla.org;  state: Exp;  lines: +1
 -1
Fix JSOP_SETCALL and JSOP_POPV precedences; turn off auto-parens for bracketed i
ndex values (352079, r=mrbkap).

/be

Comment 11

[Mike Schroepfer]

Comment on attachment 237860 [details] [diff] [review]
fix

a=schrep for 181drivers for JS decompiler bug.

Comment 12

[Mike Schroepfer]

Comment on attachment 237920 [details] [diff] [review]
forgot the JSOP_POPV precedence fix

a=schrep for 181drivers for JS decompiler bug.

Comment 13

[Brendan Eich [:brendan]]

All fixed on the 1.8 branch.

/be

Comment 14

[Bob Clary [:bc] (inactive)]

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

Comment 15

[Bob Clary [:bc] (inactive)]

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