Proxies: wrong "receiver" used in default "set" trap

Tom Van Cutsem tomvc.be at gmail.com
Thu Dec 20 02:09:12 PST 2012


[+jwalden]

2012/12/19 Allen Wirfs-Brock <allen at wirfs-brock.com>

> Consider:
>
> let target = {foo() {return this}};
> let proxy = Proxy(target, {});
>
> As currently spec'ed.  proxy.foo() is going to return proxy rather than
> target.
>
> That's because,  proxy.foo evaluates to a Reference whose base is proxy.
>  Getting the value of the reference does a [[GetP]] that has proxy as both
> its this value and receiver value.  That is directly forwarded to target
> (as currently spec'ed: still with proxy as the receiver value), with which
> returns the function.   We then do a [[Call]] of the function using the
> base of the Reference (ie, proxy) as the this value.  So, for proxy.foo()
> we are not getting pure forwarding.
>
> If we make the Option A change that seems right for  [[GetP]]/[[SetP]]
> then we will have an inconsistency between the this value used for a method
> invoked as proxy.foo() and a accessor invoked as proxy.bar
>
> This issue arises because method lookup and method invocation is done in
> two separate steps.  I'm not sure there is an easy fix that doesn't involve
> introduce a [[CallProperty]] internal method.
>

Very good point!

Since adding a [[CallProperty]] (or [[Invoke]]) internal method is a big
change, I reconsidered how we could stick with the current semantics (where
|this| always remains bound to the proxy in all cases), yet avoid the
[[SetP]] anomaly.

The fix was obvious once I saw it, and involves only a minor change in
[[SetP]], and no changes at all in the forwarding behavior of proxies.

Consider the current [[SetP]] algorithm (I will use the version on p.70 of
your ES6 draft rev. 12 for reference)

The [[SetP]] algorithm first walks the proto chain in search for either a)
an inherited setter, or b) an inherited non-writable data property.
The moment we hit a writable data property, we can stop looking, and we are
allowed to add or update the data property to the initial receiver
object. The algorithm then just has to decide whether to update an existing
data property, or add a new one.

Currently, that test is performed in step 5.b. by testing whether the
"current" object we are visiting in the proto chain O is the Receiver
object. At first sight, this is a rather weird way of deciding between
update vs. addition. We can get away with this test because we've just
queried the O object for an own property (ownDesc). Hence, if O ===
Receiver, then we know Receiver already has the same property and we must
update, rather than add.

In the presence of proxies, this test is no longer valid, as Receiver may
be a proxy for O.

The right fix (at least, it feels like the obvious right fix to me), is not
to test whether O === Receiver, but rather just let the algorithm
explicitly test whether the Receiver object already defines an own property
with key P, to decide between update vs. addition:

replace step 5.b with:
5.b Let existingDesc be the result of calling the [[GetOwnProperty]]
internal method of Receiver with argument P.
5.c If existingDesc is not undefined, then
  ... // same steps as before

Now, for normal objects, this test is redundant since existingDesc and
ownDesc will denote the same property. But that redundancy is harmless,
just as in ES5 [[Put]] it was redundant to call both [[GetProperty]] and
[[GetOwnProperty]] on the same object. In the fast path, engines will not
follow this algorithm anyway. If Receiver is a proxy, then the difference
matters and the proxy will be able to intercept the call to
[[GetOwnProperty]] (which is what makes this revised algorithm work in the
presence of proxies).

Jeff, I'd appreciate your input as you previously helped revise the
[[SetP]] algorithm [1]. Actually, looking back at your message, it seems we
may have originally performed the redundant check, but removed it because
we couldn't see a reason not to.

In any case, I updated my little test-harness where I check conformance
between ES5 [[Put]] and ES6 [[SetP]], and with this change all tests
continue to pass (meaning that both algorithms yield the same result on
normal objects).

For those who rather read Javascript than spec language, there's a diff [2]
showing the changes in my harmony-reflect shim needed to fix the issue I
reported on earlier.

[1] https://mail.mozilla.org/pipermail/es-discuss/2012-April/022561.html
[2] http://git.io/p3jV0w

Cheers,
Tom
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20121220/1818ea11/attachment.html>


More information about the es-discuss mailing list