Re: Proto-walking proposal [[SetP]] comments

2012-05-03 Thread Tom Van Cutsem
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 tomvc...@gmail.com

 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=svn678r=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...@gmail.com

 2012/4/24 Jeff Walden jwalden...@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 

Re: Proto-walking proposal [[SetP]] comments

2012-04-27 Thread Tom Van Cutsem
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=svn678r=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...@gmail.com

 2012/4/24 Jeff Walden jwalden...@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
 

Re: Proto-walking proposal [[SetP]] comments

2012-04-25 Thread Tom Van Cutsem
2012/4/24 Jeff Walden jwalden...@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 

Proto-walking proposal [[SetP]] comments

2012-04-23 Thread Jeff Walden
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.

Step 5c should return true after calling the setter.

Step 5d(i) to recheck for extensibility is redundant with 
[[DefineOwnProperty]]'s check of the same.

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.

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.

Jeff
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss