330243 - Array.prototype.concat isn't even as generic as ECMA requires

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

According to ECMA, we shouldn't care if the arguments to concat are Array objects or not, just if they have .length.  (Though the algorithm looks like it would really suck if called on an object that _doesn't_ have .length, since it'd keep going until k was equal to undefined (NaN?)!)

Our implementation cares, quite explicitly:

http://lxr.mozilla.org/seamonkey/source/js/src/jsarray.c#1416

and forces |this| through this path as well, at:

http://lxr.mozilla.org/seamonkey/source/js/src/jsarray.c#1400

Can we fix this?  IE apes our current behaviour, and Safari seems to as well.

If we can't fix it -- and this is where Deb understands why she's cc:'d -- then we should fix our documentation, which currently says that concat is generic:

http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Global_Objects:Array#Generic_methods

(ECMA 15.4.4.4 also requires that |this| be an Array object to have it expanded correctly, which I think is bogus -- it should require that it have a length property only, like other operations on array-like things, and it even has a comment to that effect affter the definition of concat!  So maybe we want to fix that in ES4?)

Comment 1

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Waldo: can you interpret the ES5 runes for me and WONTFIX or re-version this bug?  <3

Comment 2

[Tom S [:evilpie]]

We now follow the ES5 spec.
Pseudo-code:
E = [this, param1, param2, ...]
A = []
for all x in E:
   x is Array (not is-array-like!):
      for y in x:
         if y is not hole:
            A.append(y)
   else:
      A.append(x)

From my perspective not really generic.