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 5b3959f5c317e85875b160de8e97b7e1849c6ce9..873a653fd66257b6211785a6d3e119c87182cfbe 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -683,8 +683,9 @@ 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()),
+      empty_string_(info->isolate()->factory()->empty_string()) {}

[Henrique Ferreiro, 2016/10/17 11:41:11]

Leftover from a preview version

 
 Handle<BytecodeArray> BytecodeGenerator::FinalizeBytecode(Isolate* isolate) {
   AllocateDeferredConstants();
@@ -1493,6 +1494,9 @@ void BytecodeGenerator::VisitClassLiteral(ClassLiteral* expr) {
       .StoreAccumulatorInRegister(prototype);
 
   VisitClassLiteralProperties(expr, literal, prototype);
+
+  VisitClassLiteralNameProperty(expr, literal);
+
   builder()->CallRuntime(Runtime::kToFastProperties, literal, 1);
   // Assign to class variable.
   if (expr->class_variable_proxy() != nullptr) {
@@ -1612,6 +1616,45 @@ void BytecodeGenerator::VisitClassLiteralStaticPrototypeWithComputedName(
       .Bind(&done);
 }
 
+void BytecodeGenerator::VisitClassLiteralNameProperty(ClassLiteral* expr,

[rmcilroy, 2016/10/24 14:22:20]

Nit - BuildClassLiteralNameProperty

+                                                      Register literal) {
+  DCHECK(!expr->constructor()->name().is_null());
+
+  // Abort if 'name' is undefined.
+  // FIXME: fails many tests in a slow dcheck about dereferencing a handle
+  if (expr->constructor()->name()->length() == 0) return;

[Henrique Ferreiro, 2016/10/17 11:41:11]

Should I use a runtime function to check for this?

[rmcilroy, 2016/10/24 14:22:20]

You could either do this in the runtime function I suggest adding below, or have
the check not dereference a handle (e.g., check raw_name() or add some field on
the AST node which states whether the name is empty or not).

[caitp, 2016/10/24 15:56:10]

It looks like expr->class_variable_proxy() returns nullptr for anonymous
classes, so I think you could check that instead of checking the constructor
name.

[Henrique Ferreiro, 2016/11/08 22:28:58]

I have moved everything to a runtime function. Is there any overhead in calling
them? Should I first check class_variable_proxy() in case I can omit the call?

[caitp, 2016/11/08 22:59:40]

I'll defer to Ignition experts to answer that --- I assume it's not as bad as it
is to call a runtime function from JIT code, but I don't know.

[rmcilroy, 2016/11/10 12:37:44]

A runtime call is just as expensive as it would be from JIT code, so if we can
avoid generating one when we know statically from the parser that it's not
needed then that would be good.

[Toon Verwaest, 2016/11/24 07:16:23]

This is exactly what I was referring to. Checking it without dereferencing the
handle is easy because the AstValueFactory has the empty string; plus you can
just use IsEmpty() on the string as well.

+
+  BytecodeLabel end;
+
+  RegisterAllocationScope register_scope(this);
+  register_allocator()->PrepareForConsecutiveAllocations(5);
+  Register receiver = register_allocator()->NextConsecutiveRegister();
+  Register key = register_allocator()->NextConsecutiveRegister();
+  Register value = register_allocator()->NextConsecutiveRegister();
+  Register attr = register_allocator()->NextConsecutiveRegister();
+  Register set_function_name = register_allocator()->NextConsecutiveRegister();
+
+  // If "name" is already defined, exit.
+  builder()
+      ->MoveRegister(literal, receiver)
+      .LoadLiteral(name_string())
+      .StoreAccumulatorInRegister(key)
+      .CallRuntime(Runtime::kObjectHasOwnProperty, receiver, 2)
+      .JumpIfTrue(&end);
+
+  // Define "name" property.
+  builder()
+      ->LoadLiteral(expr->constructor()->name())
+      .StoreAccumulatorInRegister(value)
+      .LoadLiteral(Smi::FromInt(DONT_ENUM | READ_ONLY))
+      .StoreAccumulatorInRegister(attr)
+      .LoadLiteral(Smi::FromInt(0))
+      .StoreAccumulatorInRegister(set_function_name)
+      .CallRuntime(Runtime::kDefineDataPropertyInLiteral, receiver, 5);

[rmcilroy, 2016/10/24 14:22:20]

Rather than creating a load of bytecode for this (which calls into the runtime
anyway), how about just creating a runtime function which does what you want
(e.g., checks if name is undefined or already defined on the property, and if
not, sets it on the class object).

+
+  builder()->Bind(&end);
+}
+
 void BytecodeGenerator::VisitNativeFunctionLiteral(
     NativeFunctionLiteral* expr) {
   size_t entry = builder()->AllocateConstantPoolEntry();