Implement Object.keys for proxies.

Implement Object.keys for proxies.

R=kmillikin@chromium.org,ager@chromium.org
BUG=
TEST=

Committed: http://code.google.com/p/v8/source/detail?r=8626

[Mads Ager (chromium), 2011-07-12 13:28:19]

LGTM with a couple of performance and behavior questions.

http://codereview.chromium.org/7321004/diff/1/src/proxy.js
File src/proxy.js (right):

http://codereview.chromium.org/7321004/diff/1/src/proxy.js#newcode140
src/proxy.js:140: return this.getOwnPropertyNames().filter(
Is this specified in terms of filter and bind?

Is it supposed to work if the user overwrites filter, bind, getOwnPropertyNames,
getOwnPropertyDescriptor, etc?

Also, I think performance would be a lot better if you get the names in an array
and iterate through it directly.

http://codereview.chromium.org/7321004/diff/1/src/proxy.js#newcode142
src/proxy.js:142: return this.getOwnPropertyDescriptor('' + name).enumerable
Do you need the string conversion here? Does getOwnPropertyNames ever return
somthing that is not a string? If so, consider using TO_STRING_INLINE(name) to
make this faster.

http://codereview.chromium.org/7321004/diff/1/src/v8natives.js
File src/v8natives.js (right):

http://codereview.chromium.org/7321004/diff/1/src/v8natives.js#newcode315
src/v8natives.js:315: var names = keys.call(handler);
Do you want to rely on Function.prototype.call here or do you want to use
%_CallFunction(handler, keys)?

[rossberg, 2011-07-13 10:11:52]

http://codereview.chromium.org/7321004/diff/1/src/proxy.js
File src/proxy.js (right):

http://codereview.chromium.org/7321004/diff/1/src/proxy.js#newcode140
src/proxy.js:140: return this.getOwnPropertyNames().filter(
On 2011/07/12 13:28:19, Mads Ager wrote:
> Is this specified in terms of filter and bind?

Yes, although it is not clear whether that is what they really _mean_. It's in
my list of open questions.

> Is it supposed to work if the user overwrites filter, bind,
getOwnPropertyNames,
> getOwnPropertyDescriptor, etc?

Well, `this' is the handler, and the latter two are other traps on the same
handler, so that's correct. Wrt filter and bind, it's probably not what they
really want.

> Also, I think performance would be a lot better if you get the names in an
array
> and iterate through it directly.

Done.

http://codereview.chromium.org/7321004/diff/1/src/proxy.js#newcode142
src/proxy.js:142: return this.getOwnPropertyDescriptor('' + name).enumerable
On 2011/07/12 13:28:19, Mads Ager wrote:
> Do you need the string conversion here? Does getOwnPropertyNames ever return
> somthing that is not a string?

Yes, that's actually a minor inconsistency in the current spec (which doesn't
have the conversion) that I reported -- getOwnPropertyNames is a user trap, so
it can return anything.

There are a few inconsistencies like that in the spec...

> If so, consider using TO_STRING_INLINE(name) to
> make this faster.

Done, but I have a question: doing TO_STRING_INLINE(names[i]) kept segfaulting
on me, until I found out that I better do "var name = names[i];
TO_STRING_INLINE(name)" instead. Is that a known (documented?) limitation?

http://codereview.chromium.org/7321004/diff/1/src/v8natives.js
File src/v8natives.js (right):

http://codereview.chromium.org/7321004/diff/1/src/v8natives.js#newcode315
src/v8natives.js:315: var names = keys.call(handler);
On 2011/07/12 13:28:19, Mads Ager wrote:
> Do you want to rely on Function.prototype.call here or do you want to use
> %_CallFunction(handler, keys)?

Done, here and in a few other places. (Btw, who picked the argument order of
that built-in??)

[Mads Ager (chromium), 2011-07-13 10:17:25]

Still LGTM.

http://codereview.chromium.org/7321004/diff/1/src/proxy.js
File src/proxy.js (right):

http://codereview.chromium.org/7321004/diff/1/src/proxy.js#newcode142
src/proxy.js:142: return this.getOwnPropertyDescriptor('' + name).enumerable
On 2011/07/13 10:11:52, rossberg wrote:
> On 2011/07/12 13:28:19, Mads Ager wrote:
> > Do you need the string conversion here? Does getOwnPropertyNames ever return
> > somthing that is not a string?
> 
> Yes, that's actually a minor inconsistency in the current spec (which doesn't
> have the conversion) that I reported -- getOwnPropertyNames is a user trap, so
> it can return anything.
> 
> There are a few inconsistencies like that in the spec...
> 
> > If so, consider using TO_STRING_INLINE(name) to
> > make this faster.
> 
> Done, but I have a question: doing TO_STRING_INLINE(names[i]) kept segfaulting
> on me, until I found out that I better do "var name = names[i];
> TO_STRING_INLINE(name)" instead. Is that a known (documented?) limitation?

Yes, that is on purpose. The point is that TO_STRING_INLINE is a macro (defined
in macros.py) which has a fast case and a slow case. The macro duplicates the
argument and therefore we require it to be a variable to help ourselves avoid
repeating stuff with side effects.

http://codereview.chromium.org/7321004/diff/1/src/v8natives.js
File src/v8natives.js (right):

http://codereview.chromium.org/7321004/diff/1/src/v8natives.js#newcode315
src/v8natives.js:315: var names = keys.call(handler);
On 2011/07/13 10:11:52, rossberg wrote:
> On 2011/07/12 13:28:19, Mads Ager wrote:
> > Do you want to rely on Function.prototype.call here or do you want to use
> > %_CallFunction(handler, keys)?
> 
> Done, here and in a few other places. (Btw, who picked the argument order of
> that built-in??)

I don't remember. I would have preferred having the function first... :-)