Adjust key behaviour for weak collections

R=dslomov@chromium.org
BUG=460083,v8:3970,v8:3971
LOG=Y

Review URL: https://codereview.chromium.org/1018923002

Cr-Commit-Position: refs/heads/master@{#27272}

diff --git a/src/weak-collection.js b/src/weak-collection.js
index 6266fb8..776043d 100644 (file)
--- a/src/weak-collection.js
+++ b/src/weak-collection.js
@@ -42,9 +42,7 @@ function WeakMapGet(key) {
     throw MakeTypeError('incompatible_method_receiver',
                         ['WeakMap.prototype.get', this]);
   }
-  if (!(IS_SPEC_OBJECT(key) || IS_SYMBOL(key))) {
-    throw %MakeTypeError('invalid_weakmap_key', [this, key]);
-  }
+  if (!IS_SPEC_OBJECT(key)) return UNDEFINED;
   return %WeakCollectionGet(this, key);
 }
 
@@ -54,7 +52,7 @@ function WeakMapSet(key, value) {
     throw MakeTypeError('incompatible_method_receiver',
                         ['WeakMap.prototype.set', this]);
   }
-  if (!(IS_SPEC_OBJECT(key) || IS_SYMBOL(key))) {
+  if (!IS_SPEC_OBJECT(key)) {
     throw %MakeTypeError('invalid_weakmap_key', [this, key]);
   }
   return %WeakCollectionSet(this, key, value);
@@ -66,9 +64,7 @@ function WeakMapHas(key) {
     throw MakeTypeError('incompatible_method_receiver',
                         ['WeakMap.prototype.has', this]);
   }
-  if (!(IS_SPEC_OBJECT(key) || IS_SYMBOL(key))) {
-    throw %MakeTypeError('invalid_weakmap_key', [this, key]);
-  }
+  if (!IS_SPEC_OBJECT(key)) return false;
   return %WeakCollectionHas(this, key);
 }
 
@@ -78,9 +74,7 @@ function WeakMapDelete(key) {
     throw MakeTypeError('incompatible_method_receiver',
                         ['WeakMap.prototype.delete', this]);
   }
-  if (!(IS_SPEC_OBJECT(key) || IS_SYMBOL(key))) {
-    throw %MakeTypeError('invalid_weakmap_key', [this, key]);
-  }
+  if (!IS_SPEC_OBJECT(key)) return false;
   return %WeakCollectionDelete(this, key);
 }
 
@@ -135,7 +129,7 @@ function WeakSetAdd(value) {
     throw MakeTypeError('incompatible_method_receiver',
                         ['WeakSet.prototype.add', this]);
   }
-  if (!(IS_SPEC_OBJECT(value) || IS_SYMBOL(value))) {
+  if (!IS_SPEC_OBJECT(value)) {
     throw %MakeTypeError('invalid_weakset_value', [this, value]);
   }
   return %WeakCollectionSet(this, value, true);
@@ -147,9 +141,7 @@ function WeakSetHas(value) {
     throw MakeTypeError('incompatible_method_receiver',
                         ['WeakSet.prototype.has', this]);
   }
-  if (!(IS_SPEC_OBJECT(value) || IS_SYMBOL(value))) {
-    throw %MakeTypeError('invalid_weakset_value', [this, value]);
-  }
+  if (!IS_SPEC_OBJECT(value)) return false;
   return %WeakCollectionHas(this, value);
 }
 
@@ -159,9 +151,7 @@ function WeakSetDelete(value) {
     throw MakeTypeError('incompatible_method_receiver',
                         ['WeakSet.prototype.delete', this]);
   }
-  if (!(IS_SPEC_OBJECT(value) || IS_SYMBOL(value))) {
-    throw %MakeTypeError('invalid_weakset_value', [this, value]);
-  }
+  if (!IS_SPEC_OBJECT(value)) return false;
   return %WeakCollectionDelete(this, value);
 }
 

diff --git a/test/mjsunit/es6/collections.js b/test/mjsunit/es6/collections.js
index 989ded8..888b686 100644 (file)
--- a/test/mjsunit/es6/collections.js
+++ b/test/mjsunit/es6/collections.js
@@ -51,6 +51,21 @@ function TestValidMapCalls(m) {
   assertDoesNotThrow(function () { m.set(new Object) });
   assertDoesNotThrow(function () { m.has(new Object) });
   assertDoesNotThrow(function () { m.delete(new Object) });
+  assertDoesNotThrow(function () { m.get(undefined) });
+  assertDoesNotThrow(function () { m.get(null) });
+  assertDoesNotThrow(function () { m.get(0) });
+  assertDoesNotThrow(function () { m.get('a-key') });
+  assertDoesNotThrow(function () { m.get(Symbol()) });
+  assertDoesNotThrow(function () { m.has(undefined) });
+  assertDoesNotThrow(function () { m.has(null) });
+  assertDoesNotThrow(function () { m.has(0) });
+  assertDoesNotThrow(function () { m.has('a-key') });
+  assertDoesNotThrow(function () { m.has(Symbol()) });
+  assertDoesNotThrow(function () { m.delete(undefined) });
+  assertDoesNotThrow(function () { m.delete(null) });
+  assertDoesNotThrow(function () { m.delete(0) });
+  assertDoesNotThrow(function () { m.delete('a-key') });
+  assertDoesNotThrow(function () { m.delete(Symbol()) });
 }
 TestValidMapCalls(new Map);
 TestValidMapCalls(new WeakMap);
@@ -58,14 +73,11 @@ TestValidMapCalls(new WeakMap);
 
 // Test invalid getter and setter calls for WeakMap only
 function TestInvalidCalls(m) {
-  assertThrows(function () { m.get(undefined) }, TypeError);
   assertThrows(function () { m.set(undefined, 0) }, TypeError);
-  assertThrows(function () { m.get(null) }, TypeError);
   assertThrows(function () { m.set(null, 0) }, TypeError);
-  assertThrows(function () { m.get(0) }, TypeError);
   assertThrows(function () { m.set(0, 0) }, TypeError);
-  assertThrows(function () { m.get('a-key') }, TypeError);
   assertThrows(function () { m.set('a-key', 0) }, TypeError);
+  assertThrows(function () { m.set(Symbol(), 0) }, TypeError);
 }
 TestInvalidCalls(new WeakMap);
 
@@ -73,57 +85,79 @@ TestInvalidCalls(new WeakMap);
 // Test expected behavior for Sets and WeakSets
 function TestSet(set, key) {
   assertFalse(set.has(key));
-  assertSame(set, set.add(key));
-  assertTrue(set.has(key));
-  assertTrue(set.delete(key));
+  assertFalse(set.delete(key));
+  if (typeof key === 'object' && !(set instanceof WeakSet)) {
+    assertSame(set, set.add(key));
+    assertTrue(set.has(key));
+    assertTrue(set.delete(key));
+  }
   assertFalse(set.has(key));
   assertFalse(set.delete(key));
   assertFalse(set.has(key));
 }
 function TestSetBehavior(set) {
+  // Fill
   for (var i = 0; i < 20; i++) {
     TestSet(set, new Object);
     TestSet(set, i);
     TestSet(set, i / 100);
     TestSet(set, 'key-' + i);
+    TestSet(set, Symbol(i));
   }
-  var keys = [ +0, -0, +Infinity, -Infinity, true, false, null, undefined ];
+
+  var keys = [
+    -0, +0, 1, 1/3, 10, +Infinity, -Infinity, NaN, true, false, null, undefined,
+    'x', Symbol(), {}, function(){}
+  ];
   for (var i = 0; i < keys.length; i++) {
     TestSet(set, keys[i]);
   }
 }
 TestSetBehavior(new Set);
-TestSet(new WeakSet, new Object);
+TestSetBehavior(new WeakSet);
 
 
 // Test expected mapping behavior for Maps and WeakMaps
 function TestMapping(map, key, value) {
-  assertSame(map, map.set(key, value));
-  assertSame(value, map.get(key));
+  assertFalse(map.has(key));
+  assertSame(undefined, map.get(key));
+  assertFalse(map.delete(key));
+  if (typeof key === 'object' && !(map instanceof WeakMap)) {
+    assertSame(map, map.set(key, value));
+    assertSame(value, map.get(key));
+    assertTrue(map.has(key));
+    assertTrue(map.delete(key));
+  }
+  assertFalse(map.has(key));
+  assertSame(undefined, map.get(key));
+  assertFalse(map.delete(key));
+  assertFalse(map.has(key));
+  assertSame(undefined, map.get(key));
 }
-function TestMapBehavior1(m) {
+function TestMapBehavior(m) {
+  // Fill
   TestMapping(m, new Object, 23);
   TestMapping(m, new Object, 'the-value');
   TestMapping(m, new Object, new Object);
-}
-TestMapBehavior1(new Map);
-TestMapBehavior1(new WeakMap);
-
-
-// Test expected mapping behavior for Maps only
-function TestMapBehavior2(m) {
   for (var i = 0; i < 20; i++) {
     TestMapping(m, i, new Object);
     TestMapping(m, i / 10, new Object);
     TestMapping(m, 'key-' + i, new Object);
+    TestMapping(m, Symbol(i), new Object);
   }
-  // -0 is handled in TestMinusZeroMap
-  var keys = [ 0, +Infinity, -Infinity, true, false, null, undefined ];
+
+  var keys = [
+    -0, +0, 1, 1/3, 10, +Infinity, -Infinity, NaN, true, false, null, undefined,
+    'x', Symbol(), {}, function(){}
+  ];
   for (var i = 0; i < keys.length; i++) {
+    TestMapping(m, keys[i], 23);
+    TestMapping(m, keys[i], 'the-value');
     TestMapping(m, keys[i], new Object);
   }
 }
-TestMapBehavior2(new Map);
+TestMapBehavior(new Map);
+TestMapBehavior(new WeakMap);
 
 
 // Test expected querying behavior of Maps and WeakMaps
@@ -132,8 +166,6 @@ function TestQuery(m) {
   var values = [ 'x', 0, +Infinity, -Infinity, true, false, null, undefined ];
   for (var i = 0; i < values.length; i++) {
     TestMapping(m, key, values[i]);
-    assertTrue(m.has(key));
-    assertFalse(m.has(new Object));
   }
 }
 TestQuery(new Map);
@@ -144,7 +176,6 @@ TestQuery(new WeakMap);
 function TestDelete(m) {
   var key = new Object;
   TestMapping(m, key, 'to-be-deleted');
-  assertTrue(m.delete(key));
   assertFalse(m.delete(key));
   assertFalse(m.delete(new Object));
   assertSame(m.get(key), undefined);

diff --git a/test/mjsunit/es6/symbols.js b/test/mjsunit/es6/symbols.js
index b9811f5..46c3dab 100644 (file)
--- a/test/mjsunit/es6/symbols.js
+++ b/test/mjsunit/es6/symbols.js
@@ -245,25 +245,20 @@ TestCall()
 function TestCollections() {
   var set = new Set
   var map = new Map
-  var weakmap = new WeakMap
   for (var i in symbols) {
     set.add(symbols[i])
     map.set(symbols[i], i)
-    weakmap.set(symbols[i], i)
   }
   assertEquals(symbols.length, set.size)
   assertEquals(symbols.length, map.size)
   for (var i in symbols) {
     assertTrue(set.has(symbols[i]))
     assertTrue(map.has(symbols[i]))
-    assertTrue(weakmap.has(symbols[i]))
     assertEquals(i, map.get(symbols[i]))
-    assertEquals(i, weakmap.get(symbols[i]))
   }
   for (var i in symbols) {
     assertTrue(set.delete(symbols[i]))
     assertTrue(map.delete(symbols[i]))
-    assertTrue(weakmap.delete(symbols[i]))
   }
   assertEquals(0, set.size)
   assertEquals(0, map.size)

diff --git a/test/mjsunit/harmony/private.js b/test/mjsunit/harmony/private.js
index 218094c..c08daf1 100644 (file)
--- a/test/mjsunit/harmony/private.js
+++ b/test/mjsunit/harmony/private.js
@@ -196,25 +196,20 @@ TestSet()
 function TestCollections() {
   var set = new Set
   var map = new Map
-  var weakmap = new WeakMap
   for (var i in symbols) {
     set.add(symbols[i])
     map.set(symbols[i], i)
-    weakmap.set(symbols[i], i)
   }
   assertEquals(symbols.length, set.size)
   assertEquals(symbols.length, map.size)
   for (var i in symbols) {
     assertTrue(set.has(symbols[i]))
     assertTrue(map.has(symbols[i]))
-    assertTrue(weakmap.has(symbols[i]))
     assertEquals(i, map.get(symbols[i]))
-    assertEquals(i, weakmap.get(symbols[i]))
   }
   for (var i in symbols) {
     assertTrue(set.delete(symbols[i]))
     assertTrue(map.delete(symbols[i]))
-    assertTrue(weakmap.delete(symbols[i]))
   }
   assertEquals(0, set.size)
   assertEquals(0, map.size)