This is already looking good. Just a few comments.

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js
File src/collection.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js#newcode91
src/collection.js:91: function SetSize() {
I think "SetGetSize" or "SetSizeGetter" would be a better naming
convention for getters.

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js#newcode157
src/collection.js:157: function MapSize() {
Likewise for "MapGetSize" or "MapSizeGetter".

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js#newcode235
src/collection.js:235: function DefineGetter(object, name, fun) {
Can we move this into v8natives.js near the implementation of
InstallFunctions, so that it can become the bottle-neck for all builtin
getters that we install?

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmony/collections.js
File test/mjsunit/harmony/collections.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmony/collections.js#newcode319
test/mjsunit/harmony/collections.js:319: // Set and Map Set size
Comment seems to contain a typo.

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmony/collections.js#newcode324
test/mjsunit/harmony/collections.js:324:
assertFalse(setSizeDescriptor.enumerable);
I couldn't find the expected value of the attributes (i.e. enumerable
and configurable) in the draft spec. Is that still unspeced?

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmony/collections.js#newcode343
test/mjsunit/harmony/collections.js:343:
assertFalse(mapSizeDescriptor.enumerable);
Likewise.

https://chromiumcodereview.appspot.com/11360089/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to