Proto-walking proposal [[SetP]] comments

Tom Van Cutsem tomvc.be at gmail.com
Fri Apr 27 03:51:27 PDT 2012


Jeff,

I implemented your proposed algorithm in Javascript and ran it against my
earlier test framework. It passed all cases. I had to add explicit checks
for non-extensibility, otherwise Object.defineProperty would throw. See:
http://code.google.com/p/es-lab/source/browse/trunk/src/es5adapt/setProperty.js?spec=svn678&r=678#114

If no one raises any further objections, I'll add the updated algorithm to
the wiki page.

Cheers,
Tom

2012/4/25 Tom Van Cutsem <tomvc.be at gmail.com>

> 2012/4/24 Jeff Walden <jwalden+es at mit.edu>
>
>> In the [[SetP]] implementation on this page:
>>
>> http://wiki.ecmascript.org/doku.php?id=harmony:proto_climbing_refactoring
>>
>> In step 2, the property lookup should stop when a data descriptor of any
>> sort, writable or non-writable is uncovered.  A property closer to the
>> start of a lookup shadows one further along the prototype chain, and these
>> semantics don't preserve that.
>>
>
> Right.
>
>
>> Step 5c should return true after calling the setter.
>>
>
> Right again.
>
>
>> Step 5d(i) to recheck for extensibility is redundant with
>> [[DefineOwnProperty]]'s check of the same.
>>
>
> This is true, except the difference is observable if Receiver is a proxy:
> with the current algorithm, that proxy's defineProperty trap will not be
> called. With the extensibility check removed, it will.
>
> The step 5d(i) check was inspired by the corresponding step 4 check in ES5
> 8.12.4 [[CanPut]] (the spec for [[SetP]] was based on the structure of
> [[CanPut]]). In other words: in ES5, technically, the extensibility check
> is also performed twice: once in [[CanPut]] and once when
> [[DefineOwnProperty]] is called in [[Put]].
>
> I would argue to keep the redundant check in there to avoid spurious proxy
> trap invocations, but I don't feel strongly about this (the invariant
> enforcement mechanism will force a non-extensible proxy's defineProperty
> trap to return a consistent return value anyway).
>
>
>> Technically, only step 2 needs to be changed in order to actually make
>> the logic sane on the first point.  And the second point could be fixed
>> with a one-line addition, and the third with a one-line removal.  But the
>> algorithm's unwieldy enough with just adding more steps (particularly to
>> step 2), I think you want a somewhat broader refactoring.  I make this
>> proposal:
>>
>> [[SetP]](Receiver, P, V)
>> When the [[SetP]] internal method of O is called with initial receiver
>> Receiver, property name P, and value V, the following steps are taken:
>>
>> 1. Let ownDesc be the result of calling the [[GetOwnProperty]] internal
>> method of O with argument P.
>> 2. If ownDesc is not undefined, then
>>   a. If IsAccessorDescriptor(ownDesc) is true, then
>>      i.   Let setter be ownDesc.[[Set]].
>>      ii.  If setter is undefined, return false.
>>      iii. Call the [[Call]] internal method of setter providing Receiver
>> as the this value and providing V as the sole argument.
>>      iv.  Return true.
>>   b. Otherwise IsDataDescriptor(ownDesc) must be true.
>>      i.   If ownDesc.[[Writable]] is false, return false.
>>      ii.  If Receiver === O, then
>>           1. Let updateDesc be the Property Descriptor { [[Value]]: V }.
>>           2. Return the result of calling the [[DefineOwnProperty]]
>> internal method of Receiver passing P, updateDesc, and false as arguments.
>>      iii. Else
>>           1. Let newDesc be the Property Descriptor {[[Value]]: V,
>> [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}.
>>           2. Return the result of calling the [[DefineOwnProperty]]
>> internal method of Receiver passing P, newDesc, and false as arguments.
>> 3. Let proto be the value of the [[Prototype]] internal property of O.
>> 4. If proto is null, then define the property on Receiver:
>>   a. Let newDesc be the Property Descriptor {[[Value]]: V, [[Writable]]:
>> true, [[Enumerable]]: true, [[Configurable]]: true}.
>>   b. Return the result of calling the [[DefineOwnProperty]] internal
>> method of Receiver passing P, newDesc, and false as arguments.
>> 5. Return the result of calling the [[SetP]] internal method of proto
>> with arguments Receiver, P, and V.
>>
>> Aside from fixing the noted bugs, this makes one further notable change.
>>  When the property lookup to determine whether there's a setting conflict
>> bottoms out at the end of the prototype chain, without finding the
>> property, this algorithm simple defines the property on the receiver as a
>> fully mutable property.  It doesn't reget the property on the receiver to
>> determine if anything's "changed", to set the property consistent with its
>> attributes at that instant.  First, this seems more efficient.  Under the
>> current algorithm any property miss must make an effort to reget the
>> original property, even "just in case".  Second, I have difficulty
>> imagining how changes would legitimately happen, in a way that we might
>> consider good coding style.  But perhaps I'm missing some reason why this
>> reget is a design requirement; please let me know if I've missed it.
>>
>
> Your alternative looks good to me. As noted above, I based myself on ES5
> [[Put]] which, because of the call to [[CanPut]], also did the lookup twice.
>
> The only observable case I can come up with is something along the lines
> of:
>
> var parent = Proxy({}, {
>   set: function(target, name, value, receiver) {
>     Object.defineProperty(receiver, name,
>       { value: 0,
>         writable: true,
>         enumerable: true,
>         configurable: false });
>     return Reflect.set(target, name, value, receiver);
>   }
> });
> var child = Object.create(parent);
> child.x = 1;
>
> The last line would trigger parent's "set" trap, which then defines a
> non-configurable 'x' on child, and continues the lookup by calling
> Reflect.set explicitly. This Reflect.set call reaches the root of the
> protochain and then tries to add 'x' again to 'child'. With an explicit
> re-check, this will update x's value from 0 to 1 (leaving other attributes
> unchanged). Without the re-check, adding 'x' as a configurable property
> fails since it's non-configurable.
>
> As you note, this is not a very motivating example for burdening all
> property assignments with redundant checks. In fact, I think the above
> behavior without the re-check is equally fine.
>
>
>> Anyway, comments welcome on this -- I'm working on implementing it now,
>> so feedback is particularly timely for me, and I'll be able to provide
>> implementation feedback quickly.
>>
>
> I previously wrote a little test page that compared the output of ES5's
> [[Put]] algorithm against the refactored version:
> http://es-lab.googlecode.com/svn/trunk/src/es5adapt/testSetProperty.html
> (source is here:
> http://code.google.com/p/es-lab/source/browse/#svn%2Ftrunk%2Fsrc%2Fes5adapt
>  )
>
> When I find the time I'll test your proposal to see if it passes.
>
> That said, any simplification we can make to this algorithm is welcome, so
> I like your changes. I would perhaps add a check for [[Extensible]] in step
> 4.b. to avoid trapping defineProperty on non-extensible proxy Receivers)
>
> Cheers,
> Tom
>
>
>>
>> Jeff
>> _______________________________________________
>> es-discuss mailing list
>> es-discuss at mozilla.org
>> https://mail.mozilla.org/listinfo/es-discuss
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20120427/40d30912/attachment.html>


More information about the es-discuss mailing list