Add support for computed property names in object literals

src/ia32/full-codegen-ia32.cc

 // Copyright 2012 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 "src/v8.h"
 
 #if V8_TARGET_ARCH_IA32
 
 #include "src/code-stubs.h"
 #include "src/codegen.h"
@@ skipping 1582 matching lines @@
   expr->BuildConstantProperties(isolate());
   Handle<FixedArray> constant_properties = expr->constant_properties();
   int flags = expr->fast_elements()
       ? ObjectLiteral::kFastElements
       : ObjectLiteral::kNoFlags;
   flags |= expr->has_function()
       ? ObjectLiteral::kHasFunction
       : ObjectLiteral::kNoFlags;
   int properties_count = constant_properties->length() / 2;
   if (expr->may_store_doubles() || expr->depth() > 1 ||
-      masm()->serializer_enabled() ||
-      flags != ObjectLiteral::kFastElements ||
+      masm()->serializer_enabled() || flags != ObjectLiteral::kFastElements ||
       properties_count > FastCloneShallowObjectStub::kMaximumClonedProperties) {
     __ mov(edi, Operand(ebp, JavaScriptFrameConstants::kFunctionOffset));
     __ push(FieldOperand(edi, JSFunction::kLiteralsOffset));
     __ push(Immediate(Smi::FromInt(expr->literal_index())));
     __ push(Immediate(constant_properties));
     __ push(Immediate(Smi::FromInt(flags)));
     __ CallRuntime(Runtime::kHiddenCreateObjectLiteral, 4);
   } else {
     __ mov(edi, Operand(ebp, JavaScriptFrameConstants::kFunctionOffset));
     __ mov(eax, FieldOperand(edi, JSFunction::kLiteralsOffset));
     __ mov(ebx, Immediate(Smi::FromInt(expr->literal_index())));
     __ mov(ecx, Immediate(constant_properties));
     __ mov(edx, Immediate(Smi::FromInt(flags)));
     FastCloneShallowObjectStub stub(isolate(), properties_count);
     __ CallStub(&stub);
   }
 
   // If result_saved is true the result is on top of the stack.  If
   // result_saved is false the result is in eax.
   bool result_saved = false;
 
   // Mark all computed expressions that are bound to a key that
   // is shadowed by a later occurrence of the same key. For the
   // marked expressions, no store code is emitted.
   expr->CalculateEmitStore(zone());
 
   AccessorTable accessor_table(zone());
-  for (int i = 0; i < expr->properties()->length(); i++) {
-    ObjectLiteral::Property* property = expr->properties()->at(i);
+  int property_index = 0;
+  for (; property_index < expr->properties()->length(); property_index++) {
+    ObjectLiteral::Property* property = expr->properties()->at(property_index);
     if (property->IsCompileTimeValue()) continue;
+    if (property->kind() == ObjectLiteral::Property::COMPUTED_NAME) break;
 
-    Literal* key = property->key();
+    Literal* key = property->key()->AsLiteral();
     Expression* value = property->value();
     if (!result_saved) {
       __ push(eax);  // Save result on the stack
       result_saved = true;
     }
     switch (property->kind()) {
       case ObjectLiteral::Property::CONSTANT:
+      case ObjectLiteral::Property::COMPUTED_NAME:
         UNREACHABLE();
       case ObjectLiteral::Property::MATERIALIZED_LITERAL:
         ASSERT(!CompileTimeValue::IsCompileTimeValue(value));
         // Fall through.
       case ObjectLiteral::Property::COMPUTED:
         if (key->value()->IsInternalizedString()) {
           if (property->emit_store()) {
             VisitForAccumulatorValue(value);
             __ mov(ecx, Immediate(key->value()));
             __ mov(edx, Operand(esp, 0));
@@ skipping 38 matching lines @@
        it != accessor_table.end();
        ++it) {
     __ push(Operand(esp, 0));  // Duplicate receiver.
     VisitForStackValue(it->first);
     EmitAccessor(it->second->getter);
     EmitAccessor(it->second->setter);
     __ push(Immediate(Smi::FromInt(NONE)));
     __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
   }
 
+  // Object literals have two parts.  The "static" part on the left contains no

[rossberg, 2014/06/13 15:19:24]

I'm not convinced that it's best to duplicate this whole loop, given that it
shares a lot of (sub)cases. I found the previous version with the phase flag
somewhat more pleasant.

[wingo, 2014/06/16 08:32:27]

I tried to do this at first, but ran into problems with accessors.  Notably, in
the original code accessors are not set until the end, to allow the
AccessorsTable to accumulate both the getter and the setter for a value.  These
values are then set together in a post-pass.  

However in the computed properties case we need to set the "get" and "set"
accessors separately, when we see them.

I also had problems getting the switch fallthrough logic to do the right thing. 
I can give it another go if you think it's the right thing.

[rossberg, 2014/06/18 14:13:40]

Perhaps this problem also goes away with the spec change quoted above?

+  // computed property names, and so we can compute its map ahead of time; see
+  // runtime.cc::CreateObjectLiteralBoilerplate.  The second "dynamic" part
+  // starts with the first computed property name, and continues with all
+  // properties to its right.  All the code from above initializes the static
+  // component of the object literal, and arranges for the map of the result to
+  // reflect the static order in which the keys appear.  For the dynamic
+  // properties, we compile them into a series of "PutOwnProperty" runtime
+  // calls.  This will preserve insertion order.
+  for (; property_index < expr->properties()->length(); property_index++) {
+    ObjectLiteral::Property* property = expr->properties()->at(property_index);
+
+    Expression* key = property->key();
+    Expression* value = property->value();
+    if (!result_saved) {
+      __ push(eax);  // Save result on the stack
+      result_saved = true;
+    }
+
+    switch (property->kind()) {
+      case ObjectLiteral::Property::CONSTANT:
+      case ObjectLiteral::Property::COMPUTED_NAME:
+      case ObjectLiteral::Property::MATERIALIZED_LITERAL:
+      case ObjectLiteral::Property::COMPUTED:
+        __ push(Operand(esp, 0));  // Duplicate receiver.
+        VisitForStackValue(key);
+        VisitForStackValue(value);
+        if (property->emit_store()) {
+          __ CallRuntime(Runtime::kPutOwnProperty, 3);
+        } else {
+          __ Drop(3);
+        }
+        break;
+      case ObjectLiteral::Property::PROTOTYPE:
+        __ push(Operand(esp, 0));  // Duplicate receiver.
+        VisitForStackValue(value);
+        if (property->emit_store()) {
+          __ CallRuntime(Runtime::kSetPrototype, 2);
+        } else {
+          __ Drop(2);
+        }
+        break;
+      case ObjectLiteral::Property::GETTER: {
+        ObjectLiteral::Accessors *accessors =
+            accessor_table.lookup(key->AsLiteral())->second;
+        accessors->getter = value;
+
+        __ push(Operand(esp, 0));  // Duplicate receiver.
+        VisitForStackValue(key);
+        EmitAccessor(accessors->getter);
+        EmitAccessor(accessors->setter);
+        __ push(Immediate(Smi::FromInt(NONE)));
+        __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+        break;
+      }
+      case ObjectLiteral::Property::SETTER: {
+        ObjectLiteral::Accessors *accessors =
+            accessor_table.lookup(key->AsLiteral())->second;

[rossberg, 2014/06/13 15:19:24]

Is this correct? Can't accessors have computed property names as well?

[wingo, 2014/06/16 08:32:27]

According to spec, yes, but not in the current patch.  I can add a TODO.

+        accessors->setter = value;
+
+        __ push(Operand(esp, 0));  // Duplicate receiver.
+        VisitForStackValue(key);
+        EmitAccessor(accessors->getter);
+        EmitAccessor(accessors->setter);
+        __ push(Immediate(Smi::FromInt(NONE)));
+        __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+        break;
+      }
+    }
+  }
+
   if (expr->has_function()) {
     ASSERT(result_saved);
     __ push(Operand(esp, 0));
     __ CallRuntime(Runtime::kToFastProperties, 1);
   }
 
   if (result_saved) {
     context()->PlugTOS();
   } else {
     context()->Plug(eax);
@@ skipping 3091 matching lines @@
   ASSERT_EQ(isolate->builtins()->OsrAfterStackCheck()->entry(),
             Assembler::target_address_at(call_target_address,
                                          unoptimized_code));
   return OSR_AFTER_STACK_CHECK;
 }
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_IA32