Teach OrderedHashSet::Remove to report whether it actually removed anything

This avoids an unnecessary runtime call from Set.prototype.delete().

R=mstarzinger@chromium.org

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

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

src/collection.js

@@ -47,12 +47,7 @@ function SetDeleteJS(key) {
     throw MakeTypeError('incompatible_method_receiver',
                         ['Set.prototype.delete', this]);
   }
-  if (%SetHas(this, key)) {
-    %SetDelete(this, key);
-    return true;
-  } else {
-    return false;
-  }
+  return %SetDelete(this, key);
 }
 
 

src/objects.cc

@@ -16456,9 +16456,14 @@ Handle<OrderedHashSet> OrderedHashSet::Add(Handle<OrderedHashSet> table,
 
 
 Handle<OrderedHashSet> OrderedHashSet::Remove(Handle<OrderedHashSet> table,
-                                              Handle<Object> key) {
+                                              Handle<Object> key,
+                                              bool* was_present) {
   int entry = table->FindEntry(key);
-  if (entry == kNotFound) return table;
+  if (entry == kNotFound) {
+    *was_present = false;
+    return table;
+  }
+  *was_present = true;
   table->RemoveEntry(entry);
   return Shrink(table);
 }

src/objects.h

@@ -4323,7 +4323,7 @@ class OrderedHashSet: public OrderedHashTable<
   static Handle<OrderedHashSet> Add(
       Handle<OrderedHashSet> table, Handle<Object> key);
   static Handle<OrderedHashSet> Remove(
-      Handle<OrderedHashSet> table, Handle<Object> key);
+      Handle<OrderedHashSet> table, Handle<Object> key, bool* was_present);
 };
 
 

src/runtime.cc

@@ -1554,9 +1554,10 @@ RUNTIME_FUNCTION(Runtime_SetDelete) {
   CONVERT_ARG_HANDLE_CHECKED(JSSet, holder, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, key, 1);
   Handle<OrderedHashSet> table(OrderedHashSet::cast(holder->table()));
-  table = OrderedHashSet::Remove(table, key);
+  bool was_present = false;
+  table = OrderedHashSet::Remove(table, key, &was_present);
   holder->set_table(*table);
-  return isolate->heap()->undefined_value();
+  return isolate->heap()->ToBoolean(was_present);
 }
 
 

test/cctest/test-ordered-hash-table.cc

@@ -75,10 +75,16 @@ TEST(Set) {
   ordered_set = OrderedHashSet::Add(ordered_set, obj);
   CHECK_EQ(1, ordered_set->NumberOfElements());
   CHECK(ordered_set->Contains(obj));
-  ordered_set = OrderedHashSet::Remove(ordered_set, obj);
+  bool was_present = false;
+  ordered_set = OrderedHashSet::Remove(ordered_set, obj, &was_present);
+  CHECK(was_present);
   CHECK_EQ(0, ordered_set->NumberOfElements());
   CHECK(!ordered_set->Contains(obj));
 
+  // Removing a not-present object should set was_present to false.
+  ordered_set = OrderedHashSet::Remove(ordered_set, obj, &was_present);
+  CHECK(!was_present);
+
   // Test for collisions/chaining
   Handle<JSObject> obj1 = factory->NewJSObjectFromMap(map);
   ordered_set = OrderedHashSet::Add(ordered_set, obj1);
@@ -133,10 +139,14 @@ TEST(Set) {
                         true);
 
   // Test shrinking
-  ordered_set = OrderedHashSet::Remove(ordered_set, obj);
-  ordered_set = OrderedHashSet::Remove(ordered_set, obj1);
-  ordered_set = OrderedHashSet::Remove(ordered_set, obj2);
-  ordered_set = OrderedHashSet::Remove(ordered_set, obj3);
+  ordered_set = OrderedHashSet::Remove(ordered_set, obj, &was_present);
+  CHECK(was_present);
+  ordered_set = OrderedHashSet::Remove(ordered_set, obj1, &was_present);
+  CHECK(was_present);
+  ordered_set = OrderedHashSet::Remove(ordered_set, obj2, &was_present);
+  CHECK(was_present);
+  ordered_set = OrderedHashSet::Remove(ordered_set, obj3, &was_present);
+  CHECK(was_present);
   CHECK_EQ(1, ordered_set->NumberOfElements());
   CHECK_EQ(2, ordered_set->NumberOfBuckets());
 }