As promised, changes reflected on the wiki: http://wiki.ecmascript.org/doku.php?id=harmony:proto_climbing_refactoring
Let me know if you spot any remaining errors. Cheers, Tom 2012/4/27 Tom Van Cutsem <[email protected]> > 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 <[email protected]> > >> 2012/4/24 Jeff Walden <[email protected]> >> >>> 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 >>> [email protected] >>> https://mail.mozilla.org/listinfo/es-discuss >>> >> >> >
_______________________________________________ es-discuss mailing list [email protected] https://mail.mozilla.org/listinfo/es-discuss
