LGTM from my end. Just a couple of remarks.
https://codereview.chromium.org/259883002/diff/160001/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/259883002/diff/160001/src/bootstrapper.cc#newcode2020
src/bootstrapper.cc:2020: INSTALL_EXPERIMENTAL_NATIVE(i, collections,
"collection-iterator.js")
Just out of curiosity, is there a particular reason for putting this
into a separate file? How would you feel about merging it into the
existing collection.js file?
https://codereview.chromium.org/259883002/diff/160001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/259883002/diff/160001/src/runtime.cc#newcode1597
src/runtime.cc:1597: holder->set_index(Smi::FromInt(0));
Remark: This gives a short window of time during which "index" and
"kind" are initialized to undefined (which is totally fine). The
JSSetIteratorVerify assumes those two fields are always Smi. In the
current implementation I don't see how anything in between the
allocation of an iterator and here could cause a heap verification to be
triggered.
I just wanted to leave this comment for posterity. Should there ever be
a change that causes a GC along the way from the constructor to here,
then the verification method needs to be loosened.
https://codereview.chromium.org/259883002/diff/160001/src/runtime.cc#newcode1701
src/runtime.cc:1701: holder->set_index(Smi::FromInt(0));
Likewise.
https://codereview.chromium.org/259883002/diff/160001/test/mjsunit/harmony/collection-iterator.js
File test/mjsunit/harmony/collection-iterator.js (right):
https://codereview.chromium.org/259883002/diff/160001/test/mjsunit/harmony/collection-iterator.js#newcode20
test/mjsunit/harmony/collection-iterator.js:20: assertEquals(new
Set().values().__proto__. SetIteratorPrototype);
typo: there is a period instead of a comma after "__proto__" here. Gotta
love JavaScript for that. :/
https://codereview.chromium.org/259883002/diff/160001/test/mjsunit/harmony/collection-iterator.js#newcode115
test/mjsunit/harmony/collection-iterator.js:115: assertEquals(new
Map().values().__proto__. MapIteratorPrototype);
typo: Likewise.
https://codereview.chromium.org/259883002/
--
--
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.
