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

Issue 8439069: Fix Harmony sets and maps to allow undefined as keys. (Closed)

Created:
9 years, 1 month ago by Michael Starzinger
Modified:
9 years, 1 month ago
Reviewers:
rossberg
CC:
v8-dev
Visibility:
Public.

Description

Fix Harmony sets and maps to allow undefined as keys. This uses a global sentinel as a replacement for undefined keys, which are not supported internally but required for Harmony sets and maps. R=rossberg@chromium.org BUG=v8:1622 TEST=mjsunit/harmony/collections Committed: http://code.google.com/p/v8/source/detail?r=9873

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed comments by Andreas Rossberg. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -3 lines) Patch
M src/collection.js View 3 chunks +26 lines, -0 lines 0 comments Download
M test/mjsunit/harmony/collections.js View 1 2 chunks +8 lines, -3 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Michael Starzinger
PTAL.
9 years, 1 month ago (2011-11-03 08:48:33 UTC) #1
rossberg
lgtm http://codereview.chromium.org/8439069/diff/1/test/mjsunit/harmony/collections.js File test/mjsunit/harmony/collections.js (right): http://codereview.chromium.org/8439069/diff/1/test/mjsunit/harmony/collections.js#newcode80 test/mjsunit/harmony/collections.js:80: TestSetBehavior(new Set); Can you add some tests for ...
9 years, 1 month ago (2011-11-03 14:14:31 UTC) #2
Michael Starzinger
9 years, 1 month ago (2011-11-03 14:34:21 UTC) #3
Added new patch set. Landed.

http://codereview.chromium.org/8439069/diff/1/test/mjsunit/harmony/collection...
File test/mjsunit/harmony/collections.js (right):

http://codereview.chromium.org/8439069/diff/1/test/mjsunit/harmony/collection...
test/mjsunit/harmony/collections.js:80: TestSetBehavior(new Set);
On 2011/11/03 14:14:31, rossberg wrote:
> Can you add some tests for sets with null and undefined?

Done.

Powered by Google App Engine
This is Rietveld 408576698