[class] Evaluate static computed props during class definition

This patch evaluates computed properties in the order of declaration
during class definition time.

This patch creates a synthetic variable to store the result of
evaluating a computed property and then looks this up in the
initializer function.

Bug: v8:5367
Change-Id: I4182c6a01196d2538991818142890f6afb0e532b
Reviewed-on: https://chromium-review.googlesource.com/752567
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49115}

src/ast/ast.cc

@@ -281,7 +281,8 @@ ClassLiteralProperty::ClassLiteralProperty(Expression* key, Expression* value,
                                            bool is_computed_name)
     : LiteralProperty(key, value, is_computed_name),
       kind_(kind),
-      is_static_(is_static) {}
+      is_static_(is_static),
+      computed_name_var_(nullptr) {}
 
 bool ObjectLiteral::Property::IsCompileTimeValue() const {
   return kind_ == CONSTANT ||

src/ast/ast.h

@@ -2415,6 +2415,9 @@ class ClassLiteralProperty final : public LiteralProperty {
 
   bool is_static() const { return is_static_; }
 
+  void set_computed_name_var(Variable* var) { computed_name_var_ = var; }
+  Variable* computed_name_var() const { return computed_name_var_; }
+
  private:
   friend class AstNodeFactory;
 
@@ -2423,6 +2426,7 @@ class ClassLiteralProperty final : public LiteralProperty {
 
   Kind kind_;
   bool is_static_;
+  Variable* computed_name_var_;
 };
 
 class InitializeClassFieldsStatement final : public Statement {

src/interpreter/bytecode-generator.cc

@@ -1852,6 +1852,16 @@ void BytecodeGenerator::VisitClassLiteralProperties(ClassLiteral* expr,
           .JumpIfFalse(ToBooleanMode::kAlreadyBoolean, &done)
           .CallRuntime(Runtime::kThrowStaticPrototypeError)
           .Bind(&done);
+
+      if (property->kind() == ClassLiteral::Property::FIELD) {
+        DCHECK_NOT_NULL(property->computed_name_var());
+        builder()->LoadAccumulatorWithRegister(key);
+        BuildVariableAssignment(property->computed_name_var(), Token::INIT,
+                                HoleCheckMode::kElided);
+        // We don't define the field here, but instead do it in the
+        // initializer function.
+        continue;
+      }
     }
 
     VisitForRegisterValue(property->value(), value);
@@ -1910,10 +1920,20 @@ void BytecodeGenerator::VisitInitializeClassFieldsStatement(
   Register key = register_allocator()->NewRegister();
   Register value = register_allocator()->NewRegister();
 
-  // TODO(gsathya): Fix evaluation order for computed properties.
   for (int i = 0; i < expr->fields()->length(); i++) {
     ClassLiteral::Property* property = expr->fields()->at(i);
-    BuildLoadPropertyKey(property, key);
+
+    if (property->is_computed_name()) {
+      Variable* var = property->computed_name_var();
+      DCHECK_NOT_NULL(var);
+      // The computed name is already evaluated and stored in a
+      // variable at class definition time.
+      BuildVariableLoad(var, HoleCheckMode::kElided);
+      builder()->StoreAccumulatorInRegister(key);
+    } else {
+      BuildLoadPropertyKey(property, key);
+    }
+
     DataPropertyInLiteralFlags flags = DataPropertyInLiteralFlag::kNoFlags;
     FeedbackSlot slot = feedback_spec()->AddStoreDataPropertyInLiteralICSlot();
 

src/parsing/parser.cc

@@ -3194,6 +3194,16 @@ void Parser::DeclareClassVariable(const AstRawString* name,
   }
 }
 
+namespace {
+
+const AstRawString* ClassFieldVariableName(AstValueFactory* ast_value_factory,
+                                           int index) {
+  std::string name = ".class-field-" + std::to_string(index);
+  return ast_value_factory->GetOneByteString(name.c_str());
+}
+
+}  // namespace
+
 // This method declares a property of the given class.  It updates the
 // following fields of class_info, as appropriate:
 //   - constructor
@@ -3216,6 +3226,33 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
   if (property->kind() == ClassLiteralProperty::FIELD && is_static) {
     DCHECK(allow_harmony_class_fields());
     class_info->static_fields->Add(property, zone());
+    if (property->is_computed_name()) {
+      int index = class_info->static_fields->length();
+      // We create a synthetic variable name here so that scope
+      // analysis doesn't dedupe the vars.
+      //
+      // TODO(gsathya): Ideally, this should just bypass scope
+      // analysis and allocate a slot directly on the context. We
+      // should just store this index in the AST, instead of storing
+      // the variable.
+      const AstRawString* synthetic_name =
+          ClassFieldVariableName(ast_value_factory(), index);
+      VariableProxy* proxy =
+          factory()->NewVariableProxy(synthetic_name, NORMAL_VARIABLE);
+      Declaration* declaration =
+          factory()->NewVariableDeclaration(proxy, kNoSourcePosition);
+      Variable* computed_name_var =
+          Declare(declaration, DeclarationDescriptor::NORMAL, CONST,
+                  Variable::DefaultInitializationFlag(CONST), ok);
+      // Force context allocation because the computed property
+      // variable will accessed from inside the initializer
+      // function. We don't actually create a VariableProxy in the
+      // initializer function scope referring to this variable so
+      // scope analysis is unable to figure this out for us.
+      computed_name_var->ForceContextAllocation();
+      property->set_computed_name_var(computed_name_var);
+      class_info->properties->Add(property, zone());
+    }
   } else {
     class_info->properties->Add(property, zone());
   }

test/mjsunit/harmony/public-class-fields.js

@@ -258,3 +258,59 @@
   assertEquals(undefined, C.b);
   assertEquals(undefined, C.c);
 }
+
+{
+  var log = [];
+  function eval(i) {
+    log.push(i);
+    return i;
+  }
+
+  class C {
+    static [eval(1)] = eval(6);
+    static [eval(2)] = eval(7);
+    [eval(3)]() { eval(9);}
+    static [eval(4)] = eval(8);
+    static [eval(5)]() { throw new Error('should not execute');};
+  }
+
+  var c = new C;
+  c[3]();
+  assertEquals([1, 2, 3, 4, 5, 6, 7, 8, 9], log);
+}
+
+
+
+function x() {
+
+  // This tests lazy parsing.
+  return function() {
+    var log = [];
+    function eval(i) {
+      log.push(i);
+      return i;
+    }
+
+    class C {
+      static [eval(1)] = eval(6);
+      static [eval(2)] = eval(7);
+      [eval(3)]() { eval(9);}
+      static [eval(4)] = eval(8);
+      static [eval(5)]() { throw new Error('should not execute');};
+    }
+
+    var c = new C;
+    c[3]();
+    assertEquals([1, 2, 3, 4, 5, 6, 7, 8, 9], log);
+  }
+}
+
+{
+  class C {}
+  class D {
+    static [C];
+  }
+
+  assertThrows(() => { class X { static [X] } });
+  assertEquals(undefined, D[C]);
+}