Bug in array iteration / sparse arrays since f70b509

[Webifi]

The array polyfills added in f70b509 broke sparse array support.

An array with sparse properties (say an array with a length of 10, but less than 10 of the array's elements have been set, or some of those array elements were deleted.) will incorrectly set those previously unused elements to undefined and allow iteration of them if an unshift, shift, slice, etc., operation is performed on the array.

For example, the following code:

var test = [1, 2]
test.length = 3

Object.keys(test).forEach(function(k){
  alert(test[k])
})

alert('unshifting 0')
test.unshift(0)

Object.keys(test).forEach(function(k){
  alert(test[k])
})

According to spec., should (and previously did) output:

1
2
unshifting 0
0
1
2

But on the latest version of interpreter.js it outputs:

1
2
unshifting 0
0
1
2
undefined

The issue is that after 0 is prepended to the array via test.unshift(0), the last non-iterable "empty" element in the array that was created by via test.length = 3 suddenly becomes iterable.

[Webifi]

In addition to unshift, shift and splice (and probably more) also seem to cause non-iterable elements to become iterable.

[Webifi]

It looks like this could be fixed in some of the Array polyfills by checking hasOwnPropertyand deleting the index instead of setting it when iterating the array values.

For example, in unshift:

"Object.defineProperty(Array.prototype, 'unshift',",
    "{configurable: true, writable: true, value:",
  "function unshift(var_args) {",
    "if (!this) throw TypeError();",
    "var o = Object(this);",
    "var len = o.length >>> 0;",
    "if (!len || len < 0) {",
      "len = 0;",
    "}",
    "for (var i = len - 1; i >= 0; i--) {",
      // "o[i + arguments.length] = o[i];", // This is what we used to do
      "if(o.hasOwnProperty(i)) {",
         "o[i + arguments.length] = o[i];", // Okay to set property
       "} else {",
         "delete o[i + arguments.length];", // Delete property
       "}",
    "}",
    "for (var i = 0; i < arguments.length; i++) {",
      "o[i] = arguments[i];",
    "}",
    "return o.length = len + arguments.length;",
  "}",
"});",

Edit: changed propertyIsEnumerable to hasOwnProperty Edit2: The above seems closest to Chromium's native implementation.

[Webifi]

Anyone working on this? If not, would a pull request be accepted if I take a stab at it using changes like the above?

[NeilFraser]

Go for it. The source of these polyfills is from this daughter project: https://github.com/NeilFraser/JS-Polyfills It would probably be easier to edit the polyfills there, since it's straight code (rather than strings), and executes/debugs natively (rather than being interpreted). Once checked in there, pull the changes here.

[NeilFraser]

Thanks for your work on this @Webifi, everything's been tested and checked-in to both JS-Polyfills and JS-Interpreter.