From 362381c68d6a61924d4b23311dcd2dae1d1da0a1 Mon Sep 17 00:00:00 2001 From: "mstarzinger@chromium.org" Date: Mon, 25 Jun 2012 13:33:48 +0000 Subject: [PATCH] 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 | 20 +++-------- src/objects.cc | 6 ++-- src/objects.h | 4 +-- src/runtime.cc | 66 +++++++++++++++++++++++++++++++++---- src/runtime.h | 4 +++ test/cctest/test-dictionary.cc | 14 ++++---- test/mjsunit/harmony/collections.js | 14 ++++---- 7 files changed, 88 insertions(+), 40 deletions(-) diff --git a/src/collection.js b/src/collection.js index 9ca0aae..d36fe18 100644 --- a/src/collection.js +++ b/src/collection.js @@ -1,4 +1,4 @@ -// 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); } // ------------------------------------------------------------------- diff --git a/src/objects.cc b/src/objects.cc index 055bbc4..5ec05aa 100644 --- a/src/objects.cc +++ b/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); diff --git a/src/objects.h b/src/objects.h index e0f99ba..90564f3 100644 --- a/src/objects.h +++ b/src/objects.h @@ -3325,12 +3325,12 @@ class ObjectHashTable: public HashTable, Object*> { return reinterpret_cast(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: diff --git a/src/runtime.cc b/src/runtime.cc index 8c7a340..74f3e43 100644 --- a/src/runtime.cc +++ b/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 key(args[1]); - return ObjectHashTable::cast(holder->table())->Lookup(*key); + CONVERT_ARG_HANDLE_CHECKED(Object, key, 1); + Handle table(ObjectHashTable::cast(holder->table())); + Handle 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 table(ObjectHashTable::cast(holder->table())); + Handle 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 table(ObjectHashTable::cast(holder->table())); + Handle lookup(table->Lookup(*key)); + Handle 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 key(args[1]); - Handle value(args[2]); + CONVERT_ARG_HANDLE_CHECKED(Object, key, 1); + CONVERT_ARG_HANDLE_CHECKED(Object, value, 2); Handle table(ObjectHashTable::cast(holder->table())); Handle 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 table(ObjectHashTable::cast(weakmap->table())); + Handle 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 table(ObjectHashTable::cast(weakmap->table())); + Handle 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 table(ObjectHashTable::cast(weakmap->table())); + Handle lookup(table->Lookup(*key)); + Handle new_table = + PutIntoObjectHashTable(table, key, isolate->factory()->the_hole_value()); + weakmap->set_table(*new_table); + return isolate->heap()->ToBoolean(!lookup->IsTheHole()); } diff --git a/src/runtime.h b/src/runtime.h index 9968b29..3430a58 100644 --- a/src/runtime.h +++ b/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 */ \ diff --git a/test/cctest/test-dictionary.cc b/test/cctest/test-dictionary.cc index 793e228..c704363 100644 --- a/test/cctest/test-dictionary.cc +++ b/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 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 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()); } } diff --git a/test/mjsunit/harmony/collections.js b/test/mjsunit/harmony/collections.js index 95504bc..f3db7ea 100644 --- a/test/mjsunit/harmony/collections.js +++ b/test/mjsunit/harmony/collections.js @@ -1,4 +1,4 @@ -// 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); -- 2.7.4