I haven't uploaded a new snapshot yet. I got stuck on the following:
var length = 2;
var a = [1];
assertEquals(0, Array.prototype.length);
a[Symbol.unscopables] = {length: true};
with (a) {
assertEquals(0, length); // FAILS
}
The problem seems to come down to length being implemented as an ACCESSOR
and it
seems to always return the instance (receiver) length even when:
LookupIterator it(receiver, name, holder);
// (arrayInstance, "length", ArrayPrototype)
I guess I can detect if the ACCESSOR is backed by a getter or not and
change to
get the property direct from the holder if it isn't.
This feels hacky to me. Maybe I am missing something?
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h
File src/unscopables.h (right):
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h#newcode20
src/unscopables.h:20: class UnscopableLookup {
On 2014/07/17 15:12:13, rossberg wrote:
Hm, this class seems overkill to me. It should be adequate enough to
just make
it a static function like
bool UnscopableLookup(Isolate*,
Handle<JSReceiver> object, Handle<String> name,
Handle<JSReceiver>* holder, PropertyAttributes* attr, bool* ok)
Done.
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h#newcode55
src/unscopables.h:55: JSReceiver::GetOwnPropertyAttributes(object,
name_);
On 2014/07/17 15:12:13, rossberg wrote:
Shouldn't this call HasOwnProperty?
The caller needs the PropertyAttributes.
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h#newcode59
src/unscopables.h:59: JSReceiver::GetOwnPropertyAttributes(object,
unscopables_symbol);
On 2014/07/17 15:12:13, rossberg wrote:
Same here?
This one can be changed though.
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h#newcode72
src/unscopables.h:72: blocked_attrs =
JSReceiver::GetOwnPropertyAttributes(
On 2014/07/17 15:12:13, rossberg wrote:
And here?
This can also be changed.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/proxies-with-unscopables.js
File test/mjsunit/harmony/proxies-with-unscopables.js (right):
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/proxies-with-unscopables.js#newcode6
test/mjsunit/harmony/proxies-with-unscopables.js:6: // Flags:
--harmony-proxies
On 2014/07/17 15:12:13, rossberg wrote:
Hm, shouldn't there also be some tests that actually define
@@unscopables on a
proxy, and one that uses a proxy as @@unscopables?
Like I said in a comment on the CL. Proxies cannot have symbols as
property names.
I'll add a TODO.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js
File test/mjsunit/harmony/unscopables.js (right):
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode11
test/mjsunit/harmony/unscopables.js:11: function runtTest(f) {
On 2014/07/17 15:12:13, rossberg wrote:
Nit: s/runtTest/runTest/
Done.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode23
test/mjsunit/harmony/unscopables.js:23: case 0: return {};
On 2014/07/17 15:12:13, rossberg wrote:
Why not simply use an array? Then you don't need to hardcode 4 below
either.
These needs to be new objects every time but I agree that the switch is
an unnecessary optimization.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode39
test/mjsunit/harmony/unscopables.js:39: var proto = getObject(i);
On 2014/07/17 15:12:13, rossberg wrote:
getObject(j) ? Otherwise, you don't execute anything. ;)
wow. That refactoring just completely failed.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode43
test/mjsunit/harmony/unscopables.js:43: f(getObject(i), getObject(j));
On 2014/07/17 15:12:13, rossberg wrote:
f(object, proto) ?
Done.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode180
test/mjsunit/harmony/unscopables.js:180: function
TestChangeDuringWithWithPossibleOptimization(object) {
On 2014/07/17 15:12:13, rossberg wrote:
I had in mind a test like the following:
var x = 1
object.x = 2
with (object) {
for (var i = 0; i < 10000; i++) {
if (i === 500) object[Symbol.unscopables] = {x: true}
assertEquals(i < 500 ? 1 : 2, x);
}
}
and dually,
var x = 1
object.x = 2
object[Symbol.unscopables] = {x: true}
with (object) {
for (var i = 0; i < 10000; i++) {
if (i === 500) delete object[Symbol.unscopables]
assertEquals(i < 500 ? 2 : 1, x);
}
}
and perhaps also the variants
var x = 1
object.x = 2
object[Symbol.unscopables] = {}
with (object) {
for (var i = 0; i < 10000; i++) {
if (i === 500) object[Symbol.unscopables].x = true
assertEquals(i < 500 ? 2 : 1, x);
}
}
and
var x = 1
object.x = 2
object[Symbol.unscopables] = {x: true}
with (object) {
for (var i = 0; i < 10000; i++) {
if (i === 500) delete object[Symbol.unscopables].x
assertEquals(i < 500 ? 2 : 1, x);
}
}
The point being that lookup is always for the same variable reference
`x` in the
code -- which changes its meaning over time, however.
Thanks. Added.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode354
test/mjsunit/harmony/unscopables.js:354: assertTrue(false);
On 2014/07/17 15:12:13, rossberg wrote:
assertUnreachable()
Done.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode383
test/mjsunit/harmony/unscopables.js:383: x = 3;
On 2014/07/17 15:12:13, rossberg wrote:
Yeah, what this does is really weird...
Acknowledged.
https://codereview.chromium.org/384963002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
