Refactor class declaration logic to the parser and runtime

src/parsing/parser.cc

Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index 44d1a8145880902ce0e8e176b8a887b47c3de532..a3dd5912522acc0015e0e4b8c88e50558f314903 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -769,7 +769,7 @@ FunctionLiteral* ParserTraits::ParseFunctionLiteral(
       function_token_position, type, language_mode, ok);
 }
 
-ClassLiteral* ParserTraits::ParseClassLiteral(
+Expression* ParserTraits::ParseClassLiteral(
     Type::ExpressionClassifier* classifier, const AstRawString* name,
     Scanner::Location class_name_location, bool name_is_strict_reserved,
     int pos, bool* ok) {
@@ -2236,8 +2236,8 @@ Statement* Parser::ParseClassDeclaration(ZoneList<const AstRawString*>* names,
     variable_name = name;
   }
 
-  ClassLiteral* value = ParseClassLiteral(nullptr, name, scanner()->location(),
-                                          is_strict_reserved, pos, CHECK_OK);
+  Expression* value = ParseClassLiteral(nullptr, name, scanner()->location(),
+                                        is_strict_reserved, pos, CHECK_OK);
 
   VariableProxy* proxy = NewUnresolved(variable_name, LET);
   Declaration* declaration =
@@ -4974,11 +4974,11 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
   return result;
 }
 
-ClassLiteral* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
-                                        const AstRawString* name,
-                                        Scanner::Location class_name_location,
-                                        bool name_is_strict_reserved, int pos,
-                                        bool* ok) {
+Expression* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
+                                      const AstRawString* name,
+                                      Scanner::Location class_name_location,
+                                      bool name_is_strict_reserved, int pos,
+                                      bool* ok) {
   // All parts of a ClassDeclaration and ClassExpression are strict code.
   if (name_is_strict_reserved) {
     ReportMessageAt(class_name_location,
@@ -5022,8 +5022,15 @@ ClassLiteral* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
 
 
   ClassLiteralChecker checker(this);
-  ZoneList<ObjectLiteral::Property*>* properties = NewPropertyList(4, zone());
   FunctionLiteral* constructor = NULL;
+
+  ZoneList<Expression*>* property_names =
+      new (zone()) ZoneList<Expression*>(4, zone());
+  ZoneList<Expression*>* property_funcs =
+      new (zone()) ZoneList<Expression*>(4, zone());
+  ZoneList<Expression*>* property_infos =
+      new (zone()) ZoneList<Expression*>(4, zone());
+
   bool has_seen_constructor = false;
 
   Expect(Token::LBRACE, CHECK_OK);
@@ -5052,7 +5059,50 @@ ClassLiteral* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
       constructor->set_raw_name(
           name != nullptr ? name : ast_value_factory()->empty_string());
     } else {
-      properties->Add(property, zone());
+      Expression* value = property->value();
+      int property_position = value->position();
+
+      Expression* property_key;
+
+      if (property_name != nullptr) {
+        // For static keys, no ToName call is necessary.
+        if (property->key()->IsPropertyName()) {
+          property_key = property->key();
+        } else {
+          property_key =
+              factory()->NewStringLiteral(property_name, property_position);
+        }
+      } else {
+        ZoneList<Expression*>* to_name_args =
+            new (zone()) ZoneList<Expression*>(1, zone());
+        to_name_args->Add(property->key(), zone());
+
+        property_key = factory()->NewCallRuntime(Runtime::kToName, to_name_args,

[Benedikt Meurer, 2016/07/19 04:04:27]

Nit: Please add a TODO here, that we look into this %ToName again, once
performance becomes an issue.

+                                                 property_position);
+      }
+
+      int info = property->is_static() ? kIsStaticMask : 0;
+      switch (property->kind()) {
+        case ObjectLiteral::Property::CONSTANT:
+        case ObjectLiteral::Property::MATERIALIZED_LITERAL:
+        case ObjectLiteral::Property::PROTOTYPE:
+          UNREACHABLE();
+        case ObjectLiteral::Property::COMPUTED:
+          break;
+
+        case ObjectLiteral::Property::GETTER:
+          info |= kIsAccessorMask;
+          break;
+
+        case ObjectLiteral::Property::SETTER:
+          info |= kIsAccessorMask | kIsSetterMask;
+          break;
+      }
+
+      property_names->Add(property_key, zone());
+      property_funcs->Add(value, zone());
+      property_infos->Add(factory()->NewNumberLiteral(info, kNoSourcePosition),
+                          zone());
     }
 
     if (fni_ != NULL) fni_->Infer();
@@ -5079,8 +5129,86 @@ ClassLiteral* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
     proxy->var()->set_initializer_position(end_pos);
   }
 
-  return factory()->NewClassLiteral(block_scope, proxy, extends, constructor,
-                                    properties, pos, end_pos);
+  // Construct a DoExpression representing the class literal.
+  Block* do_block = factory()->NewBlock(nullptr, 1, false, pos);
+  do_block->set_scope(scope_);
+
+  // Call DefineClass. It takes seven arguments:

[Benedikt Meurer, 2016/07/19 04:04:27]

I remember when we discussed this, we wanted to actually remove the need for
%DefineClass. I.e. use the existing primitives to create the prototype.

Is it considered beneficial to use %DefineClass instead of using stores for this
purpose?

[Dan Ehrenberg, 2016/07/19 04:52:33]

The issue here is that the properties that are created for methods need to be
non-enumerable. Is there a way to do that in the AST currently? Seems like the
main task would be to create an object literal or store AST node which creates
non-enumerable properties, and implement it in backends.

+  // 1. The superclass constructor, or the Hole if not present.
+  // 2. The constructor function.
+  // 3. An array of method names.
+  // 4. An array of said method's corresponding functions.
+  // 5. An array of flags signalling if they are accessors and/or static.
+  // 6. The location where the class's source starts.
+  // 7. The location where the class's source ends.
+  ZoneList<Expression*>* define_class_args =
+      new (zone()) ZoneList<Expression*>(7, zone());
+  define_class_args->Add(extends != nullptr
+                             ? extends
+                             : factory()->NewTheHoleLiteral(kNoSourcePosition),
+                         zone());
+  define_class_args->Add(constructor, zone());
+
+  int names_idx = function_state_->NextMaterializedLiteralIndex();
+  int funcs_idx = function_state_->NextMaterializedLiteralIndex();
+  int infos_idx = function_state_->NextMaterializedLiteralIndex();
+
+  define_class_args->Add(

[Benedikt Meurer, 2016/07/19 04:04:27]

Why do we wrap these into JSArrays? That seems rather wasteful. Since all of
this is internal, can't we just pass the FixedArrays directly instead?

[Dan Ehrenberg, 2016/07/19 04:52:33]

Is there a way to create FixedArrays from the parser? I think you currently have
to be at a lower level beyond this stage to do that sort of thing, rather than
an AST node.

[marja, 2016/07/19 09:54:34]

FixedArray is a heap object, so no. This will be called on a background thread,
so we can't interact with the V8 heap.

+      factory()->NewArrayLiteral(property_names, names_idx, kNoSourcePosition),
+      zone());
+  define_class_args->Add(
+      factory()->NewArrayLiteral(property_funcs, funcs_idx, kNoSourcePosition),
+      zone());
+  define_class_args->Add(
+      factory()->NewArrayLiteral(property_infos, infos_idx, kNoSourcePosition),
+      zone());
+
+  define_class_args->Add(factory()->NewNumberLiteral(pos, kNoSourcePosition),
+                         zone());
+  define_class_args->Add(
+      factory()->NewNumberLiteral(end_pos, kNoSourcePosition), zone());
+
+  Expression* define_class_call = factory()->NewCallRuntime(
+      Runtime::kDefineClass, define_class_args, kNoSourcePosition);
+
+  // Set the constructor to have fast properties.
+  ZoneList<Expression*>* fast_properties_args =
+      new (zone()) ZoneList<Expression*>(1, zone());
+  fast_properties_args->Add(define_class_call, zone());
+
+  // This call returns its argument, so we can chain it.
+  Expression* fast_properties_call = factory()->NewCallRuntime(
+      Runtime::kToFastProperties, fast_properties_args, kNoSourcePosition);
+
+  if (name != nullptr) {
+    DCHECK_NOT_NULL(proxy);
+    // Init the variable representing the class name only after evaluating all
+    // property names.
+    Expression* name_assignment = factory()->NewAssignment(
+        Token::INIT, proxy, fast_properties_call, end_pos);
+
+    do_block->statements()->Add(
+        factory()->NewExpressionStatement(name_assignment, end_pos), zone());
+  } else {
+    do_block->statements()->Add(factory()->NewExpressionStatement(
+                                    fast_properties_call, kNoSourcePosition),
+                                zone());
+  }
+
+  Variable* result = scope_->NewTemporary(ast_value_factory()->empty_string());
+  DoExpression* expr = factory()->NewDoExpression(do_block, result, pos);
+
+  Rewriter::Rewrite(this, expr,
+                    ast_value_factory());  // TODO(bakkot) This is just going to
+                                           // turn the final ExpressionStatement
+                                           // into an assignment to result; we
+                                           // could do that ourselves slightly
+                                           // cheaper.
+
+  // This enables function name inference.
+  expr->set_represented_function(constructor);
+
+  return expr;
 }
 
 
@@ -6056,9 +6184,10 @@ void ParserTraits::SetFunctionNameFromPropertyName(
     function->set_raw_name(name);
     DCHECK_EQ(ObjectLiteralProperty::COMPUTED, property->kind());
   } else {
-    DCHECK(value->IsClassLiteral());
+    DCHECK(value->IsDoExpression());
     DCHECK_EQ(ObjectLiteralProperty::COMPUTED, property->kind());
-    value->AsClassLiteral()->constructor()->set_raw_name(name);
+    DCHECK_NOT_NULL(value->AsDoExpression()->represented_function());
+    value->AsDoExpression()->represented_function()->set_raw_name(name);
   }
 }
 
@@ -6075,8 +6204,9 @@ void ParserTraits::SetFunctionNameFromIdentifierRef(Expression* value,
   if (function != nullptr) {
     function->set_raw_name(name);
   } else {
-    DCHECK(value->IsClassLiteral());
-    value->AsClassLiteral()->constructor()->set_raw_name(name);
+    DCHECK(value->IsDoExpression());
+    DCHECK_NOT_NULL(value->AsDoExpression()->represented_function());
+    value->AsDoExpression()->represented_function()->set_raw_name(name);
   }
 }