Bug 147925 – [ES6] Implement Reflect.get

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

[ES6] Implement Reflect.get

[Yusuke Suzuki]

Created attachment 258809 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 258809 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258809&action=review

> Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp:-154
> -}

This is only the place that does not use the PropertySlot::thisValue() as the |this| value.
But this code is not used anymore, so just drop it.

[Geoffrey Garen]

Comment on attachment 258809 [details]
Patch

Should we be implementing these in JavaScript? Seems like it might be faster that way.

[Yusuke Suzuki]

(In reply to comment #3)
> Comment on attachment 258809 [details]
> Patch
> 
> Should we be implementing these in JavaScript? Seems like it might be faster
> that way.

Reflect.get can take the receiver as its optional argument.
`Reflect.get ( target, propertyKey [ , receiver ])`

When the receiver is specified, we need to call the getter with the receiver as the |this|.
So, how about the mixed implementation?

in JS:

if (receiver === target)
    return target[propertyName];
return @getSlowCase(target, propertyKey, receiver);

in C++
the above code.

http://www.ecma-international.org/ecma-262/6.0/#sec-reflect.get

[Geoffrey Garen]

> When the receiver is specified, we need to call the getter with the receiver
> as the |this|.
> So, how about the mixed implementation?
> 
> in JS:
> 
> if (receiver === target)
>     return target[propertyName];
> return @getSlowCase(target, propertyKey, receiver);

Interesting.

Ryosuke is working on a similar problem, where the same behavior is required by super.getter.

I can see that it would be awkward to override the receiver in JS. Perhaps we should just go with your current patch for now.

[Yusuke Suzuki]

(In reply to comment #5)
> > When the receiver is specified, we need to call the getter with the receiver
> > as the |this|.
> > So, how about the mixed implementation?
> > 
> > in JS:
> > 
> > if (receiver === target)
> >     return target[propertyName];
> > return @getSlowCase(target, propertyKey, receiver);
> 
> Interesting.
> 
> Ryosuke is working on a similar problem, where the same behavior is required
> by super.getter.
> 
> I can see that it would be awkward to override the receiver in JS. Perhaps
> we should just go with your current patch for now.

Oh, that's very nice!
Maybe, super.setter = value; also has the same problem. Reflect.set has the receiver argument optionally.

Anyway, if Ryosuke is working on this "taking receiver optionally" by adding the byte code, when landing this change, we can easily use it for Reflect.get by adding the byte code intrinsics (maybe @get()?). This makes the Reflect.get written in JS (in the future) and seems cleanest way.

So agree with you. In the meantime, I'll land this current C++ implementation patch.
When the new byte code is introduced, we'll replace Reflect.get with the JS implementation :)

[Yusuke Suzuki]

Committed r188532: <http://trac.webkit.org/changeset/188532>