Closed Bug 691993 Opened 8 years ago Closed 8 years ago

Fully split up deleteProperty into property/element/special/generic forms

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: Waldo, Assigned: Waldo)

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
More dreariness.

This one in particular would be much nicer, generally, if the opcode format were split such that we had delprop, delelem, delval -- that is, we had specific bytecodes for deleting named properties and deleting index properties.  I tend to think that sort of bytecode mucking is better left to later on in API changes, so for now we get a bunch of deleteGeneric.
Attachment #564723 - Flags: review?(bhackett1024)
Comment on attachment 564723 [details] [diff] [review]
Patch

I don't understand the opcode format comment.  There are separate bytecodes already for DELNAME, DELPROP and DELELEM, and for DELELEM there is no way for the parser to tell ahead of time whether the property being deleted is named or indexed.
Attachment #564723 - Flags: review?(bhackett1024) → review+
What I suggest is that DELELEM be split into a variant that deletes using a value on the stack to indicate the property to delete, and a variant that deletes using a constant index included in the bytecode stream:

  delete a.b  => delprop "b"
  delete a[2] => delelem 2
  delete a[c] => delval #(slot for c)

Or something like that.

It's possible there might not be the payoff in splitting up delelem that way, since delete is kind of uncommon.  But if there were such a split, delelem could use obj->deleteElement, delprop could use obj->deleteProperty, and delval could use obj->deleteGeneric (or longer-run something to disambiguate the val into an element, property, or special, then use delete{not Generic}).
https://hg.mozilla.org/integration/mozilla-inbound/rev/9291abf9fd17
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/9291abf9fd17
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.