Class fields, part 3 (backends)

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 1474 matching lines @@
   // Load the "prototype" from the constructor.
   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);
 
+  // TODO(bakkot) This shouldn't be necessary; I just couldn't figure out how to
+  // invoke the function literal with appropriate home object and receiver
+  // (mainly because of FeedbackVector check failures which are totally opaque
+  // to me)

[Dan Ehrenberg, 2016/09/10 04:06:54]

Seems OK; it'd be good to note that this is motivated by semantics under
discussion at https://github.com/tc39/proposal-class-public-fields/issues/50 .
From off-line discussion, sounds like you have reasons for structuring it this
way, so maybe reword the comment to explain what it's doing exactly (rather than
"shouldn't be necessary") and how it ties into the parser. It'd also help if you
wrote out the desugaring in pseudo-JS. The feedback vector seems to be used
analogously to elsewhere, so no need to comment on that.

[bakkot, 2016/09/12 23:05:15]

Added a more reasonable comment mentioning that this is only necessary to
implement a particular semantics which are not what is currently specified.

+  VariableProxy* static_initializer_proxy = expr->static_initializer_proxy();
+  if (static_initializer_proxy != nullptr) {
+    Variable* variable = static_initializer_proxy->var();
+    builder()->LoadAccumulatorWithRegister(literal);
+    VisitVariableAssignment(variable, Token::INIT,  // TODO(bakkot) tokens

[Dan Ehrenberg, 2016/09/10 04:06:54]

What is this TODO?

[bakkot, 2016/09/12 23:05:15]

Oh, I just wasn't sure if Token::INIT or Token::ASSIGN was proper here. Looking
more into the code, it doesn't appear to matter.

+                            FeedbackVectorSlot::Invalid());
+  }
+
   VisitClassLiteralProperties(expr, literal, prototype);
   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);
   }
@@ skipping 34 matching lines @@
   Register attr = register_allocator()->NextConsecutiveRegister();
   Register set_function_name = register_allocator()->NextConsecutiveRegister();
 
   bool attr_assigned = false;
   Register old_receiver = Register::invalid_value();
 
   // 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);
 
+    if (property->kind() == ClassLiteral::Property::FIELD &&
+        !property->is_static()) {
+      // This just does the ToName call on the field name and stores it in a

[Dan Ehrenberg, 2016/09/10 04:06:54]

Was Melissa O'Neill at Mudd when you went there? One thing that she told me that
really stuck was to never write just 'this', always write 'this <something>',
otherwise what you're saying becomes unintelligible.

It'd be much clearer to say something like, "the parser overloads the value of
this property with a variable which is used to store the possibly-computed name
of this property; desugar the result to assign the resulting key to the variable
for later access by the generated initializer function". If you could also write
out what the desugaring is in pseudo-JS, that would help.

[bakkot, 2016/09/12 23:05:15]

I've written a more reasonable if slightly wordy comment here also.

+      // variable (a proxy for which is the value() of this property)
+      DCHECK(property->value()->IsVariableProxy());
+      Variable* variable = property->value()->AsVariableProxy()->var();
+      VisitForAccumulatorValue(property->key());
+      VisitVariableAssignment(variable, Token::INIT,
+                              FeedbackVectorSlot::Invalid());
+      continue;
+    }
+
     // Set-up receiver.
     Register new_receiver = property->is_static() ? literal : prototype;
     if (new_receiver != old_receiver) {
       builder()->MoveRegister(new_receiver, receiver);
       old_receiver = new_receiver;
     }
 
     VisitForAccumulatorValue(property->key());
     builder()->ConvertAccumulatorToName(key);
     // The static prototype property is read only. We handle the non computed
@@ skipping 28 matching lines @@
         builder()->CallRuntime(Runtime::kDefineGetterPropertyUnchecked,
                                receiver, 4);
         break;
       }
       case ClassLiteral::Property::SETTER: {
         builder()->CallRuntime(Runtime::kDefineSetterPropertyUnchecked,
                                receiver, 4);
         break;
       }
       case ClassLiteral::Property::FIELD: {
-        UNREACHABLE();
+        DCHECK(property->is_static());
+        builder()
+            ->LoadLiteral(Smi::FromInt(property->NeedsSetFunctionName()))
+            .StoreAccumulatorInRegister(set_function_name);
+        builder()->CallRuntime(Runtime::kDefineDataPropertyInLiteral, receiver,
+                               5);
         break;
       }
     }
   }
 }
 
 void BytecodeGenerator::VisitClassLiteralStaticPrototypeWithComputedName(
     Register key) {
   BytecodeLabel done;
   builder()
@@ skipping 1783 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