Decompilation of "new let" includes parentheses incorrectly

VERIFIED FIXED in mozilla1.8.1

Status

()

P2
normal
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: jruderman, Assigned: mrbkap)

Tracking

({testcase, verified1.8.1})

Trunk
mozilla1.8.1
testcase, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [181approval pending])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

13 years ago
> f = function() { new let (x = 3) x }
function () { new let (x = 3) x(); }

Suggested fix:
(1) don't include those extra parentheses, or
(2) also put parens around "let (x = 3) x" so the extra parens will be interpreted as intended.
mrbkap, you game to take?

/be
Flags: blocking1.8.1?
Flags: blocking1.8.1? → blocking1.8.1+
Target Milestone: --- → mozilla1.8.1
Summary: Decompliation of "new let" includes parentheses incorrectly → Decompilation of "new let" includes parentheses incorrectly
(Assignee)

Comment 2

13 years ago
Created attachment 234848 [details] [diff] [review]
Proposed fix

This fix has two parts:

The first is to give JSOP_LEAVEBLOCKEXPR a non-zero but very small precedence, so that we can correctly parenthesize it when we are decompiling. The second is to ensure that we look at the correct op when popping argv[0], thus forcing the parentheses around argv[0] if it has lower precedence.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #234848 - Flags: review?(brendan)
(Assignee)

Updated

13 years ago
OS: Mac OS X 10.4 → All
Priority: -- → P2
Hardware: Macintosh → All
Comment on attachment 234848 [details] [diff] [review]
Proposed fix

>Index: jsopcode.tbl
>===================================================================
>RCS file: /cvsroot/mozilla/js/src/jsopcode.tbl,v
>retrieving revision 3.61
>diff -p -U8 -r3.61 jsopcode.tbl
>--- jsopcode.tbl	17 Aug 2006 20:24:38 -0000	3.61
>+++ jsopcode.tbl	21 Aug 2006 21:33:45 -0000
>@@ -434,9 +434,9 @@ OPDEF(JSOP_ARRAYPUSH,     212,"arraypush
> 
> OPDEF(JSOP_FOREACHKEYVAL, 213,"foreachkeyval",NULL,   1,  0,  0,  0,  JOF_BYTE)
> 
> /*
>  * Variant of JSOP_ENUMELEM for destructuring const (const [a, b] = ...).
>  */
> OPDEF(JSOP_ENUMCONSTELEM, 214,"enumconstelem",NULL,   1,  3,  0,  1,  JOF_BYTE|JOF_SET|JOF_ASSIGNING)
> 
>-OPDEF(JSOP_LEAVEBLOCKEXPR,215,"leaveblockexpr",NULL,  3,  0,  0,  0,  JOF_UINT16)
>+OPDEF(JSOP_LEAVEBLOCKEXPR,215,"leaveblockexpr",NULL,  3,  0,  0,  1,  JOF_UINT16)

Add a comment before the LEAVEBLOCK* pair about why the precedence for LEAVEBLOCKEXPR is 1.

r=me with that.

/be
Attachment #234848 - Flags: review?(brendan) → review+
(Assignee)

Comment 4

13 years ago
Created attachment 234853 [details] [diff] [review]
With the comment

How's this comment look?
Attachment #234848 - Attachment is obsolete: true
Attachment #234853 - Flags: review?(brendan)
Comment on attachment 234853 [details] [diff] [review]
With the comment

Great!

/be
Attachment #234853 - Flags: review?(brendan) → review+
(Assignee)

Comment 6

13 years ago
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Updated

13 years ago
Attachment #234853 - Flags: approval1.8.1?

Comment 7

13 years ago
Checking in regress-349499.js;
/cvsroot/mozilla/js/tests/js1_7/block/regress-349499.js,v  <--  regress-349499.js
initial revision: 1.1
Flags: in-testsuite+
Whiteboard: [181approval pending]

Comment 8

13 years ago
Comment on attachment 234853 [details] [diff] [review]
With the comment

a=schrep for drivers - approving all [181approval pending] bugs now that tree is open.
Attachment #234853 - Flags: approval1.8.1? → approval1.8.1+
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1

Comment 10

13 years ago
verified fixed 1.8, 1.9 20060824 windows/mac*/linux
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1

Comment 11

12 years ago
decompilation no longer uses () for new Constructor.
Checking in regress-349499.js;
/cvsroot/mozilla/js/tests/js1_7/block/regress-349499.js,v  <--  regress-349499.js
new revision: 1.2; previous revision: 1.1
done
You need to log in before you can comment on or make changes to this bug.