Use generic array functions to setup the boilerplate to avoid logic duplication.

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index a7bb4e7985f7a3ac75de531d41415a4d73f47b5a..9b6a723fc1cefe3f744183dadbe922456f11a397 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3714,16 +3714,16 @@ Expression* Parser::ParseArrayLiteral(bool* ok) {
   int literal_index = current_function_state_->NextMaterializedLiteralIndex();
 
   // Allocate a fixed array to hold all the object literals.
-  Handle<FixedArray> object_literals =
-      isolate()->factory()->NewFixedArray(values->length(), TENURED);
-  Handle<FixedDoubleArray> double_literals;
-  ElementsKind elements_kind = FAST_SMI_ELEMENTS;
-  bool has_hole_values = false;
+  Handle<JSArray> array =
+      isolate()->factory()->NewJSArray(0, FAST_HOLEY_SMI_ELEMENTS);
+  isolate()->factory()->SetElementsCapacityAndLength(
+      array, values->length(), values->length());
 
   // Fill in the literals.
   Heap* heap = isolate()->heap();
   bool is_simple = true;
   int depth = 1;
+  bool is_holey = false;

[danno, 2012/11/27 09:03:57]

Can't you automatically track holiness through SetOwnElement? If you skip an
element, the array's elements kind should become holey.

[Toon Verwaest, 2012/11/27 10:02:39]

The reason is that I preallocate the full backing store. To do this properly, I
need to also set the length already. Otherwise, the conversion from smi to
double, or double to object, may cause the array to go into dictionary mode
since it looks sparse (when a large part of the array isn't filled in yet). Of
course, when I set the length to something larger than the values already
present, the type has to be HOLEY. Hence I have to keep track of holeyness by
hand.

On 2012/11/27 09:03:57, danno wrote:

   for (int i = 0, n = values->length(); i < n; i++) {
     MaterializedLiteral* m_literal = values->at(i)->AsMaterializedLiteral();
     if (m_literal != NULL && m_literal->depth() + 1 > depth) {
@@ -3731,83 +3731,33 @@ Expression* Parser::ParseArrayLiteral(bool* ok) {
     }
     Handle<Object> boilerplate_value = GetBoilerplateValue(values->at(i));
     if (boilerplate_value->IsTheHole()) {
-      has_hole_values = true;
-      object_literals->set_the_hole(i);
-      if (elements_kind == FAST_DOUBLE_ELEMENTS) {
-        double_literals->set_the_hole(i);
-      }
+      is_holey = true;
     } else if (boilerplate_value->IsUndefined()) {
       is_simple = false;
-      object_literals->set(i, Smi::FromInt(0));
-      if (elements_kind == FAST_DOUBLE_ELEMENTS) {
-        double_literals->set(i, 0);
-      }
-    } else {
-      // Examine each literal element, and adjust the ElementsKind if the
-      // literal element is not of a type that can be stored in the current
-      // ElementsKind.  Start with FAST_SMI_ONLY_ELEMENTS, and transition to
-      // FAST_DOUBLE_ELEMENTS and FAST_ELEMENTS as necessary.  Always remember
-      // the tagged value, no matter what the ElementsKind is in case we
-      // ultimately end up in FAST_ELEMENTS.
-      object_literals->set(i, *boilerplate_value);
-      if (elements_kind == FAST_SMI_ELEMENTS) {
-        // Smi only elements. Notice if a transition to FAST_DOUBLE_ELEMENTS or
-        // FAST_ELEMENTS is required.
-        if (!boilerplate_value->IsSmi()) {
-          if (boilerplate_value->IsNumber() && FLAG_smi_only_arrays) {
-            // Allocate a double array on the FAST_DOUBLE_ELEMENTS transition to
-            // avoid over-allocating in TENURED space.
-            double_literals = isolate()->factory()->NewFixedDoubleArray(
-                values->length(), TENURED);
-            // Copy the contents of the FAST_SMI_ONLY_ELEMENT array to the
-            // FAST_DOUBLE_ELEMENTS array so that they are in sync.
-            for (int j = 0; j < i; ++j) {
-              Object* smi_value = object_literals->get(j);
-              if (smi_value->IsTheHole()) {
-                double_literals->set_the_hole(j);
-              } else {
-                double_literals->set(j, Smi::cast(smi_value)->value());
-              }
-            }
-            double_literals->set(i, boilerplate_value->Number());
-            elements_kind = FAST_DOUBLE_ELEMENTS;
-          } else {
-            elements_kind = FAST_ELEMENTS;
-          }
-        }
-      } else if (elements_kind == FAST_DOUBLE_ELEMENTS) {
-        // Continue to store double values in to FAST_DOUBLE_ELEMENTS arrays
-        // until the first value is seen that can't be stored as a double.
-        if (boilerplate_value->IsNumber()) {
-          double_literals->set(i, boilerplate_value->Number());
-        } else {
-          elements_kind = FAST_ELEMENTS;
-        }
-      }
+      JSObject::SetOwnElement(
+          array, i, handle(Smi::FromInt(0), isolate()), kNonStrictMode);
+    } else if (!boilerplate_value->IsTheHole()) {

[danno, 2012/11/27 09:03:57]

You already know !boilerplate_value->IsTheHole() is true, since you checked
above.

[Toon Verwaest, 2012/11/27 10:02:39]

Done.

+      JSObject::SetOwnElement(array, i, boilerplate_value, kNonStrictMode);
     }
   }
 
+  Handle<FixedArrayBase> element_values(array->elements());
+
   // Simple and shallow arrays can be lazily copied, we transform the
   // elements array to a copy-on-write array.
   if (is_simple && depth == 1 && values->length() > 0 &&
-      elements_kind != FAST_DOUBLE_ELEMENTS) {
-    object_literals->set_map(heap->fixed_cow_array_map());
+      array->HasFastSmiOrObjectElements()) {
+    element_values->set_map(heap->fixed_cow_array_map());
   }
 
-  Handle<FixedArrayBase> element_values = elements_kind == FAST_DOUBLE_ELEMENTS
-      ? Handle<FixedArrayBase>(double_literals)
-      : Handle<FixedArrayBase>(object_literals);
-
   // Remember both the literal's constant values as well as the ElementsKind
   // in a 2-element FixedArray.
-  Handle<FixedArray> literals =
-      isolate()->factory()->NewFixedArray(2, TENURED);
+  Handle<FixedArray> literals = isolate()->factory()->NewFixedArray(2, TENURED);
 
-  if (has_hole_values || !FLAG_packed_arrays) {
-    elements_kind = GetHoleyElementsKind(elements_kind);
-  }
+  ElementsKind kind = array->GetElementsKind();
+  kind = is_holey ? GetHoleyElementsKind(kind) : GetPackedElementsKind(kind);

[danno, 2012/11/27 09:03:57]

Shouldn't be necessary, see above.

[Toon Verwaest, 2012/11/27 10:02:39]

Unfortunately it is...

On 2012/11/27 09:03:57, danno wrote:

 
-  literals->set(0, Smi::FromInt(elements_kind));
+  literals->set(0, Smi::FromInt(kind));
   literals->set(1, *element_values);
 
   return factory()->NewArrayLiteral(