Closed Bug 430717 Opened 15 years ago Closed 15 years ago

Dense arrays don't inherit deleted elements from Array.prototype

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: jorendorff, Assigned: shaver)

References

Details

Attachments

(2 files)

Test case:

  Array.prototype[2] = "two";
  var x = [0, 1, 2, 3, 4, 5];
  delete x[2];
  if (x[2] != "two") throw "bug";

In trunk, x[2] === undefined.
La, la, la.  This is caused by using rval to store the sampled obj->dslots[i], which is then used to fill id -- making a jsid from JSVAL_HOLE is bad.

Patch coming.  Blocking: embarrassingly core language regression.
Assignee: general → shaver
Depends on: 419152
Flags: blocking1.9+
Attachment #317589 - Flags: review?(mrbkap) → review+
My stylistic choices, as I saw them:

A: load obj->dslots[i] twice, and hope the compiler doesn't mind

if (obj->dslots[i] != JSVAL_HOLE) {
    rval = obj->dslots[i];
    goto end_getelem;
}

B: add a temporary

C: just reload rval in the uncommon case, to avoid any penalty to the common case

I went with door C.

Thanks to crowder for recognizing instantly that (jsval)22 was JSVAL_HOLE; saved me a bunch of time.
Comment on attachment 317589 [details] [diff] [review]
reload rval if we have a hole

a=mconnor on behalf of 1.9 drivers
Attachment #317589 - Flags: approval1.9? → approval1.9+
Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.491; previous revision: 3.490
done

The test looks like:

Array.prototype[2] = "two";
var a = [0,1,2,3];
delete a[2];
assert(a[2] === "two");

bclary: where should the test for this go?  In with the ECMA tests for looking in prototype?  I couldn't find those at a glance, and wasn't 100% sure it was the right place anyway.
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
The best location is debatable imo. The best option is to find the section of the specification describing the feature and place the test appropriately. Trying to tie a test to a specific location in the hierarchy is difficult especially considering that ecma/ == ecma 262-1st edition, ecma_2/ == ecma 262 2nd edition, etc and that the section names in the documents have changed between editions. 

I normally don't worry too much about the purity of the final location of the test and in this case would have probably chosen ecma_3/Array, but if the inheritance is more important then js1_3/inherit would be a better location. I prefer not to put anything in js1_3 though unless you have a gun pointed at me. 

I prefer ecma_3 over earlier ecma editions as the older tests have a degree of funkiness which I do not care for. I prefer js1_5 over lower js versions (unless a feature requiring a higher version is required) for the same reason and might have chosen js1_5/Array over ecma_3/Array.
I'll check in when the tree reopens
/cvsroot/mozilla/js/tests/ecma_3/Array/regress-430717.js,v  <--  regress-430717.js
initial revision: 1.1
Flags: in-testsuite?
Flags: in-testsuite+
Flags: in-litmus-
v 1.9.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.