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 {
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)
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h#newcode55
src/unscopables.h:55: JSReceiver::GetOwnPropertyAttributes(object,
name_);
Shouldn't this call HasOwnProperty?
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h#newcode59
src/unscopables.h:59: JSReceiver::GetOwnPropertyAttributes(object,
unscopables_symbol);
Same here?
https://codereview.chromium.org/384963002/diff/100001/src/unscopables.h#newcode72
src/unscopables.h:72: blocked_attrs =
JSReceiver::GetOwnPropertyAttributes(
And here?
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
Hm, shouldn't there also be some tests that actually define
@@unscopables on a proxy, and one that uses a proxy as @@unscopables?
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) {
Nit: s/runtTest/runTest/
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode23
test/mjsunit/harmony/unscopables.js:23: case 0: return {};
Why not simply use an array? Then you don't need to hardcode 4 below
either.
Perhaps add a couple more cases, and maybe a few different cases of
property names as well. For example, the matrix that we test for O.o:
var objects = [
{},
[],
function(){},
(function(){ return arguments })(),
(function(){ "use strict"; return arguments })(),
Object(1), Object(true), Object("bla"),
new Date(),
Object, Function, Date, RegExp,
new Set, new Map, new WeakMap,
new ArrayBuffer(10), new Int32Array(5),
global
];
var properties = ["a", "1", 1, "length", "prototype", "name", "caller"];
Would also be good to have the @@unscopable object itself iterate
through the `objects` list. And to install the properties on the
@@unscopables as accessors as well (particular ones without getters). :)
You may think that this should obviously work, but you never know in
V8...
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode39
test/mjsunit/harmony/unscopables.js:39: var proto = getObject(i);
getObject(j) ? Otherwise, you don't execute anything. ;)
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode43
test/mjsunit/harmony/unscopables.js:43: f(getObject(i), getObject(j));
f(object, proto) ?
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode180
test/mjsunit/harmony/unscopables.js:180: function
TestChangeDuringWithWithPossibleOptimization(object) {
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.
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode354
test/mjsunit/harmony/unscopables.js:354: assertTrue(false);
assertUnreachable()
https://codereview.chromium.org/384963002/diff/100001/test/mjsunit/harmony/unscopables.js#newcode383
test/mjsunit/harmony/unscopables.js:383: x = 3;
Yeah, what this does is really weird...
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.
