Declarative Environment Record needs to be update to reflect @@unscopable approach discussed at Sept 2013 TC39 meeting
https://github.com/rwaldron/tc39-notes/blob/master/es6/2013-09/sept-17.md#53-unscopeable
fixed in rev22 editor's draft
Note that an ordinary object is now used as the object containing the unscopable property names. they are the own property names of the unscopable object.
Here are the main spec. updates (to Rev21)
8.1.1.2.1 HasBinding(N)
The concrete Environment Record method HasBinding for object environment records determines if its associated binding object has a property whose name is the value of the argument N:
1. Let envRec be the object environment record for which the method was invoked.
2. Let bindings be the binding object for envRec.
3. If the withEnvironment flag of newEnv is true, then
a. Let unscopables be Get(bindings, @@unscopables).
b. ReturnIfAbrupt(unscopables).
c. If Type(unscopables) is Object, then
i. Let found be HasOwnProperty(unscopables, N).
ii. ReturnIfAbrupt(found).
iii. If found is true, then return false.
4. Return the result of HasProperty(bindings, N).
22.1.3.31 Array.prototype [ @@unscopables ]
The initial value of the @@unscopables data property is an object created by the following steps:
1. Let blackList be the result of calling ObjectCreate(%ObjectPrototype%).
2. Call CreateDataProperty(blackList, "find", true).
3. Call CreateDataProperty(blackList, "findIndex", true).
4. Call CreateDataProperty(blackList, "fill", true).
5. Call CreateDataProperty(blackList, "copyWithin", true).
6. Call CreateDataProperty(blackList, "entries", true).
7. Call CreateDataProperty(blackList, "keys", true).
8. Call CreateDataProperty(blackList, "values", true).
9. Assert: Each of the above calls will return true.
10. Return blackList.
fixed in Rev22 (January 20, 2013) release
Reopening since this is not in rev 24
The algorithms in #2 do not use an array. The discussion on es-discuss was using an array. I find using any Object and doing HasOwnProperty cleaner.
Another reason why using has property is better is that we do not need to invoke any getters or any toString methods.
The correct code was in rev21, but somehow got lost in rev22 and later. What is currently in the spec. is essentially what was in rev20.
(In reply to comment #6)
> The correct code was in rev21, but somehow got lost in rev22 and later. What
> is currently in the spec. is essentially what was in rev20.
I looked through rev 20, 21, 22 and the unscopables are never extracted in those revs either.
unscopables extraction was added in rev17 and then later removed in rev19.
(In reply to comment #8)
> unscopables extraction was added in rev17 and then later removed in rev19.
Just double checked and there is more spec text here but it has issues. (unscopables is never set, probably just a typo)
Upping the priority of this as discussed.
I would also appreciate the rationale for why @@unscopable cannot be an object.
If @@unscopable must be an array, please consider looking up entries in the @@unscopable array using GetOwn rather than Get as it's simpler.
Here is a gist with an update spec algorithm for 8.1.1.2.1 HasBinding(N)
https://gist.github.com/arv/f820c7c02a8119eb27a3
With this we can remove the notion that an ObjectEnvironment has an unscopables list since this is all localized to HasBinding.
The HasBinding algorithm in the gist retrieves @@unscopables even if `hasProperty` is false. Is this on purpose?
(In reply to comment #12)
> The HasBinding algorithm in the gist retrieves @@unscopables even if
> `hasProperty` is false. Is this on purpose?
You are right, this can be optimized to do one less HasOwnProperty call.
Fixed.
(In reply to comment #11)
> Here is a gist with an update spec algorithm for 8.1.1.2.1 HasBinding(N)
>
> https://gist.github.com/arv/f820c7c02a8119eb27a3
I see a problem with this design. Consider this ES5ish code snippet:
window.values = 100;
Array.prototype.__proto__ = {values: 42};
var obj = [ ];
with (obj) console.log(value); //should be 42;
In ES5, the HasBinding call on the with environment would find the property with the value 42. But, with Erik's changes we would we would log "100" because his HasBinding look up would stop when it saw an @@unscopable 'values' property on Array.prototype. This causes the with environment to be by passed and lexical lookup would climb the environment until it came to the global environment and finds 'values' with the value 100.
This issue might or might not be observable if 'values' was added to Object.prototype instead of splicing in a new prototype object. It would depend upon whether the global object also had a 'values' own property.
In practice we may never see this, but we're already adding a fair amount of complexity to maintain backwards compatability while adding these new methods. To fix it in HasBinding we would have to continue looking up the prototype chain even when isBlocked is true. Worse, we will also have to add a similar @@unscopable lookup algorithm to Object Environment Record's GetBindingValue.
I think we have to do it? Any other thoughts?
(In reply to comment #14)
> (In reply to comment #11)
> > Here is a gist with an update spec algorithm for 8.1.1.2.1 HasBinding(N)
> >
> > https://gist.github.com/arv/f820c7c02a8119eb27a3
>
> I see a problem with this design. Consider this ES5ish code snippet:
>
> window.values = 100;
> Array.prototype.__proto__ = {values: 42};
> var obj = [ ];
> with (obj) console.log(value); //should be 42;
>
> In ES5, the HasBinding call on the with environment would find the property
> with the value 42. But, with Erik's changes we would we would log "100" because
> his HasBinding look up would stop when it saw an @@unscopable 'values' property
> on Array.prototype. This causes the with environment to be by passed and
> lexical lookup would climb the environment until it came to the global
> environment and finds 'values' with the value 100.
>
> This issue might or might not be observable if 'values' was added to
> Object.prototype instead of splicing in a new prototype object. It would
> depend upon whether the global object also had a 'values' own property.
>
> In practice we may never see this, but we're already adding a fair amount of
> complexity to maintain backwards compatability while adding these new methods.
> To fix it in HasBinding we would have to continue looking up the prototype
> chain even when isBlocked is true. Worse, we will also have to add a similar
> @@unscopable lookup algorithm to Object Environment Record's GetBindingValue.
>
> I think we have to do it? Any other thoughts?
It is unfortunate that we have to do this dance in both HasBinding and GetBindingValue but I don't see any other way.
Let me think about it a bit more.
Do we need to update SetMutableBinding too? If not we get very strange results when the object has blacklisted a property that is then set.
(function TestSetterOnBlacklisted() {
var proto = {
set x(x) {
assertTrue(false);
},
get x() {
return 'proto';
}
};
var object = {
__proto__: proto,
get x() {
return this.x_;
},
set x(x) {
this.x_ = x;
},
x_: 1
};
with (object) {
x = 2;
assertEquals(x, 2);
}
assertEquals(object.x, 2);
object[Symbol.unscopables] = {x: true};
with (object) {
x = 3;
assertEquals(x, 'proto');
}
assertEquals(object.x, 3);
})();
(In reply to comment #16)
> Do we need to update SetMutableBinding too? If not we get very strange results
> when the object has blacklisted a property that is then set.
>
ugh!! The swamp just keeps getting deeper and deeper...
I think we probably have to do it, if we continue down this path. We would also have to consider the interaction with data property assignments when there is an inherited non-writable property that is @@unscoped
And this whole approach completely ignores that fact that the with object or any of its prototypes could be Proxy object that has redefined [[Get]] or [[Set]] and we may be imposing the a wrong lookup semantics on them.
At the F2F today we decided to back track this to plan a. This means that we do a HasProperty for name, if found we do a Get for @@unscopables and if found we do a HasProperty on the unscopables object.
https://gist.github.com/arv/f820c7c02a8119eb27a3
Allen, as far as I can tell we do not need to update SetMutableBinding anymore, since before the assignment, we will have already determined whether the property name has been black listed or not. Also, with plan a we always do a Get/Put on the object being the receiver so we do not get into the strange edge cases we got to before where a property on the instance was blocked but accepted further down the prototype type chain.
(sorry for not making the SetMtableBinding case clear today with the plan b approach)
fixed in rev27 editor's draft
fixed in rev27 draft