Use the LookupIterator to transition to elements accessors

BUG=v8:4137
LOG=n

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

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

src/lookup.cc

@@ -85,7 +85,7 @@ Handle<JSReceiver> LookupIterator::GetRoot(Isolate* isolate,
 
 
 Handle<Map> LookupIterator::GetReceiverMap() const {
-  if (receiver_->IsNumber()) return isolate_->factory()->heap_number_map();
+  if (receiver_->IsNumber()) return factory()->heap_number_map();
   return handle(Handle<HeapObject>::cast(receiver_)->map(), isolate_);
 }
 
@@ -278,17 +278,18 @@ void LookupIterator::TransitionToAccessorProperty(
   // observable.
   Handle<JSObject> receiver = GetStoreTarget();
   holder_ = receiver;
-  holder_map_ =
-      Map::TransitionToAccessorProperty(handle(receiver->map(), isolate_),
-                                        name_, component, accessor, attributes);
-  JSObject::MigrateToMap(receiver, holder_map_);
 
-  ReloadPropertyInformation();
+  if (!IsElement() && !receiver->map()->is_dictionary_map()) {
+    holder_map_ = Map::TransitionToAccessorProperty(
+        handle(receiver->map(), isolate_), name_, component, accessor,
+        attributes);
+    JSObject::MigrateToMap(receiver, holder_map_);
 
-  if (!holder_map_->is_dictionary_map()) return;
+    ReloadPropertyInformation();
 
+    if (!holder_map_->is_dictionary_map()) return;
+  }
 
-  // Install the accessor into the dictionary-mode object.
   PropertyDetails details(attributes, ACCESSOR_CONSTANT, 0,
                           PropertyCellType::kMutable);
   Handle<AccessorPair> pair;
@@ -302,12 +303,38 @@ void LookupIterator::TransitionToAccessorProperty(
       pair->set(component, *accessor);
     }
   } else {
-    pair = isolate()->factory()->NewAccessorPair();
+    pair = factory()->NewAccessorPair();
     pair->set(component, *accessor);
   }
-  JSObject::SetNormalizedProperty(receiver, name_, pair, details);
 
-  JSObject::ReoptimizeIfPrototype(receiver);
+  if (IsElement()) {
+    // TODO(verwaest): Remove this hack once we have a quick way to check the
+    // prototype chain in element setters.
+    // TODO(verwaest): Move code into the element accessor.
+    bool was_dictionary = receiver->HasDictionaryElements();
+    Handle<SeededNumberDictionary> dictionary =
+        JSObject::NormalizeElements(receiver);
+    was_dictionary = was_dictionary && dictionary->requires_slow_elements();
+
+    dictionary = SeededNumberDictionary::Set(dictionary, index_, pair, details);
+    dictionary->set_requires_slow_elements();
+
+    if (receiver->HasSlowArgumentsElements()) {
+      FixedArray* parameter_map = FixedArray::cast(receiver->elements());
+      uint32_t length = parameter_map->length() - 2;
+      if (number_ < length) {
+        parameter_map->set(number_ + 2, heap()->the_hole_value());
+      }
+      FixedArray::cast(receiver->elements())->set(1, *dictionary);
+    } else {
+      receiver->set_elements(*dictionary);
+      if (!was_dictionary) heap()->ClearAllICsByKind(Code::KEYED_STORE_IC);
+    }
+  } else {
+    JSObject::SetNormalizedProperty(receiver, name_, pair, details);
+    JSObject::ReoptimizeIfPrototype(receiver);
+  }
+
   holder_map_ = handle(receiver->map(), isolate_);
   ReloadPropertyInformation();
 }

src/lookup.h

@@ -168,7 +168,7 @@ class LookupIterator final BASE_EMBEDDED {
   Handle<Name> GetName() {
     if (name_.is_null()) {
       DCHECK(IsElement());
-      name_ = isolate_->factory()->Uint32ToString(index_);
+      name_ = factory()->Uint32ToString(index_);
     }
     return name_;
   }
@@ -183,6 +183,7 @@ class LookupIterator final BASE_EMBEDDED {
     state_ = NOT_FOUND;
   }
 
+  Heap* heap() const { return isolate_->heap(); }
   Factory* factory() const { return isolate_->factory(); }
   Handle<Object> GetReceiver() const { return receiver_; }
   Handle<JSObject> GetStoreTarget() const;

src/objects.cc

@@ -6230,100 +6230,6 @@ MaybeHandle<FixedArray> JSReceiver::GetKeys(Handle<JSReceiver> object,
 }
 
 
-// Try to update an accessor in an elements dictionary. Return true if the
-// update succeeded, and false otherwise.
-static bool UpdateGetterSetterInDictionary(
-    SeededNumberDictionary* dictionary,
-    uint32_t index,
-    Object* getter,
-    Object* setter,
-    PropertyAttributes attributes) {
-  int entry = dictionary->FindEntry(index);
-  if (entry != SeededNumberDictionary::kNotFound) {
-    Object* result = dictionary->ValueAt(entry);
-    PropertyDetails details = dictionary->DetailsAt(entry);
-    if (details.type() == ACCESSOR_CONSTANT && result->IsAccessorPair()) {
-      DCHECK(details.IsConfigurable());
-      if (details.attributes() != attributes) {
-        dictionary->DetailsAtPut(
-            entry, PropertyDetails(attributes, ACCESSOR_CONSTANT, index,
-                                   PropertyCellType::kNoCell));
-      }
-      AccessorPair::cast(result)->SetComponents(getter, setter);
-      return true;
-    }
-  }
-  return false;
-}
-
-
-void JSObject::DefineElementAccessor(Handle<JSObject> object,
-                                     uint32_t index,
-                                     Handle<Object> getter,
-                                     Handle<Object> setter,
-                                     PropertyAttributes attributes) {
-  switch (object->GetElementsKind()) {
-    case FAST_SMI_ELEMENTS:
-    case FAST_ELEMENTS:
-    case FAST_DOUBLE_ELEMENTS:
-    case FAST_HOLEY_SMI_ELEMENTS:
-    case FAST_HOLEY_ELEMENTS:
-    case FAST_HOLEY_DOUBLE_ELEMENTS:
-      break;
-
-#define TYPED_ARRAY_CASE(Type, type, TYPE, ctype, size)                        \
-    case EXTERNAL_##TYPE##_ELEMENTS:                                           \
-    case TYPE##_ELEMENTS:                                                      \
-
-    TYPED_ARRAYS(TYPED_ARRAY_CASE)
-#undef TYPED_ARRAY_CASE
-      // Ignore getters and setters on pixel and external array elements.
-      return;
-
-    case DICTIONARY_ELEMENTS:
-      if (UpdateGetterSetterInDictionary(object->element_dictionary(),
-                                         index,
-                                         *getter,
-                                         *setter,
-                                         attributes)) {
-        return;
-      }
-      break;
-    case FAST_SLOPPY_ARGUMENTS_ELEMENTS:
-    case SLOW_SLOPPY_ARGUMENTS_ELEMENTS: {
-      // Ascertain whether we have read-only properties or an existing
-      // getter/setter pair in an arguments elements dictionary backing
-      // store.
-      FixedArray* parameter_map = FixedArray::cast(object->elements());
-      uint32_t length = parameter_map->length();
-      Object* probe =
-          index < (length - 2) ? parameter_map->get(index + 2) : NULL;
-      if (probe == NULL || probe->IsTheHole()) {
-        FixedArray* arguments = FixedArray::cast(parameter_map->get(1));
-        if (arguments->IsDictionary()) {
-          SeededNumberDictionary* dictionary =
-              SeededNumberDictionary::cast(arguments);
-          if (UpdateGetterSetterInDictionary(dictionary,
-                                             index,
-                                             *getter,
-                                             *setter,
-                                             attributes)) {
-            return;
-          }
-        }
-      }
-      break;
-    }
-  }
-
-  Isolate* isolate = object->GetIsolate();
-  Handle<AccessorPair> accessors = isolate->factory()->NewAccessorPair();
-  accessors->SetComponents(*getter, *setter);
-
-  SetElementCallback(object, index, accessors, attributes);
-}
-
-
 bool Map::DictionaryElementsInPrototypeChainOnly() {
   if (IsDictionaryElementsKind(elements_kind())) {
     return false;
@@ -6431,6 +6337,12 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
     it.Next();
   }
 
+  // Ignore accessors on typed arrays.
+  if (it.IsElement() && (object->HasFixedTypedArrayElements() ||
+                         object->HasExternalArrayElements())) {
+    return it.factory()->undefined_value();
+  }
+
   Handle<Object> old_value = isolate->factory()->the_hole_value();
   bool is_observed = object->map()->is_observed() &&
                      !isolate->IsInternallyUsedPropertyName(name);
@@ -6444,22 +6356,15 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
     }
   }
 
-  if (it.IsElement()) {
-    DefineElementAccessor(it.GetStoreTarget(), it.index(), getter, setter,
-                          attributes);
-  } else {
-    DCHECK(getter->IsSpecFunction() || getter->IsUndefined() ||
-           getter->IsNull());
-    DCHECK(setter->IsSpecFunction() || setter->IsUndefined() ||
-           setter->IsNull());
-    // At least one of the accessors needs to be a new value.
-    DCHECK(!getter->IsNull() || !setter->IsNull());
-    if (!getter->IsNull()) {
-      it.TransitionToAccessorProperty(ACCESSOR_GETTER, getter, attributes);
-    }
-    if (!setter->IsNull()) {
-      it.TransitionToAccessorProperty(ACCESSOR_SETTER, setter, attributes);
-    }
+  DCHECK(getter->IsSpecFunction() || getter->IsUndefined() || getter->IsNull());
+  DCHECK(setter->IsSpecFunction() || setter->IsUndefined() || setter->IsNull());
+  // At least one of the accessors needs to be a new value.
+  DCHECK(!getter->IsNull() || !setter->IsNull());
+  if (!getter->IsNull()) {
+    it.TransitionToAccessorProperty(ACCESSOR_GETTER, getter, attributes);
+  }
+  if (!setter->IsNull()) {
+    it.TransitionToAccessorProperty(ACCESSOR_SETTER, setter, attributes);
   }
 
   if (is_observed) {

src/objects.h

@@ -2319,11 +2319,6 @@ class JSObject: public JSReceiver {
                                   Handle<Name> name,
                                   Handle<Object> structure,
                                   PropertyAttributes attributes);
-  static void DefineElementAccessor(Handle<JSObject> object,
-                                    uint32_t index,
-                                    Handle<Object> getter,
-                                    Handle<Object> setter,
-                                    PropertyAttributes attributes);
 
   // Return the hash table backing store or the inline stored identity hash,
   // whatever is found.

test/mjsunit/element-accessor.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(function () {
+  var o = [];
+  o.__proto__ = {};
+
+  function store(o, i, v) {
+    o[i] = v;
+  }
+
+  store(o, 0, 0);
+  store(o, 1, 0);
+  store(o, 2, 0);
+  o.__proto__[10000000] = 1;
+
+  var set = 0;
+
+  Object.defineProperty(o, "3", {
+    get:function() { return 100; },
+    set:function(v) { set = v; }});
+
+  store(o, 3, 1000);
+  assertEquals(1000, set);
+  assertEquals(100, o[3]);
+})();
+
+(function () {
+  var o = new Int32Array();
+  Object.defineProperty(o, "0", {get: function(){}});
+  assertEquals(undefined, Object.getOwnPropertyDescriptor(o, "0"));
+})();