Fix Harmony Maps and WeakMaps for undefined values.

R=rossberg@chromium.org
BUG=chromium:132744
TEST=mjsunit/harmony/collections

Review URL: https://chromiumcodereview.appspot.com/10658014

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11924 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/collection.js

-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
@@ -129,7 +129,7 @@ function MapHas(key) {
   if (IS_UNDEFINED(key)) {
     key = undefined_sentinel;
   }
-  return !IS_UNDEFINED(%MapGet(this, key));
+  return %MapHas(this, key);
 }
 
 
@@ -141,12 +141,7 @@ function MapDelete(key) {
   if (IS_UNDEFINED(key)) {
     key = undefined_sentinel;
   }
-  if (!IS_UNDEFINED(%MapGet(this, key))) {
-    %MapSet(this, key, void 0);
-    return true;
-  } else {
-    return false;
-  }
+  return %MapDelete(this, key);
 }
 
 
@@ -191,7 +186,7 @@ function WeakMapHas(key) {
   if (!IS_SPEC_OBJECT(key)) {
     throw %MakeTypeError('invalid_weakmap_key', [this, key]);
   }
-  return !IS_UNDEFINED(%WeakMapGet(this, key));
+  return %WeakMapHas(this, key);
 }
 
 
@@ -203,12 +198,7 @@ function WeakMapDelete(key) {
   if (!IS_SPEC_OBJECT(key)) {
     throw %MakeTypeError('invalid_weakmap_key', [this, key]);
   }
-  if (!IS_UNDEFINED(%WeakMapGet(this, key))) {
-    %WeakMapSet(this, key, void 0);
-    return true;
-  } else {
-    return false;
-  }
+  return %WeakMapDelete(this, key);
 }
 
 // -------------------------------------------------------------------

src/objects.cc

@@ -12931,11 +12931,11 @@ Object* ObjectHashTable::Lookup(Object* key) {
   // If the object does not have an identity hash, it was never used as a key.
   { MaybeObject* maybe_hash = key->GetHash(OMIT_CREATION);
     if (maybe_hash->ToObjectUnchecked()->IsUndefined()) {
-      return GetHeap()->undefined_value();
+      return GetHeap()->the_hole_value();
     }
   }
   int entry = FindEntry(key);
-  if (entry == kNotFound) return GetHeap()->undefined_value();
+  if (entry == kNotFound) return GetHeap()->the_hole_value();
   return get(EntryToIndex(entry) + 1);
 }
 
@@ -12952,7 +12952,7 @@ MaybeObject* ObjectHashTable::Put(Object* key, Object* value) {
   int entry = FindEntry(key);
 
   // Check whether to perform removal operation.
-  if (value->IsUndefined()) {
+  if (value->IsTheHole()) {
     if (entry == kNotFound) return this;
     RemoveEntry(entry);
     return Shrink(key);

src/objects.h

@@ -3325,12 +3325,12 @@ class ObjectHashTable: public HashTable<ObjectHashTableShape<2>, Object*> {
     return reinterpret_cast<ObjectHashTable*>(obj);
   }
 
-  // Looks up the value associated with the given key. The undefined value is
+  // Looks up the value associated with the given key. The hole value is
   // returned in case the key is not present.
   Object* Lookup(Object* key);
 
   // Adds (or overwrites) the value associated with the given key. Mapping a
-  // key to the undefined value causes removal of the whole entry.
+  // key to the hole value causes removal of the whole entry.
   MUST_USE_RESULT MaybeObject* Put(Object* key, Object* value);
 
  private:

src/runtime.cc

@@ -795,8 +795,35 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_MapGet) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 2);
   CONVERT_ARG_HANDLE_CHECKED(JSMap, holder, 0);
-  Handle<Object> key(args[1]);
-  return ObjectHashTable::cast(holder->table())->Lookup(*key);
+  CONVERT_ARG_HANDLE_CHECKED(Object, key, 1);
+  Handle<ObjectHashTable> table(ObjectHashTable::cast(holder->table()));
+  Handle<Object> lookup(table->Lookup(*key));
+  return lookup->IsTheHole() ? isolate->heap()->undefined_value() : *lookup;
+}
+
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_MapHas) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 2);
+  CONVERT_ARG_HANDLE_CHECKED(JSMap, holder, 0);
+  CONVERT_ARG_HANDLE_CHECKED(Object, key, 1);
+  Handle<ObjectHashTable> table(ObjectHashTable::cast(holder->table()));
+  Handle<Object> lookup(table->Lookup(*key));
+  return isolate->heap()->ToBoolean(!lookup->IsTheHole());
+}
+
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_MapDelete) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 2);
+  CONVERT_ARG_HANDLE_CHECKED(JSMap, holder, 0);
+  CONVERT_ARG_HANDLE_CHECKED(Object, key, 1);
+  Handle<ObjectHashTable> table(ObjectHashTable::cast(holder->table()));
+  Handle<Object> lookup(table->Lookup(*key));
+  Handle<ObjectHashTable> new_table =
+      PutIntoObjectHashTable(table, key, isolate->factory()->the_hole_value());
+  holder->set_table(*new_table);
+  return isolate->heap()->ToBoolean(!lookup->IsTheHole());
 }
 
 
@@ -804,8 +831,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_MapSet) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 3);
   CONVERT_ARG_HANDLE_CHECKED(JSMap, holder, 0);
-  Handle<Object> key(args[1]);
-  Handle<Object> value(args[2]);
+  CONVERT_ARG_HANDLE_CHECKED(Object, key, 1);
+  CONVERT_ARG_HANDLE_CHECKED(Object, value, 2);
   Handle<ObjectHashTable> table(ObjectHashTable::cast(holder->table()));
   Handle<ObjectHashTable> new_table = PutIntoObjectHashTable(table, key, value);
   holder->set_table(*new_table);
@@ -826,11 +853,38 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_WeakMapInitialize) {
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_WeakMapGet) {
-  NoHandleAllocation ha;
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 2);
+  CONVERT_ARG_HANDLE_CHECKED(JSWeakMap, weakmap, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, key, 1);
+  Handle<ObjectHashTable> table(ObjectHashTable::cast(weakmap->table()));
+  Handle<Object> lookup(table->Lookup(*key));
+  return lookup->IsTheHole() ? isolate->heap()->undefined_value() : *lookup;
+}
+
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_WeakMapHas) {
+  HandleScope scope(isolate);
   ASSERT(args.length() == 2);
   CONVERT_ARG_HANDLE_CHECKED(JSWeakMap, weakmap, 0);
   CONVERT_ARG_HANDLE_CHECKED(JSReceiver, key, 1);
-  return ObjectHashTable::cast(weakmap->table())->Lookup(*key);
+  Handle<ObjectHashTable> table(ObjectHashTable::cast(weakmap->table()));
+  Handle<Object> lookup(table->Lookup(*key));
+  return isolate->heap()->ToBoolean(!lookup->IsTheHole());
+}
+
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_WeakMapDelete) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 2);
+  CONVERT_ARG_HANDLE_CHECKED(JSWeakMap, weakmap, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, key, 1);
+  Handle<ObjectHashTable> table(ObjectHashTable::cast(weakmap->table()));
+  Handle<Object> lookup(table->Lookup(*key));
+  Handle<ObjectHashTable> new_table =
+      PutIntoObjectHashTable(table, key, isolate->factory()->the_hole_value());
+  weakmap->set_table(*new_table);
+  return isolate->heap()->ToBoolean(!lookup->IsTheHole());
 }
 
 

src/runtime.h

@@ -302,11 +302,15 @@ namespace internal {
   /* Harmony maps */ \
   F(MapInitialize, 1, 1) \
   F(MapGet, 2, 1) \
+  F(MapHas, 2, 1) \
+  F(MapDelete, 2, 1) \
   F(MapSet, 3, 1) \
   \
   /* Harmony weakmaps */ \
   F(WeakMapInitialize, 1, 1) \
   F(WeakMapGet, 2, 1) \
+  F(WeakMapHas, 2, 1) \
+  F(WeakMapDelete, 2, 1) \
   F(WeakMapSet, 3, 1) \
   \
   /* Statements */ \

test/cctest/test-dictionary.cc

@@ -48,24 +48,24 @@ TEST(ObjectHashTable) {
   table = PutIntoObjectHashTable(table, a, b);
   CHECK_EQ(table->NumberOfElements(), 1);
   CHECK_EQ(table->Lookup(*a), *b);
-  CHECK_EQ(table->Lookup(*b), HEAP->undefined_value());
+  CHECK_EQ(table->Lookup(*b), HEAP->the_hole_value());
 
   // Keys still have to be valid after objects were moved.
   HEAP->CollectGarbage(NEW_SPACE);
   CHECK_EQ(table->NumberOfElements(), 1);
   CHECK_EQ(table->Lookup(*a), *b);
-  CHECK_EQ(table->Lookup(*b), HEAP->undefined_value());
+  CHECK_EQ(table->Lookup(*b), HEAP->the_hole_value());
 
   // Keys that are overwritten should not change number of elements.
   table = PutIntoObjectHashTable(table, a, FACTORY->NewJSArray(13));
   CHECK_EQ(table->NumberOfElements(), 1);
   CHECK_NE(table->Lookup(*a), *b);
 
-  // Keys mapped to undefined should be removed permanently.
-  table = PutIntoObjectHashTable(table, a, FACTORY->undefined_value());
+  // Keys mapped to the hole should be removed permanently.
+  table = PutIntoObjectHashTable(table, a, FACTORY->the_hole_value());
   CHECK_EQ(table->NumberOfElements(), 0);
   CHECK_EQ(table->NumberOfDeletedElements(), 1);
-  CHECK_EQ(table->Lookup(*a), HEAP->undefined_value());
+  CHECK_EQ(table->Lookup(*a), HEAP->the_hole_value());
 
   // Keys should map back to their respective values and also should get
   // an identity hash code generated.
@@ -85,7 +85,7 @@ TEST(ObjectHashTable) {
     Handle<JSObject> key = FACTORY->NewJSArray(7);
     CHECK(key->GetIdentityHash(ALLOW_CREATION)->ToObjectChecked()->IsSmi());
     CHECK_EQ(table->FindEntry(*key), ObjectHashTable::kNotFound);
-    CHECK_EQ(table->Lookup(*key), HEAP->undefined_value());
+    CHECK_EQ(table->Lookup(*key), HEAP->the_hole_value());
     CHECK(key->GetIdentityHash(OMIT_CREATION)->ToObjectChecked()->IsSmi());
   }
 
@@ -93,7 +93,7 @@ TEST(ObjectHashTable) {
   // should not get an identity hash code generated.
   for (int i = 0; i < 100; i++) {
     Handle<JSObject> key = FACTORY->NewJSArray(7);
-    CHECK_EQ(table->Lookup(*key), HEAP->undefined_value());
+    CHECK_EQ(table->Lookup(*key), HEAP->the_hole_value());
     CHECK_EQ(key->GetIdentityHash(OMIT_CREATION), HEAP->undefined_value());
   }
 }

test/mjsunit/harmony/collections.js

-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
@@ -119,12 +119,12 @@ TestMapBehavior2(new Map);
 // Test expected querying behavior of Maps and WeakMaps
 function TestQuery(m) {
   var key = new Object;
-  TestMapping(m, key, 'to-be-present');
-  assertTrue(m.has(key));
-  assertFalse(m.has(new Object));
-  TestMapping(m, key, undefined);
-  assertFalse(m.has(key));
-  assertFalse(m.has(new Object));
+  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);
 TestQuery(new WeakMap);