Issue 338323003: Add @@iterator to Array.prototype

Issue 338323003: Add @@iterator to Array.prototype (Closed)

Created:
6 years, 6 months ago by wingo

Modified:
6 years, 6 months ago

Reviewers:
arv (Not doing code reviews), rossberg

CC:
v8-dev, arv (Not doing code reviews)

Base URL:
https://v8.googlecode.com/svn/branches/bleeding_edge

Visibility:
Public.

More Reviews

Description

Add @@iterator to Array.prototype R=rossberg@chromium.org BUG= Committed: https://code.google.com/p/v8/source/detail?r=21993

Patch Set 2 : Fix @@iterator property attributes, add tests #

Total comments: 2

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+43 lines, -17 lines)			Patch
	M	src/array-iterator.js	View	1	2 chunks	+3 lines, -1 line	0 comments	Download
	M	test/mjsunit/harmony/array-iterator.js	View	1	4 chunks	+40 lines, -16 lines	2 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

arv (Not doing code reviews)

Can you add a test that makes sure that Array.prototype[Symbol.iterator] is configurable and writable? https://codereview.chromium.org/338323003/diff/1/src/array-iterator.js ...

6 years, 6 months ago (2014-06-17 15:39:37 UTC) #2

wingo

Updated to fix @@iterator property attributes and add tests. (Also removed one redundant test.)

6 years, 6 months ago (2014-06-18 08:53:40 UTC) #3

rossberg

LGTM with comment https://codereview.chromium.org/338323003/diff/20001/test/mjsunit/harmony/array-iterator.js File test/mjsunit/harmony/array-iterator.js (right): https://codereview.chromium.org/338323003/diff/20001/test/mjsunit/harmony/array-iterator.js#newcode236 test/mjsunit/harmony/array-iterator.js:236: assertEquals(array[i], buffer[i]); Use assertSame to avoid ...

6 years, 6 months ago (2014-06-18 09:25:45 UTC) #4

wingo

Committed patchset #2 manually as r21993 (presubmit successful).

6 years, 6 months ago (2014-06-25 07:33:41 UTC) #5

rossberg

https://codereview.chromium.org/338323003/diff/20001/test/mjsunit/harmony/array-iterator.js File test/mjsunit/harmony/array-iterator.js (right): https://codereview.chromium.org/338323003/diff/20001/test/mjsunit/harmony/array-iterator.js#newcode236 test/mjsunit/harmony/array-iterator.js:236: assertEquals(array[i], buffer[i]); On 2014/06/18 09:25:45, rossberg wrote: > Use ...

6 years, 6 months ago (2014-06-25 07:52:39 UTC) #6

wingo

6 years, 6 months ago (2014-06-25 08:26:19 UTC) #7

Message was sent while issue was closed.

On 2014/06/25 07:52:39, rossberg wrote:
>
https://codereview.chromium.org/338323003/diff/20001/test/mjsunit/harmony/arr...
> File test/mjsunit/harmony/array-iterator.js (right):
> 
>
https://codereview.chromium.org/338323003/diff/20001/test/mjsunit/harmony/arr...
> test/mjsunit/harmony/array-iterator.js:236: assertEquals(array[i], buffer[i]);
> On 2014/06/18 09:25:45, rossberg wrote:
> > Use assertSame to avoid the special case for NaN.
> 
> Just saw that you changed this to assertSame, but the point was that this
makes
> the special case that you (still) have for the last NaN unnecessary. ;)

Ah, now I see what you mean.  Good catch, will make another CL.

Expand Messages | Collapse Messages