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

Issue 345613003: Map/Set: Implement constructor parameter handling (Closed)

Created:
5 years ago by arv (Not doing code reviews)
Modified:
5 years ago
Reviewers:
adamk, rossberg
CC:
v8-dev
Visibility:
Public.

Description

Map/Set: Implement constructor parameter handling When an iterable object is passed in as the argument to the Map and Set constructor the elements of the iterable object are used to populate the Map and Set. http://people.mozilla.org/~jorendorff/es6-draft.html#sec-map-iterable http://people.mozilla.org/~jorendorff/es6-draft.html#sec-set-iterable BUG=v8:3398 LOG=Y R=rossberg@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=21950

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : Added test where set/add is replaced during set/add #

Total comments: 11

Patch Set 4 : Code review updates #

Patch Set 5 : fix typo #

Patch Set 6 : ascii art #

Unified diffs Side-by-side diffs Delta from patch set Stats (+345 lines, -10 lines) Patch
M src/collection.js View 1 2 3 2 chunks +81 lines, -8 lines 0 comments Download
M src/messages.js View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M test/mjsunit/harmony/collections.js View 1 2 3 2 chunks +259 lines, -1 line 0 comments Download
M tools/generate-runtime-tests.py View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 14 (0 generated)
arv (Not doing code reviews)
5 years ago (2014-06-18 19:20:17 UTC) #1
adamk
Code looks fine (especially so since it closely mirrors the spec), one test suggestion. https://codereview.chromium.org/345613003/diff/20001/test/mjsunit/harmony/collections.js ...
5 years ago (2014-06-18 20:43:27 UTC) #2
arv (Not doing code reviews)
On 2014/06/18 20:43:27, adamk wrote: > Code looks fine (especially so since it closely mirrors ...
5 years ago (2014-06-18 20:51:19 UTC) #3
arv (Not doing code reviews)
Added test where set/add is replaced during set/add
5 years ago (2014-06-18 21:01:16 UTC) #4
arv (Not doing code reviews)
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 ) This should probably ...
5 years ago (2014-06-18 21:02:06 UTC) #5
arv (Not doing code reviews)
Added one more test as suggested.
5 years ago (2014-06-18 21:03:04 UTC) #6
rossberg
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/18 21:02:05, ...
5 years ago (2014-06-23 14:16:00 UTC) #7
arv (Not doing code reviews)
Code review updates
5 years ago (2014-06-23 15:36:36 UTC) #8
arv (Not doing code reviews)
fix typo
5 years ago (2014-06-23 15:40:35 UTC) #9
arv (Not doing code reviews)
ascii art
5 years ago (2014-06-23 15:42:08 UTC) #10
arv (Not doing code reviews)
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 ...
5 years ago (2014-06-23 15:43:57 UTC) #11
rossberg
lgtm
5 years ago (2014-06-23 16:12:37 UTC) #12
arv (Not doing code reviews)
Would you mind landing this when you have time? Thanks.
5 years ago (2014-06-23 16:14:57 UTC) #13
adamk
5 years ago (2014-06-23 18:06:03 UTC) #14
Message was sent while issue was closed.
Committed patchset #6 manually as r21950 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698