Bug 147925

Summary: [ES6] Implement Reflect.get
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ggaren, oliver, rniwa, saam, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ggaren: review+

Description Yusuke Suzuki 2015-08-11 23:32:05 PDT
[ES6] Implement Reflect.get
Comment 1 Yusuke Suzuki 2015-08-11 23:33:22 PDT
Created attachment 258809 [details]
Patch
Comment 2 Yusuke Suzuki 2015-08-11 23:34:28 PDT
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.
Comment 3 Geoffrey Garen 2015-08-17 11:16:42 PDT
Comment on attachment 258809 [details]
Patch

Should we be implementing these in JavaScript? Seems like it might be faster that way.
Comment 4 Yusuke Suzuki 2015-08-17 11:22:05 PDT
(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
Comment 5 Geoffrey Garen 2015-08-17 11:28:34 PDT
> 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.
Comment 6 Yusuke Suzuki 2015-08-17 11:46:34 PDT
(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 :)
Comment 7 Yusuke Suzuki 2015-08-17 11:56:56 PDT
Committed r188532: <http://trac.webkit.org/changeset/188532>