More cleanly separate adding from setting elements

src/objects.cc

 // Copyright 2013 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.
 
 #include <iomanip>
 #include <sstream>
 
 #include "src/v8.h"
 
 #include "src/accessors.h"
@@ skipping 11772 matching lines @@
     if (IsHoleyElementsKind(elements_kind)) {
       new_elements_kind = FAST_HOLEY_ELEMENTS;
     } else {
       new_elements_kind = FAST_ELEMENTS;
     }
   }
   Handle<FixedArrayBase> old_elements(object->elements());
   ElementsAccessor* accessor = ElementsAccessor::ForKind(new_elements_kind);
   accessor->CopyElements(object, new_elements, elements_kind);
 
-  if (elements_kind != SLOPPY_ARGUMENTS_ELEMENTS) {
+  if (elements_kind == SLOPPY_ARGUMENTS_ELEMENTS) {
+    Handle<FixedArray> parameter_map = Handle<FixedArray>::cast(old_elements);
+    parameter_map->set(1, *new_elements);
+  } else {
     Handle<Map> new_map = (new_elements_kind != elements_kind)
         ? GetElementsTransitionMap(object, new_elements_kind)
         : handle(object->map());
     JSObject::ValidateElements(object);
     JSObject::SetMapAndElements(object, new_map, new_elements);
 
     // Transition through the allocation site as well if present.
     JSObject::UpdateAllocationSite(object, new_elements_kind);
-  } else {
-    Handle<FixedArray> parameter_map = Handle<FixedArray>::cast(old_elements);
-    parameter_map->set(1, *new_elements);
   }
 
   if (FLAG_trace_elements_transitions) {
     PrintElementsTransition(stdout, object, elements_kind, old_elements,
                             object->GetElementsKind(), new_elements);
   }
 
   return new_elements;
 }
 
@@ skipping 605 matching lines @@
   if (!elements()->IsFixedArray()) return false;
   FixedArray* elements = FixedArray::cast(this->elements());
   if (elements->map() != heap->sloppy_arguments_elements_map()) {
     return false;
   }
   FixedArray* arguments = FixedArray::cast(elements->get(1));
   return arguments->IsDictionary();
 }
 
 
-void JSObject::SetFastElement(Handle<JSObject> object, uint32_t index,
+void JSObject::AddFastElement(Handle<JSObject> object, uint32_t index,
                               Handle<Object> value) {
   DCHECK(object->HasFastSmiOrObjectElements() ||
          object->HasFastArgumentsElements());
 
   Isolate* isolate = object->GetIsolate();
 
-  // Array optimizations rely on the prototype lookups of Array objects always
-  // returning undefined. If there is a store to the initial prototype object,
-  // make sure all of these optimizations are invalidated.
-  isolate->UpdateArrayProtectorOnSetElement(object);
-
   Handle<FixedArray> backing_store(FixedArray::cast(object->elements()));
   if (object->HasSloppyArgumentsElements()) {
     backing_store = handle(FixedArray::cast(backing_store->get(1)));
   } else {
+    // Array optimizations rely on the prototype lookups of Array objects always
+    // returning undefined. If there is a store to the initial prototype object,
+    // make sure all of these optimizations are invalidated.
+    isolate->UpdateArrayProtectorOnSetElement(object);
     backing_store = EnsureWritableFastElements(object);
   }
   uint32_t capacity = static_cast<uint32_t>(backing_store->length());
 
   uint32_t new_capacity = capacity;
   // Check if the length property of this object needs to be updated.
   uint32_t array_length = 0;
   bool must_update_array_length = false;
   bool introduces_holes = true;
   if (object->IsJSArray()) {
     CHECK(
         Handle<JSArray>::cast(object)->length()->ToArrayLength(&array_length));
     introduces_holes = index > array_length;
     if (index >= array_length) {
       must_update_array_length = true;
       array_length = index + 1;
     }
   } else {
     introduces_holes = index >= capacity;
   }
 
   // If the array is growing, and it's not growth by a single element at the
   // end, make sure that the ElementsKind is HOLEY.
-  ElementsKind elements_kind = object->GetElementsKind();
-  if (introduces_holes &&
-      IsFastElementsKind(elements_kind) &&
-      !IsFastHoleyElementsKind(elements_kind)) {
-    ElementsKind transitioned_kind = GetHoleyElementsKind(elements_kind);
+  if (introduces_holes && !IsFastHoleyElementsKind(object->GetElementsKind())) {
+    ElementsKind transitioned_kind =
+        GetHoleyElementsKind(object->GetElementsKind());
     TransitionElementsKind(object, transitioned_kind);
   }
 
   // Check if the capacity of the backing store needs to be increased, or if
   // a transition to slow elements is necessary.
   if (index >= capacity) {
     bool convert_to_slow = true;
     if ((index - capacity) < kMaxGap) {
       new_capacity = NewElementsCapacity(index + 1);
       DCHECK(new_capacity > index);
       if (!object->ShouldConvertToSlowElements(new_capacity)) {
         convert_to_slow = false;
       }
     }
     if (convert_to_slow) {
       NormalizeElements(object);
-      SetDictionaryElement(object, index, value, NONE);
+      AddDictionaryElement(object, index, value, NONE);
       return;
     }
   }
+
   // Convert to fast double elements if appropriate.
   if (object->HasFastSmiElements() && !value->IsSmi() && value->IsNumber()) {
     // Consider fixing the boilerplate as well if we have one.
-    ElementsKind to_kind = IsHoleyElementsKind(elements_kind)
-        ? FAST_HOLEY_DOUBLE_ELEMENTS
-        : FAST_DOUBLE_ELEMENTS;
+    ElementsKind to_kind = IsHoleyElementsKind(object->GetElementsKind())
+                               ? FAST_HOLEY_DOUBLE_ELEMENTS
+                               : FAST_DOUBLE_ELEMENTS;
 
     UpdateAllocationSite(object, to_kind);
 
     SetFastDoubleElementsCapacityAndLength(object, new_capacity, array_length);
     FixedDoubleArray::cast(object->elements())->set(index, value->Number());
     JSObject::ValidateElements(object);
     return;
   }
+
   // Change elements kind from Smi-only to generic FAST if necessary.
   if (object->HasFastSmiElements() && !value->IsSmi()) {
     ElementsKind kind = object->HasFastHoleyElements()
         ? FAST_HOLEY_ELEMENTS
         : FAST_ELEMENTS;
 
     UpdateAllocationSite(object, kind);
     Handle<Map> new_map = GetElementsTransitionMap(object, kind);
     JSObject::MigrateToMap(object, new_map);
     DCHECK(IsFastObjectElementsKind(object->GetElementsKind()));
   }
+
   // Increase backing store capacity if that's been decided previously.
   if (new_capacity != capacity) {
     SetFastElementsCapacitySmiMode smi_mode =
         value->IsSmi() && object->HasFastSmiElements()
             ? kAllowSmiElements
             : kDontAllowSmiElements;
     Handle<FixedArray> new_elements =
         SetFastElementsCapacityAndLength(object, new_capacity, array_length,
                                          smi_mode);
     new_elements->set(index, *value);
@@ skipping 15 matching lines @@
                                          PropertyAttributes attributes) {
   // TODO(verwaest): Handle with the elements accessor.
   Isolate* isolate = object->GetIsolate();
 
   DCHECK(object->HasSloppyArgumentsElements());
 
   Handle<FixedArray> parameter_map(FixedArray::cast(object->elements()));
   uint32_t length = parameter_map->length();
   Handle<Object> probe =
       index < length - 2
-          ? Handle<Object>(parameter_map->get(index + 2), isolate)
-          : Handle<Object>();
-  if (!probe.is_null() && !probe->IsTheHole()) {
+          ? handle(parameter_map->get(index + 2), isolate)
+          : Handle<Object>::cast(isolate->factory()->the_hole_value());

[Jakob Kummerow, 2015/06/18 11:32:47]

nit: do you even need the cast?

+  if (!probe->IsTheHole()) {
     Handle<Context> context(Context::cast(parameter_map->get(0)));
     int context_index = Handle<Smi>::cast(probe)->value();
     DCHECK(!context->get(context_index)->IsTheHole());
     context->set(context_index, *value);
 
     if (attributes == NONE) return;
 
     // Redefining attributes of an aliased element destroys fast aliasing.
     parameter_map->set_the_hole(index + 2);
     // For elements that are still writable we re-establish slow aliasing.
     if ((attributes & READ_ONLY) == 0) {
-      value = Handle<Object>::cast(
-          isolate->factory()->NewAliasedArgumentsEntry(context_index));
+      value = isolate->factory()->NewAliasedArgumentsEntry(context_index);
     }
-  }
-
-  Handle<FixedArray> arguments(FixedArray::cast(parameter_map->get(1)));
-  if (arguments->IsDictionary()) {
+    DCHECK(parameter_map->get(1)->IsDictionary());

[Jakob Kummerow, 2015/06/18 11:32:47]

Since this is something that callers must ensure, maybe move the DCHECK to the
beginning of the function (near
"DCHECK(object->HasSloppyArgumentsElements());")?

+    AddDictionaryElement(object, index, value, attributes);
+  } else {
+    DCHECK(parameter_map->get(1)->IsDictionary());
     SetDictionaryElement(object, index, value, attributes);
-  } else {
-    SetFastElement(object, index, value);
   }
 }
 
+
+void JSObject::AddSloppyArgumentsElement(Handle<JSObject> object,
+                                         uint32_t index, Handle<Object> value,
+                                         PropertyAttributes attributes) {
+  DCHECK(object->HasSloppyArgumentsElements());
+
+  // TODO(verwaest): Handle with the elements accessor.
+  FixedArray* parameter_map = FixedArray::cast(object->elements());
+
+#ifdef DEBUG
+  uint32_t length = parameter_map->length();
+  if (index < length - 2) {
+    Object* probe = parameter_map->get(index + 2);
+    DCHECK(probe->IsTheHole());
+  }
+#endif
+
+  if (parameter_map->get(1)->IsDictionary()) {
+    AddDictionaryElement(object, index, value, attributes);
+  } else {
+    AddFastElement(object, index, value);
+  }
+}
+
 
 void JSObject::SetDictionaryElement(Handle<JSObject> object, uint32_t index,
                                     Handle<Object> value,
                                     PropertyAttributes attributes) {
   // TODO(verwaest): Handle with the elements accessor.
   Isolate* isolate = object->GetIsolate();
 
   // Insert element in the dictionary.
   Handle<FixedArray> elements(FixedArray::cast(object->elements()));
   bool is_arguments =
       (elements->map() == isolate->heap()->sloppy_arguments_elements_map());
 
   DCHECK(object->HasDictionaryElements() ||
          object->HasDictionaryArgumentsElements());
 
   Handle<SeededNumberDictionary> dictionary(is_arguments
     ? SeededNumberDictionary::cast(elements->get(1))
     : SeededNumberDictionary::cast(*elements));
 
   int entry = dictionary->FindEntry(index);
-  if (entry != SeededNumberDictionary::kNotFound) {
+  DCHECK_NE(SeededNumberDictionary::kNotFound, entry);
+
+  PropertyDetails details = dictionary->DetailsAt(entry);
+  details = PropertyDetails(attributes, DATA, details.dictionary_index(),
+                            PropertyCellType::kNoCell);
+  dictionary->DetailsAtPut(entry, details);
+
+  // Elements of the arguments object in slow mode might be slow aliases.
+  if (is_arguments) {
     Handle<Object> element(dictionary->ValueAt(entry), isolate);
-    PropertyDetails details = dictionary->DetailsAt(entry);
-    DCHECK(details.IsConfigurable() || !details.IsReadOnly() ||
-           element->IsTheHole());
-    dictionary->UpdateMaxNumberKey(index);
-
-    details = PropertyDetails(attributes, DATA, details.dictionary_index(),
-                              PropertyCellType::kNoCell);
-    dictionary->DetailsAtPut(entry, details);
-
-    // Elements of the arguments object in slow mode might be slow aliases.
-    if (is_arguments && element->IsAliasedArgumentsEntry()) {
+    if (element->IsAliasedArgumentsEntry()) {
       Handle<AliasedArgumentsEntry> entry =
           Handle<AliasedArgumentsEntry>::cast(element);
       Handle<Context> context(Context::cast(elements->get(0)));
       int context_index = entry->aliased_context_slot();
       DCHECK(!context->get(context_index)->IsTheHole());
       context->set(context_index, *value);
       // For elements that are still writable we keep slow aliasing.
       if (!details.IsReadOnly()) value = element;
     }
-    dictionary->ValueAtPut(entry, *value);
-  } else {
-    DCHECK(object->map()->is_extensible());
-    PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell);
-    Handle<SeededNumberDictionary> new_dictionary =
-        SeededNumberDictionary::AddNumberEntry(dictionary, index, value,
-                                               details);
-    if (*dictionary != *new_dictionary) {
-      if (is_arguments) {
-        elements->set(1, *new_dictionary);
-      } else {
-        object->set_elements(*new_dictionary);
-      }
-      dictionary = new_dictionary;
+  }
+
+  dictionary->ValueAtPut(entry, *value);
+}
+
+
+void JSObject::AddDictionaryElement(Handle<JSObject> object, uint32_t index,
+                                    Handle<Object> value,
+                                    PropertyAttributes attributes) {
+  // TODO(verwaest): Handle with the elements accessor.
+  Isolate* isolate = object->GetIsolate();
+
+  // Insert element in the dictionary.
+  Handle<FixedArray> elements(FixedArray::cast(object->elements()));
+  bool is_arguments =
+      (elements->map() == isolate->heap()->sloppy_arguments_elements_map());
+
+  DCHECK(object->HasDictionaryElements() ||
+         object->HasDictionaryArgumentsElements());
+
+  Handle<SeededNumberDictionary> dictionary(
+      is_arguments ? SeededNumberDictionary::cast(elements->get(1))
+                   : SeededNumberDictionary::cast(*elements));
+
+#ifdef DEBUG
+  int entry = dictionary->FindEntry(index);
+  DCHECK_EQ(SeededNumberDictionary::kNotFound, entry);
+  DCHECK(object->map()->is_extensible());
+#endif
+
+  PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell);
+  Handle<SeededNumberDictionary> new_dictionary =
+      SeededNumberDictionary::AddNumberEntry(dictionary, index, value, details);
+  if (*dictionary != *new_dictionary) {
+    if (is_arguments) {
+      elements->set(1, *new_dictionary);
+    } else {
+      object->set_elements(*new_dictionary);
     }
+    dictionary = new_dictionary;
   }
 
   // Update the array length if this JSObject is an array.
   if (object->IsJSArray()) {
     JSArray::JSArrayUpdateLengthFromIndex(Handle<JSArray>::cast(object), index,
                                           value);
   }
 
   // Attempt to put this object back in fast case.
   if (object->ShouldConvertToFastElements()) {
@@ skipping 20 matching lines @@
 #ifdef DEBUG
     if (FLAG_trace_normalization) {
       OFStream os(stdout);
       os << "Object elements are fast case again:\n";
       object->Print(os);
     }
 #endif
   }
 }
 
-void JSObject::SetFastDoubleElement(Handle<JSObject> object, uint32_t index,
+
+void JSObject::AddFastDoubleElement(Handle<JSObject> object, uint32_t index,
                                     Handle<Object> value) {
   DCHECK(object->HasFastDoubleElements());
 
   Handle<FixedArrayBase> base_elms(FixedArrayBase::cast(object->elements()));
   uint32_t elms_length = static_cast<uint32_t>(base_elms->length());
   uint32_t length = elms_length;
 
   bool introduces_holes = true;
   if (object->IsJSArray()) {
     // In case of JSArray, the length does not equal the capacity.
     CHECK(Handle<JSArray>::cast(object)->length()->ToArrayLength(&length));
     introduces_holes = index > length;
   } else {
     introduces_holes = index >= elms_length;
   }
 
   // If the value object is not a heap number, switch to fast elements and try
   // again.
   if (!value->IsNumber()) {
     SetFastElementsCapacityAndLength(object, elms_length, length,
                                      kDontAllowSmiElements);
-    SetFastElement(object, index, value);
+    AddFastElement(object, index, value);
     return;
   }
 
   // If the array is growing, and it's not growth by a single element at the
   // end, make sure that the ElementsKind is HOLEY.
-  ElementsKind elements_kind = object->GetElementsKind();
-  if (introduces_holes && !IsFastHoleyElementsKind(elements_kind)) {
-    ElementsKind transitioned_kind = GetHoleyElementsKind(elements_kind);
+  if (introduces_holes && !IsFastHoleyElementsKind(object->GetElementsKind())) {
+    ElementsKind transitioned_kind = FAST_HOLEY_DOUBLE_ELEMENTS;
     TransitionElementsKind(object, transitioned_kind);
   }
 
   // Check whether there is extra space in the fixed array.
   if (index < elms_length) {
     Handle<FixedDoubleArray> elms(FixedDoubleArray::cast(object->elements()));
     elms->set(index, value->Number());
     if (object->IsJSArray()) {
       // Update the length of the array if needed.
       uint32_t array_length = 0;
@@ skipping 20 matching lines @@
   }
 
   // Otherwise default to slow case.
   DCHECK(object->HasFastDoubleElements());
   DCHECK(object->map()->has_fast_double_elements());
   DCHECK(object->elements()->IsFixedDoubleArray() ||
          object->elements()->length() == 0);
 
   NormalizeElements(object);
   DCHECK(object->HasDictionaryElements());
-  SetDictionaryElement(object, index, value, NONE);
+  AddDictionaryElement(object, index, value, NONE);
 }
 
 
 // static
 MaybeHandle<Object> JSReceiver::SetElement(Handle<JSReceiver> object,
                                            uint32_t index, Handle<Object> value,
                                            LanguageMode language_mode) {
   Isolate* isolate = object->GetIsolate();
   LookupIterator it(isolate, object, index);
   return SetProperty(&it, value, language_mode, MAY_BE_STORE_FROM_KEYED);
@@ skipping 21 matching lines @@
     d->set_requires_slow_elements();
   }
 
   Handle<Object> result = value;
 
   switch (receiver->GetElementsKind()) {
     case FAST_SMI_ELEMENTS:
     case FAST_ELEMENTS:
     case FAST_HOLEY_SMI_ELEMENTS:
     case FAST_HOLEY_ELEMENTS:
-      SetFastElement(receiver, index, value);
+      AddFastElement(receiver, index, value);
       break;
     case FAST_DOUBLE_ELEMENTS:
     case FAST_HOLEY_DOUBLE_ELEMENTS:
-      SetFastDoubleElement(receiver, index, value);
+      AddFastDoubleElement(receiver, index, value);
       break;
 
     case DICTIONARY_ELEMENTS:
-      SetDictionaryElement(receiver, index, value, attributes);
+      AddDictionaryElement(receiver, index, value, attributes);
       break;
     case SLOPPY_ARGUMENTS_ELEMENTS:
-      SetSloppyArgumentsElement(receiver, index, value, attributes);
+      AddSloppyArgumentsElement(receiver, index, value, attributes);
       break;
 
 // Elements cannot be added to typed arrays.
 #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
@@ skipping 3849 matching lines @@
                                             Handle<Object> new_value) {
   if (cell->value() != *new_value) {
     cell->set_value(*new_value);
     Isolate* isolate = cell->GetIsolate();
     cell->dependent_code()->DeoptimizeDependentCodeGroup(
         isolate, DependentCode::kPropertyCellChangedGroup);
   }
 }
 }  // namespace internal
 }  // namespace v8