Closed Bug 352079 Opened 18 years ago Closed 18 years ago

Incorrect decompilation with ++ on an lvalue-return expression

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8.1

People

(Reporter: jruderman, Assigned: brendan)

References

Details

(Keywords: testcase, verified1.8.1)

Attachments

(2 files)

./js -v 170

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

function () {
    f(let (3 = 3) 4)++;
}
Summary: Incorrect decompilation for "let" in argument to lvalue-returning function → Incorrect decompilation for "let" in argument to lvalue-returning function (involving post-increment)
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))[()];
}
Same with the delete operator:

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

function () {
    delete (p(3))[()];
}
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
Depends on: 350238
OS: Mac OS X 10.4 → All
Priority: -- → P1
Hardware: Macintosh → All
Target Milestone: --- → mozilla1.8.1
Attached patch fixSplinter Review
JSOP_SETCALL is not an assignment op in precedence, it's a left-hand side expression operator.

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #237860 - Flags: review?(mrbkap)
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?
Attachment #237860 - Flags: review?(mrbkap) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Attachment #237860 - Flags: approval1.8.1?
The problem in comment 0 remains.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Oh, right -- separate bug, harder to fix.  Can you spin it out?  Thanks,

/be
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Ok, I split that out into bug 352272.  I'm resummarizing this bug to what was fixed here.
Summary: Incorrect decompilation for "let" in argument to lvalue-returning function (involving post-increment) → Incorrect decompilation with ++ on an lvalue-return expression
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
Attachment #237920 - Flags: review+
Attachment #237920 - Flags: approval1.8.1?
Comment on attachment 237860 [details] [diff] [review]
fix

a=schrep for 181drivers for JS decompiler bug.
Attachment #237860 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 237920 [details] [diff] [review]
forgot the JSOP_POPV precedence fix

a=schrep for 181drivers for JS decompiler bug.
Attachment #237920 - Flags: approval1.8.1? → approval1.8.1+
All fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1
Checking in regress-352079.js;
/cvsroot/mozilla/js/tests/js1_7/lexical/regress-352079.js,v  <--  regress-352079.js
initial revision: 1.1
done
Flags: in-testsuite+
verified fixed 1.8 20060914 windows/linux 1.9 20060914 windows/mac*/linux
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: