Install the 'name' property in classes at runtime

src/interpreter/bytecode-generator.cc

Index: src/interpreter/bytecode-generator.cc
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index c67e5d43f706b797a63f5fe9b77257d0fefefe69..db6643fd62dcfcb722aa1be0e8bcbf714c83fd05 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -571,8 +571,8 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
       generator_state_(),
       loop_depth_(0),
       home_object_symbol_(info->isolate()->factory()->home_object_symbol()),
-      prototype_string_(info->isolate()->factory()->prototype_string()) {
-}
+      prototype_string_(info->isolate()->factory()->prototype_string()),
+      name_string_(info->isolate()->factory()->name_string()) {}
 
 Handle<BytecodeArray> BytecodeGenerator::FinalizeBytecode(Isolate* isolate) {
   AllocateDeferredConstants();
@@ -1382,7 +1382,7 @@ void BytecodeGenerator::VisitClassLiteral(ClassLiteral* expr) {
   VisitClassLiteralForRuntimeDefinition(expr);
 
   // Load the "prototype" from the constructor.
-  RegisterList args = register_allocator()->NewRegisterList(2);
+  RegisterList args = register_allocator()->NewRegisterList(3);
   Register literal = args[0];
   Register prototype = args[1];
   FeedbackVectorSlot slot = expr->PrototypeSlot();
@@ -1391,7 +1391,9 @@ void BytecodeGenerator::VisitClassLiteral(ClassLiteral* expr) {
       .LoadNamedProperty(literal, prototype_string(), feedback_index(slot))
       .StoreAccumulatorInRegister(prototype);
 
-  VisitClassLiteralProperties(expr, literal, prototype);
+  Register needs_name = args[2];

[rmcilroy, 2016/11/28 10:28:59]

This shouldn't be allocated in the same RegisterList as args since it's never
passed to a runtime call (actually looking at this, I'm not sure why we use a
RegisterList for this at all any more). It should just be a single register.
Saying that, I'm not keen on this approach of passing a register as a boolean
value (see below).

+  VisitClassLiteralProperties(expr, literal, prototype, needs_name);
+  BuildClassLiteralNameProperty(expr, literal, needs_name);
   builder()->CallRuntime(Runtime::kToFastProperties, literal);
   // Assign to class variable.
   if (expr->class_variable_proxy() != nullptr) {
@@ -1420,7 +1422,8 @@ void BytecodeGenerator::VisitClassLiteralForRuntimeDefinition(
 
 void BytecodeGenerator::VisitClassLiteralProperties(ClassLiteral* expr,
                                                     Register literal,
-                                                    Register prototype) {
+                                                    Register prototype,
+                                                    Register needs_name) {
   RegisterAllocationScope register_scope(this);
   RegisterList args = register_allocator()->NewRegisterList(5);
   Register receiver = args[0], key = args[1], value = args[2], attr = args[3],
@@ -1429,6 +1432,13 @@ void BytecodeGenerator::VisitClassLiteralProperties(ClassLiteral* expr,
   bool attr_assigned = false;
   Register old_receiver = Register::invalid_value();
 
+  // Initialize has_name_static_property.
+  bool check_name = !expr->has_name_static_property() &&

[Toon Verwaest, 2016/11/28 10:40:58]

Isn't the general case that we don't have a static computed property name? If
so, we should avoid this additional work in that case.

+                    !expr->constructor()->raw_name()->IsEmpty();
+  if (check_name) {
+    builder()->LoadTrue().StoreAccumulatorInRegister(needs_name);
+  }
+
   // Create nodes to store method values into the literal.
   for (int i = 0; i < expr->properties()->length(); i++) {
     ClassLiteral::Property* property = expr->properties()->at(i);
@@ -1444,6 +1454,19 @@ void BytecodeGenerator::VisitClassLiteralProperties(ClassLiteral* expr,
     builder()->ConvertAccumulatorToName(key);
 
     if (property->is_static() && property->is_computed_name()) {
+      // Check for a static property called "name".
+      if (check_name) {
+        BytecodeLabel has_name;
+        builder()
+            ->LoadAccumulatorWithRegister(needs_name)
+            .JumpIfFalse(&has_name)
+            .LoadLiteral(name_string())
+            .CompareOperation(Token::Value::EQ_STRICT, key)
+            .LogicalNot()
+            .StoreAccumulatorInRegister(needs_name)
+            .Bind(&has_name);
+      }

[rmcilroy, 2016/11/28 10:28:59]

I'm not keen on this extra code added to every property initialization. Do we
need it, or could we just rely on the compile time checks (e.g.,
expr->constructor()->raw_name()->IsEmpty() etc.) and have the runtime call check
if there was a computed property name which added the name property ?

[Toon Verwaest, 2016/11/28 10:40:58]

I agree, we should just do something more expensive in the runtime if this was
the case. It shouldn't be the common case anyway.

We can add it if we decide we it's common enough to avoid the runtime call
altogether.

+
       // The static prototype property is read only. We handle the non computed
       // property name case in the parser. Since this is the only case where we
       // need to check for an own read only property we special case this so we
@@ -1493,6 +1516,21 @@ void BytecodeGenerator::VisitClassLiteralProperties(ClassLiteral* expr,
   }
 }
 
+void BytecodeGenerator::BuildClassLiteralNameProperty(ClassLiteral* expr,
+                                                      Register literal,
+                                                      Register needs_name) {
+  bool check_name = !expr->has_name_static_property() &&
+                    !expr->constructor()->raw_name()->IsEmpty();
+  if (check_name) {
+    BytecodeLabel has_name;
+    builder()
+        ->LoadAccumulatorWithRegister(needs_name)
+        .JumpIfFalse(&has_name)
+        .CallRuntime(Runtime::kInstallClassNameAccessor, literal)
+        .Bind(&has_name);
+  }
+}
+
 void BytecodeGenerator::VisitNativeFunctionLiteral(
     NativeFunctionLiteral* expr) {
   size_t entry = builder()->AllocateConstantPoolEntry();