Fix for-in for large indexes and indexes on proxies

BUG=v8:4130
LOG=n

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

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

src/elements.cc

@@ -130,11 +130,7 @@ static bool HasKey(Handle<FixedArray> array, Handle<Object> key_handle) {
   int len0 = array->length();
   for (int i = 0; i < len0; i++) {
     Object* element = array->get(i);
-    if (element->IsSmi() && element == key) return true;
-    if (element->IsString() &&
-        key->IsString() && String::cast(element)->Equals(String::cast(key))) {
-      return true;
-    }
+    if (key->KeyEquals(element)) return true;
   }
   return false;
 }
@@ -738,8 +734,10 @@ class ElementsAccessorBase : public ElementsAccessor {
   }
 
   virtual MaybeHandle<FixedArray> AddElementsToFixedArray(
-      Handle<Object> receiver, Handle<JSObject> holder, Handle<FixedArray> to,
-      Handle<FixedArrayBase> from, FixedArray::KeyFilter filter) final {
+      Handle<JSObject> receiver, Handle<FixedArray> to,
+      FixedArray::KeyFilter filter) final {
+    Handle<FixedArrayBase> from(receiver->elements());
+
     int len0 = to->length();
 #ifdef ENABLE_SLOW_DCHECKS
     if (FLAG_enable_slow_asserts) {
@@ -751,7 +749,7 @@ class ElementsAccessorBase : public ElementsAccessor {
 
     // Optimize if 'other' is empty.
     // We cannot optimize if 'this' is empty, as other may have holes.
-    uint32_t len1 = ElementsAccessorSubclass::GetCapacityImpl(holder, from);
+    uint32_t len1 = ElementsAccessorSubclass::GetCapacityImpl(receiver, from);
     if (len1 == 0) return to;
 
     Isolate* isolate = from->GetIsolate();
@@ -760,11 +758,11 @@ class ElementsAccessorBase : public ElementsAccessor {
     uint32_t extra = 0;
     for (uint32_t y = 0; y < len1; y++) {
       uint32_t key = ElementsAccessorSubclass::GetKeyForIndexImpl(from, y);
-      if (ElementsAccessorSubclass::HasElementImpl(holder, key, from)) {
+      if (ElementsAccessorSubclass::HasElementImpl(receiver, key, from)) {
         Handle<Object> value;
         ASSIGN_RETURN_ON_EXCEPTION(
             isolate, value,
-            ElementsAccessorSubclass::GetImpl(receiver, holder, key, from),
+            ElementsAccessorSubclass::GetImpl(receiver, receiver, key, from),
             FixedArray);
 
         DCHECK(!value->IsTheHole());
@@ -797,11 +795,11 @@ class ElementsAccessorBase : public ElementsAccessor {
     for (uint32_t y = 0; y < len1; y++) {
       uint32_t key =
           ElementsAccessorSubclass::GetKeyForIndexImpl(from, y);
-      if (ElementsAccessorSubclass::HasElementImpl(holder, key, from)) {
+      if (ElementsAccessorSubclass::HasElementImpl(receiver, key, from)) {
         Handle<Object> value;
         ASSIGN_RETURN_ON_EXCEPTION(
             isolate, value,
-            ElementsAccessorSubclass::GetImpl(receiver, holder, key, from),
+            ElementsAccessorSubclass::GetImpl(receiver, receiver, key, from),
             FixedArray);
         if (filter == FixedArray::NON_SYMBOL_KEYS && value->IsSymbol()) {
           continue;

src/elements.h

@@ -159,15 +159,8 @@ class ElementsAccessor {
   }
 
   MUST_USE_RESULT virtual MaybeHandle<FixedArray> AddElementsToFixedArray(
-      Handle<Object> receiver, Handle<JSObject> holder, Handle<FixedArray> to,
-      Handle<FixedArrayBase> from, FixedArray::KeyFilter filter) = 0;
-
-  MUST_USE_RESULT inline MaybeHandle<FixedArray> AddElementsToFixedArray(
-      Handle<Object> receiver, Handle<JSObject> holder, Handle<FixedArray> to,
-      FixedArray::KeyFilter filter) {
-    return AddElementsToFixedArray(receiver, holder, to,
-                                   handle(holder->elements()), filter);
-  }
+      Handle<JSObject> receiver, Handle<FixedArray> to,
+      FixedArray::KeyFilter filter) = 0;
 
   // Returns a shared ElementsAccessor for the specified ElementsKind.
   static ElementsAccessor* ForKind(ElementsKind elements_kind) {

src/objects-inl.h

@@ -275,6 +275,24 @@ bool Object::HasValidElements() {
 }
 
 
+bool Object::KeyEquals(Object* second) {
+  Object* first = this;
+  if (second->IsNumber()) {
+    if (first->IsNumber()) return first->Number() == second->Number();
+    Object* temp = first;
+    first = second;
+    second = temp;
+  }
+  if (first->IsNumber()) {
+    DCHECK_LE(0, first->Number());
+    uint32_t expected = static_cast<uint32_t>(first->Number());
+    uint32_t index;
+    return Name::cast(second)->AsArrayIndex(&index) && index == expected;
+  }
+  return Name::cast(first)->Equals(Name::cast(second));
+}
+
+
 Handle<Object> Object::NewStorageFor(Isolate* isolate,
                                      Handle<Object> object,
                                      Representation representation) {

src/objects.cc

@@ -8203,8 +8203,7 @@ MaybeHandle<FixedArray> FixedArray::AddKeysFromArrayLike(
   Handle<FixedArray> result;
   ASSIGN_RETURN_ON_EXCEPTION(
       array->GetIsolate(), result,
-      accessor->AddElementsToFixedArray(array, array, content, filter),
-      FixedArray);
+      accessor->AddElementsToFixedArray(array, content, filter), FixedArray);
 
 #ifdef ENABLE_SLOW_DCHECKS
   if (FLAG_enable_slow_asserts) {
@@ -8221,25 +8220,27 @@ MaybeHandle<FixedArray> FixedArray::AddKeysFromArrayLike(
 
 MaybeHandle<FixedArray> FixedArray::UnionOfKeys(Handle<FixedArray> first,
                                                 Handle<FixedArray> second) {
-  ElementsAccessor* accessor = ElementsAccessor::ForArray(second);
-  Handle<FixedArray> result;
-  ASSIGN_RETURN_ON_EXCEPTION(
-      first->GetIsolate(), result,
-      accessor->AddElementsToFixedArray(
-          Handle<Object>::null(),    // receiver
-          Handle<JSObject>::null(),  // holder
-          first, Handle<FixedArrayBase>::cast(second), ALL_KEYS),
-      FixedArray);
-
-#ifdef ENABLE_SLOW_DCHECKS
-  if (FLAG_enable_slow_asserts) {
-    DisallowHeapAllocation no_allocation;
-    for (int i = 0; i < result->length(); i++) {
-      Object* current = result->get(i);
-      DCHECK(current->IsNumber() || current->IsName());
+  if (second->length() == 0) return first;
+  if (first->length() == 0) return second;
+  Isolate* isolate = first->GetIsolate();
+  Handle<FixedArray> result =
+      isolate->factory()->NewFixedArray(first->length() + second->length());
+  for (int i = 0; i < first->length(); i++) {
+    result->set(i, first->get(i));
+  }
+  int pos = first->length();
+  for (int j = 0; j < second->length(); j++) {
+    Object* current = second->get(j);
+    int i;
+    for (i = 0; i < first->length(); i++) {
+      if (current->KeyEquals(first->get(i))) break;
+    }
+    if (i == first->length()) {
+      result->set(pos++, current);
     }
   }
-#endif
+
+  result->Shrink(pos);
   return result;
 }
 

src/objects.h

@@ -1107,6 +1107,11 @@ class Object {
     return true;
   }
 
+  // Checks whether two valid primitive encodings of a property name resolve to
+  // the same logical property. E.g., the smi 1, the string "1" and the double
+  // 1 all refer to the same property, so this helper will return true.
+  inline bool KeyEquals(Object* other);
+
   Handle<HeapType> OptimalType(Isolate* isolate, Representation representation);
 
   inline static Handle<Object> NewStorageFor(Isolate* isolate,

test/mjsunit/for-in.js

@@ -119,3 +119,15 @@ for (i=0 ; i < 3; ++i) {
   assertEquals("undefined", typeof y[2], "y[2]");
   assertEquals("undefined", typeof y[0], "y[0]");
 }
+
+(function() {
+  var large_key = 2147483650;
+  var o = {__proto__: {}};
+  o[large_key] = 1;
+  o.__proto__[large_key] = 1;
+  var keys = [];
+  for (var k in o) {
+    keys.push(k);
+  }
+  assertEquals(["2147483650"], keys);
+})();

test/mjsunit/harmony/proxies-for.js

@@ -165,4 +165,13 @@ TestForInThrow(Proxy.create({
   get: function(pr, pk) {
     return function() { throw "myexn" }
   }
-}))
+}));
+
+(function() {
+  var p = Proxy.create({enumerate:function() { return [0]; }});
+  var o = [0];
+  o.__proto__ = p;
+  var keys = [];
+  for (var k in o) { keys.push(k); };
+  assertEquals(["0"], keys);
+})();