PTAL
https://codereview.chromium.org/345613003/diff/50001/src/collection.js
File src/collection.js (right):
https://codereview.chromium.org/345613003/diff/50001/src/collection.js#newcode15
src/collection.js:15: // 7.4.1 CheckIterable ( obj )
On 2014/06/23 14:16:00, rossberg wrote:
On 2014/06/18 21:02:05, arv wrote:
> This should probably go in v8natives.js once this file is no longer
> experimental.
Agreed. Maybe add a TODO to that end?
We could also consider calling it ToIterable or something -- the spec
name is
really poorly chosen, and inconsistent with all other conversions. (I
already
filed that with one of my chapter bugs.)
Done.
https://codereview.chromium.org/345613003/diff/50001/src/collection.js#newcode187
src/collection.js:187: throw MakeTypeError('not_an_object', [next]);
On 2014/06/23 14:15:59, rossberg wrote:
We typically give more specific error messages in cases like these,
where the
type error is internal to some library function. Perhaps e.g.
'iterator result
... is not an object', or (below) 'iterator value ... is not an
entry'.
Great idea.
Done.
https://codereview.chromium.org/345613003/diff/50001/test/mjsunit/harmony/collections.js
File test/mjsunit/harmony/collections.js (right):
https://codereview.chromium.org/345613003/diff/50001/test/mjsunit/harmony/collections.js#newcode32
test/mjsunit/harmony/collections.js:32: var originalSetPrototypeAdd =
Set.prototype.add;
On 2014/06/23 14:16:00, rossberg wrote:
Nit: I'd prefer to localise these declarations into the respective
tests, so
that they are self-contained.
Done.
https://codereview.chromium.org/345613003/diff/50001/test/mjsunit/harmony/collections.js#newcode1037
test/mjsunit/harmony/collections.js:1037: var twoSet = new Set();
On 2014/06/23 14:16:00, rossberg wrote:
Nit: can't you simply use the array [1, 2] here? (Similarly, below.)
In particular, this would also test that the constructors are properly
generic.
OK. I didn't do that because I didn't want a dependency on
--harmony-iteration but I changed it based on your request.
Note that we have not yet landed Array.prototype[Symbol.iterator] so
these will have to use .values() for now.
https://codereview.chromium.org/345613003/diff/50001/test/mjsunit/harmony/collections.js#newcode1127
test/mjsunit/harmony/collections.js:1127: });
On 2014/06/23 14:16:00, rossberg wrote:
Assert TypeError, specifically.
Done.
https://codereview.chromium.org/345613003/
--
--
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.
