Array.concat() works incorrectly on proxies under V8

[billmark]

Something is wrong with Array.concat when it operates on a proxy. This may be a special case of issue #6 but I'm opening a new issue so we can more easily track the details.

var a = ['x', 'y'];
var h = {};
var aProxy = Proxy(a, h);
var aConcat = [].concat(a);
var aProxyConcat = [].concat(aProxy);

d8> print(aConcat.length);
2

d8> print(aProxyConcat.length);
1

I've confirmed that this bug exists with the latest version of harmony-reflect (0.0.7) and with today's trunk version of V8.

[tvcutsem]

This is because Array.prototype.concat explicitly tests whether one of its arguments is a real array (see http://ecma-international.org/ecma-262/5.1/#sec-15.4.4.4 ). This test fails to recognize a proxy for an array as a real array.

As with issue #6 I'm not sure whether ES6 will eventually end up auto-unwrapping proxies for these cases.

I'll consider patching Array.prototype.concat so that it recognizes proxies.

[metamatt]

You both probably already know this but for anyone else following along it's IMHO worth showing what that aProxyConcat = [].concat(aProxy) actually did:

> aConcat
[ 'x', 'y' ]
> aProxyConcat
[ [ 'x', 'y' ] ]

[billmark]

Thanks Tom for the quick and clear reply.

Is there an up-to-date list anywhere of all of the built-in routines that are currently "broken" for similar reasons? Such a list would make it easy for me to avoid filing bugs one on issues like this one that are (apparently) already known.

[tvcutsem]

There is no such list that I know of. A quick search through the ES5 spec for any text testing explicitly whether an object's [[Class]] is "Array" reveals the following useful matches:

• Array.isArray
• Array.prototype.concat
• JSON.parse
• JSON.stringify

Given that I already patch Array.isArray, it seems fair to also patch Array.prototype.concat.

Patching the JSON methods is more problematic. This entails doing the recursive parse/stringify in JS itself, thus entirely giving up on the built-in implementation (see also issue #13 )

[tvcutsem]

Fixed in v0.0.9.

Essentially, the patched Array.prototype.concat will scan its arguments for proxies-for-arrays. It "unwraps" such proxies by replacing the argument with a copy of the array, before calling the original Array.prototype.concat.

By creating a copy, we will trigger the same sequence of trap invocations on the proxy-for-array handler that would have occurred if the real Array.prototype.concat would have copied it.

The patched version should have little overhead compared to the built-in version if none of the arguments are proxies.

[billmark]

Thanks, Tom. I've grabbed v0.0.9 and it indeed fixes the issue we had.