Add support for computed property names in object literals

src/ia32/full-codegen-ia32.cc

Index: src/ia32/full-codegen-ia32.cc
diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index 030e6f6265f87040bf9218549a803c636416a826..ee798e156d7e02794649d7a088dc1ad79bceacd7 100644
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -1600,8 +1600,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
       : 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));
@@ -1629,11 +1628,13 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
   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
@@ -1641,6 +1642,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
     }
     switch (property->kind()) {
       case ObjectLiteral::Property::CONSTANT:
+      case ObjectLiteral::Property::COMPUTED_NAME:
         UNREACHABLE();
       case ObjectLiteral::Property::MATERIALIZED_LITERAL:
         ASSERT(!CompileTimeValue::IsCompileTimeValue(value));
@@ -1699,6 +1701,77 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
     __ 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));