Install the 'name' property in classes at runtime

src/interpreter/bytecode-generator.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/interpreter/bytecode-generator.h"
 
 #include "src/ast/compile-time-value.h"
 #include "src/ast/scopes.h"
 #include "src/code-stubs.h"
 #include "src/compilation-info.h"
@@ skipping 665 matching lines @@
       function_literals_(0, info->zone()),
       native_function_literals_(0, info->zone()),
       execution_control_(nullptr),
       execution_context_(nullptr),
       execution_result_(nullptr),
       register_allocator_(nullptr),
       generator_resume_points_(info->literal()->yield_count(), info->zone()),
       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();
   if (HasStackOverflow()) return Handle<BytecodeArray>();
   return builder()->ToBytecodeArray(isolate);
 }
 
 void BytecodeGenerator::AllocateDeferredConstants() {
   // Build global declaration pair arrays.
   for (GlobalDeclarationsBuilder* globals_builder : global_declarations_) {
@@ skipping 788 matching lines @@
   register_allocator()->PrepareForConsecutiveAllocations(2);
   Register literal = register_allocator()->NextConsecutiveRegister();
   Register prototype = register_allocator()->NextConsecutiveRegister();
   FeedbackVectorSlot slot = expr->PrototypeSlot();
   builder()
       ->StoreAccumulatorInRegister(literal)
       .LoadNamedProperty(literal, prototype_string(), feedback_index(slot))
       .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) {
     Variable* var = expr->class_variable_proxy()->var();
     FeedbackVectorSlot slot = expr->NeedsProxySlot()
                                   ? expr->ProxySlot()
                                   : FeedbackVectorSlot::Invalid();
     VisitVariableAssignment(var, Token::INIT, slot);
   }
   execution_result()->SetResultInAccumulator();
@@ skipping 99 matching lines @@
     Register key) {
   BytecodeLabel done;
   builder()
       ->LoadLiteral(prototype_string())
       .CompareOperation(Token::Value::EQ_STRICT, key)
       .JumpIfFalse(&done)
       .CallRuntime(Runtime::kThrowStaticPrototypeError, Register(0), 0)
       .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();
   builder()->CreateClosure(entry, NOT_TENURED);
   native_function_literals_.push_back(std::make_pair(expr, entry));
   execution_result()->SetResultInAccumulator();
 }
 
 void BytecodeGenerator::VisitDoExpression(DoExpression* expr) {
   VisitBlock(expr->block());
@@ skipping 1838 matching lines @@
   return execution_context()->scope()->language_mode();
 }
 
 int BytecodeGenerator::feedback_index(FeedbackVectorSlot slot) const {
   return TypeFeedbackVector::GetIndex(slot);
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8