[es7] refactor and fix Object.values() / Object.entries()

src/key-accumulator.cc

Index: src/key-accumulator.cc
diff --git a/src/key-accumulator.cc b/src/key-accumulator.cc
index e7a9c3ccebccd3ba56b7a87b6cc7c1f9148deae7..0f46a69ad2c6a47931925ba80453d422e695bfe8 100644
--- a/src/key-accumulator.cc
+++ b/src/key-accumulator.cc
@@ -21,8 +21,9 @@ KeyAccumulator::~KeyAccumulator() {
   }
 }
 
-
-Handle<FixedArray> KeyAccumulator::GetKeys(GetKeysConversion convert) {
+template <typename F>
+MaybeHandle<FixedArray> KeyAccumulator::GetKeys_Internal(
+    GetKeysConversion convert, bool trivial, F& accumulate) {
   if (length_ == 0) {
     return isolate_->factory()->empty_fixed_array();
   }
@@ -59,30 +60,96 @@ Handle<FixedArray> KeyAccumulator::GetKeys(GetKeysConversion convert) {
         } else {
           key = isolate_->factory()->Uint32ToString(elements->at(i));
         }
-        result->set(insertion_index, *key);
-        insertion_index++;
+        auto did_accumulate = accumulate(result, insertion_index, *key);

[Camillo Bruni, 2016/02/03 12:03:48]

auto => Maybe<bool>

+        MAYBE_RETURN(did_accumulate, Handle<FixedArray>());
+        if (did_accumulate.FromJust()) insertion_index++;
       }
     }
     // Add the string property keys for this prototype level.
     for (int i = 0; i < num_string_properties; i++) {
       Object* key = string_properties_->KeyAt(string_properties_index);
-      result->set(insertion_index, key);
-      insertion_index++;
+      auto did_accumulate = accumulate(result, insertion_index, key);
+      MAYBE_RETURN(did_accumulate, Handle<FixedArray>());
+      if (did_accumulate.FromJust()) insertion_index++;
       string_properties_index++;
     }
     // Add the symbol property keys for this prototype level.
     for (int i = 0; i < num_symbol_properties; i++) {
       Object* key = symbol_properties_->KeyAt(symbol_properties_index);
-      result->set(insertion_index, key);
-      insertion_index++;
+      auto did_accumulate = accumulate(result, insertion_index, key);
+      MAYBE_RETURN(did_accumulate, Handle<FixedArray>());
+      if (did_accumulate.FromJust()) insertion_index++;
       symbol_properties_index++;
     }
   }
 
-  DCHECK_EQ(insertion_index, length_);
+  if (trivial) {
+    DCHECK_EQ(insertion_index, length_);
+  } else if (insertion_index < length_) {
+    result->Shrink(insertion_index);
+  }
   return result;
 }
 
+Handle<FixedArray> KeyAccumulator::GetKeys(GetKeysConversion convert) {
+  auto add_key = [](Handle<FixedArray> result, int i, Object* key) {
+    result->set(i, key);
+    return Just(true);
+  };
+  return GetKeys_Internal(convert, true, add_key).ToHandleChecked();
+}
+
+MaybeHandle<FixedArray> KeyAccumulator::GetValues(Handle<JSReceiver> receiver,
+                                                  GetKeysConversion convert) {
+  Isolate* isolate = isolate_;
+  auto add_value = [receiver, isolate](Handle<FixedArray> result, int i,

[Camillo Bruni, 2016/02/03 12:03:48]

I'm fine with the "auto" type for the accumulate lambda (or whatever these
things are called in C++) but I would refrain from using them in all the other
places.

[caitp (gmail), 2016/02/03 12:57:30]

replaced all non-lambda's with proper types

+                                       Object* key) {
+    Handle<Object> value;
+    Handle<Name> name = Handle<Name>::cast(handle(key, isolate));
+
+    PropertyDescriptor descriptor;
+    auto owned = JSReceiver::GetOwnPropertyDescriptor(isolate, receiver, name,
+                                                      &descriptor);
+    MAYBE_RETURN(owned, Nothing<bool>());
+    if (!owned.FromJust() || !descriptor.enumerable()) return Just(false);
+
+    if (Object::GetPropertyOrElement(receiver, name, STRICT).ToHandle(&value)) {
+      result->set(i, *value);
+      return Just(true);
+    }
+    return Nothing<bool>();
+  };
+
+  return GetKeys_Internal(convert, false, add_value);
+}
+
+MaybeHandle<FixedArray> KeyAccumulator::GetEntries(Handle<JSReceiver> receiver,
+                                                   GetKeysConversion convert) {
+  Isolate* isolate = isolate_;
+  auto add_entry = [receiver, isolate](Handle<FixedArray> result, int i,
+                                       Object* key) {
+    Handle<Object> value;
+    Handle<Name> name = Handle<Name>::cast(handle(key, isolate));
+
+    PropertyDescriptor descriptor;
+    auto owned = JSReceiver::GetOwnPropertyDescriptor(isolate, receiver, name,
+                                                      &descriptor);
+    MAYBE_RETURN(owned, Nothing<bool>());
+    if (!owned.FromJust() || !descriptor.enumerable()) return Just(false);
+
+    if (Object::GetPropertyOrElement(receiver, name, STRICT).ToHandle(&value)) {
+      auto entry_storage = isolate->factory()->NewUninitializedFixedArray(2);
+      entry_storage->set(0, key);
+      entry_storage->set(1, *value);
+      auto entry = isolate->factory()->NewJSArrayWithElements(entry_storage);
+      result->set(i, *entry);
+      return Just(true);
+    }
+    return Nothing<bool>();
+  };
+
+  return GetKeys_Internal(convert, false, add_entry);
+}
 
 namespace {