Closed
Bug 736742
Opened 13 years ago
Closed 13 years ago
Decompiler omits sequential-evaluation parens on RHS of destructuring assignment
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: jruderman, Assigned: luke)
References
Details
(Keywords: regression, testcase)
Attachments
(1 file)
(deleted),
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
The decompiler changed the meaning here:
js> (function() { [m] = (a, b) })
(function () {[m] = a, b;})
jsfunfuzz discovered this because some variants result in a syntax error, e.g.
js> (function() { var [m] = (1, b.c); })
(function () {var [m] = 1, b.c;})
The first bad revision is:
changeset: 38344f96b3e3
user: Luke Wagner
date: Fri Oct 07 12:02:50 2011 -0700
pushed to m-c: Fri Dec 23 15:56:40 2011 -0800
summary: Bug 692274, part 4 - Rewrite parsing, emitting and decompiling of let to fix scoping properly (r=jorendorff)
Assignee | ||
Comment 1•13 years ago
|
||
Good find! Who can forget about the precedence of comma in a destructuring assignment.
Assignee | ||
Comment 2•13 years ago
|
||
Review ping. Green on try.
Comment 3•13 years ago
|
||
Comment on attachment 606890 [details] [diff] [review]
fix
OK, the change to rval I can understand, the JSOP_SETNAME means "context requires precedence at least that of the = operator" but why JSOP_DUP for lval?
According to jsopcode.tbl the precedence of JSOP_DUP is 0... but it seems like a funny thing to depend on. JSOP_POP makes more sense to me (that's the opcode for the , operator) but actually just about any precedence will do, since left-hand sides are pretty much all primitive expressions.
Anyway. r=me with that explained. :)
Attachment #606890 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 4•13 years ago
|
||
Oh, the only reason I was using JSOP_DUP is that is what was being implicitly passed by the POP_STR() macro before (op == JSOP_DUP). But JSOP_POP would be clearer, so I'll use that and add a comment.
Assignee | ||
Comment 5•13 years ago
|
||
Target Milestone: --- → mozilla14
Comment 6•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•