Closed
Bug 471703
Opened 16 years ago
Closed 15 years ago
Don't optimize group assignment given holey RHS
Categories
(Core :: JavaScript Engine, defect, P4)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
People
(Reporter: brendan, Assigned: brendan)
Details
(Keywords: verified1.9.1, Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 3 obsolete files)
|
2.85 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
See bug 469625 comment 27. /be
|
Assignee | |
Comment 2•15 years ago
|
||
|
|
Assignee | |
Updated•15 years ago
|
Priority: -- → P4
|
||
Updated•15 years ago
|
Attachment #369342 -
Flags: review?(mrbkap) → review+
|
||
Comment 3•15 years ago
|
||
(In reply to comment #2) > Created an attachment (id=369342) [details] > fix This does not remove the code in EmitGroupAssignment that deals with holes in the right-hand-side. Any reason for keeping this deadwood?
|
|
Assignee | |
Comment 4•15 years ago
|
||
(In reply to comment #3) > (In reply to comment #2) > > Created an attachment (id=369342) [details] [details] > > fix > > This does not remove the code in EmitGroupAssignment that deals with holes in > the right-hand-side. Any reason for keeping this deadwood? Nope, I was just reading that, while killing time at the TC39 meeting, then forgot to remove when I patched. /be
|
|
Assignee | |
Comment 5•15 years ago
|
||
Attachment #369342 -
Attachment is obsolete: true
Attachment #369411 -
Flags: review?(igor)
|
|
||
Comment 6•15 years ago
|
||
There is another pre-existing deadwood in EmitGroupAssignment. MaybeEmitGroupAssignment calls it only when lhs->pn_count <= rhs->pn_count. But the second loop over lhs does not use this condition and makes sure that extra undefs are placed on the stack to initialize elements from the left without the corresponding part on the right like in [a,b] = [1]. So either the condition MaybeEmitGroupAssignment or the push code in the EmitGroupAssignment loop can be removed.
|
|
||
Updated•15 years ago
|
Attachment #369411 -
Flags: review?(igor) → review+
|
|
||
Comment 7•15 years ago
|
||
Comment on attachment 369411 [details] [diff] [review] fix, v2 Note that the patch would need a merge with changes due to the bug 484769.
|
|
Assignee | |
Comment 8•15 years ago
|
||
One more time, just to make sure. Jet-lag is hurting me... /be
Attachment #369411 -
Attachment is obsolete: true
Attachment #369922 -
Flags: review?(igor)
|
|
||
Updated•15 years ago
|
Attachment #369922 -
Flags: review?(igor) → review+
|
|
Assignee | |
Comment 9•15 years ago
|
||
Attachment #369922 -
Attachment is obsolete: true
Attachment #376370 -
Flags: review+
|
|
Assignee | |
Comment 10•15 years ago
|
||
Fixed: http://hg.mozilla.org/tracemonkey/rev/a4197febbf1d http://hg.mozilla.org/mozilla-central/rev/834e62999a36 /be
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-tracemonkey
Target Milestone: --- → mozilla1.9.2a1
|
||
Comment 11•15 years ago
|
||
Looks like this burned the static analysis boxes
|
|
||
Comment 12•15 years ago
|
||
Ah, bsmedberg says on irc: sayrer: I'm bisecting the static analysis burnage... there was a treehydra commit around the same time which might have exposed a latent older SM issue
|
|
Assignee | |
Comment 13•15 years ago
|
||
Yeah, I looked last night, but it wasn't me! ;-) /be
|
|
||
Updated•15 years ago
|
Flags: wanted1.9.1+
|
|
||
Comment 14•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9824a01132b0
Keywords: fixed1.9.1
|
||
Comment 15•15 years ago
|
||
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•