Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(53)

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

Created:
5 years, 1 month ago by wingo
Modified:
5 years ago
CC:
v8-dev, arv (Not doing code reviews)
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 1

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

Total comments: 2
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)
wingo
5 years, 1 month ago (2014-06-17 13:35:31 UTC) #1
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 ...
5 years ago (2014-06-17 15:39:37 UTC) #2
wingo
Updated to fix @@iterator property attributes and add tests. (Also removed one redundant test.)
5 years 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 ...
5 years ago (2014-06-18 09:25:45 UTC) #4
wingo
Committed patchset #2 manually as r21993 (presubmit successful).
5 years 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 ...
5 years ago (2014-06-25 07:52:39 UTC) #6
wingo
5 years 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.

Powered by Google App Engine
This is Rietveld 408576698