Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(16)

Issue 259883002: ES6: Add support for Map and Set Iterator (Closed)

Created:
6 years, 8 months ago by arv (Not doing code reviews)
Modified:
6 years, 6 months ago
CC:
v8-dev
Visibility:
Public.

Description

ES6: Add support for values/keys/entries for Map and Set This allows code like this: var map = new Map(); map.set(1, 'One'); ... var iter = map.values(); var res; while (!(res = iter.next()).done) { print(res.value); } BUG=v8:1793 LOG=Y R=mstarzinger@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=21615

Patch Set 1 : cleanup #

Patch Set 2 : Update to support Symbol.iterator #

Patch Set 3 : Remove C++ code for Create iterator" #

Patch Set 4 : #

Patch Set 5 : Remove Symbol.iterator part #

Patch Set 6 : No need for harmony_symbol implication for now #

Total comments: 2

Patch Set 7 : Short license header #

Patch Set 8 : Add back @iterator part #

Total comments: 11

Patch Set 9 : Code review changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+427 lines, -191 lines) Patch
M src/bootstrapper.cc View 1 2 3 4 2 chunks +17 lines, -8 lines 0 comments Download
M src/collection.js View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
A src/collection-iterator.js View 1 2 3 4 5 6 7 8 1 chunk +162 lines, -0 lines 0 comments Download
M src/flag-definitions.h View 1 2 3 4 6 7 1 chunk +1 line, -0 lines 0 comments Download
M src/macros.py View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M src/objects.h View 1 2 3 4 3 chunks +0 lines, -14 lines 0 comments Download
M src/objects.cc View 1 2 3 4 3 chunks +0 lines, -24 lines 0 comments Download
M src/objects-debug.cc View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -4 lines 0 comments Download
M src/objects-inl.h View 1 2 3 4 1 chunk +0 lines, -14 lines 0 comments Download
M src/runtime.h View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M src/runtime.cc View 1 2 3 4 2 chunks +20 lines, -12 lines 0 comments Download
M test/cctest/test-api.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M test/cctest/test-ordered-hash-table.cc View 1 2 3 7 chunks +0 lines, -81 lines 0 comments Download
A test/mjsunit/harmony/collection-iterator.js View 1 2 3 4 5 6 7 8 1 chunk +195 lines, -0 lines 0 comments Download
M test/mjsunit/regress/regress-3281.js View 1 2 1 chunk +3 lines, -4 lines 0 comments Download
D test/mjsunit/runtime-gen/mapcreateiterator.js View 1 2 1 chunk +0 lines, -6 lines 0 comments Download
A + test/mjsunit/runtime-gen/mapiteratorinitialize.js View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M test/mjsunit/runtime-gen/mapiteratornext.js View 1 2 1 chunk +1 line, -1 line 0 comments Download
D test/mjsunit/runtime-gen/setcreateiterator.js View 1 2 1 chunk +0 lines, -6 lines 0 comments Download
A + test/mjsunit/runtime-gen/setiteratorinitialize.js View 1 2 1 chunk +4 lines, -2 lines 0 comments Download
M test/mjsunit/runtime-gen/setiteratornext.js View 1 2 1 chunk +1 line, -1 line 0 comments Download
M tools/generate-runtime-tests.py View 1 2 3 4 5 6 7 5 chunks +8 lines, -8 lines 0 comments Download
M tools/gyp/v8.gyp View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 29 (0 generated)
arv (Not doing code reviews)
cleanup
6 years, 8 months ago (2014-04-25 18:45:27 UTC) #1
arv (Not doing code reviews)
6 years, 8 months ago (2014-04-25 18:48:05 UTC) #2
arv (Not doing code reviews)
Toon, would you mind taking a look or suggest a better reviewer?
6 years, 7 months ago (2014-05-06 20:49:41 UTC) #3
arv (Not doing code reviews)
Changing back to Andreas since Toon is on/going on vacation.
6 years, 7 months ago (2014-05-07 18:45:11 UTC) #4
Michael Starzinger
This creates iterators for Maps and Sets that are never closed. IIUC this couples their ...
6 years, 7 months ago (2014-05-07 19:15:23 UTC) #5
rossberg
I agree with Michael, that seems too simplistic an implementation. We need a proper solution ...
6 years, 7 months ago (2014-05-08 09:17:48 UTC) #6
arv (Not doing code reviews)
On 2014/05/08 09:17:48, rossberg wrote: > I agree with Michael, that seems too simplistic an ...
6 years, 7 months ago (2014-05-08 13:11:50 UTC) #7
rossberg
On 2014/05/08 13:11:50, arv wrote: > The idea is to have a single linked list. ...
6 years, 7 months ago (2014-05-08 13:38:04 UTC) #8
Sven Panne
I'm working on weak stuff this quarter, so I'd like to chime in... :-) Adding ...
6 years, 7 months ago (2014-05-08 14:03:13 UTC) #9
rossberg
On 2014/05/08 14:03:13, Sven Panne wrote: > I'm working on weak stuff this quarter, so ...
6 years, 7 months ago (2014-05-08 14:05:28 UTC) #10
arv (Not doing code reviews)
On 2014/05/08 13:38:04, rossberg wrote: > Symbols will be the next thing we stage, so ...
6 years, 7 months ago (2014-05-08 16:26:57 UTC) #11
arv (Not doing code reviews)
On 2014/05/08 14:05:28, rossberg wrote: > On 2014/05/08 14:03:13, Sven Panne wrote: > > I'm ...
6 years, 7 months ago (2014-05-08 16:31:19 UTC) #12
arv (Not doing code reviews)
On 2014/05/08 13:38:04, rossberg wrote: > Off-hand this sounds plausible, although it looks like it ...
6 years, 7 months ago (2014-05-08 16:37:16 UTC) #13
rossberg
We threw around a lot of different ideas for avoiding complicated weakness schemes for maps ...
6 years, 7 months ago (2014-05-12 09:38:47 UTC) #14
rossberg
On 2014/05/12 09:38:47, rossberg wrote: > We threw around a lot of different ideas for ...
6 years, 7 months ago (2014-05-12 09:51:08 UTC) #15
arv (Not doing code reviews)
On 2014/05/12 09:51:08, rossberg wrote: > On 2014/05/12 09:38:47, rossberg wrote: > > We threw ...
6 years, 7 months ago (2014-05-12 17:52:00 UTC) #16
arv (Not doing code reviews)
Update to support Symbol.iterator
6 years, 7 months ago (2014-05-21 02:17:25 UTC) #17
arv (Not doing code reviews)
Remove Symbol.iterator part
6 years, 6 months ago (2014-05-28 19:15:57 UTC) #18
arv (Not doing code reviews)
No need for harmony_symbol implication for now
6 years, 6 months ago (2014-05-28 19:21:36 UTC) #19
arv (Not doing code reviews)
This is now ready for review again. The big difference from before is that the ...
6 years, 6 months ago (2014-05-28 19:23:20 UTC) #20
adamk
Mostly looks fine to me, happy to see some C++ code die. Only nits from ...
6 years, 6 months ago (2014-05-29 00:07:13 UTC) #21
arv (Not doing code reviews)
Short license header
6 years, 6 months ago (2014-05-29 17:21:35 UTC) #22
arv (Not doing code reviews)
PTAL
6 years, 6 months ago (2014-05-29 17:25:31 UTC) #23
arv (Not doing code reviews)
Add back @iterator part
6 years, 6 months ago (2014-05-29 21:00:29 UTC) #24
Michael Starzinger
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: ...
6 years, 6 months ago (2014-06-02 11:53:36 UTC) #25
rossberg
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); On 2014/06/02 11:53:36, Michael Starzinger wrote: ...
6 years, 6 months ago (2014-06-02 12:24:27 UTC) #26
arv (Not doing code reviews)
Code review changes
6 years, 6 months ago (2014-06-02 23:35:01 UTC) #27
arv (Not doing code reviews)
All fixed 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") On 2014/06/02 11:53:36, Michael ...
6 years, 6 months ago (2014-06-02 23:40:52 UTC) #28
adamk
6 years, 6 months ago (2014-06-03 00:34:12 UTC) #29
Message was sent while issue was closed.
Committed patchset #9 manually as r21615 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698