Move desugaring of super calls with trailing spread to one runtime call.

Unfortunately we have to split this up into two cases: those with exactly one spread argument as the final argument, and all others, due to any side-effects of evaluation being visible.

This is in preparation for a new bytecode which handles super calls.

BUG=v8:5659

Review-Url: https://codereview.chromium.org/2540593003
Cr-Commit-Position: refs/heads/master@{#41415}

src/ast/ast-numbering.cc

@@ -393,9 +393,13 @@ void AstNumberingVisitor::VisitCompareOperation(CompareOperation* node) {
   ReserveFeedbackSlots(node);
 }
 
-
-void AstNumberingVisitor::VisitSpread(Spread* node) { UNREACHABLE(); }
-
+void AstNumberingVisitor::VisitSpread(Spread* node) {
+  IncrementNodeCount();
+  // We can only get here from super calls currently.
+  DisableFullCodegenAndCrankshaft(kSuperReference);
+  node->set_base_id(ReserveIdRange(Spread::num_ids()));
+  Visit(node->expression());
+}
 
 void AstNumberingVisitor::VisitEmptyParentheses(EmptyParentheses* node) {
   UNREACHABLE();

src/interpreter/bytecode-generator.cc

@@ -2488,24 +2488,49 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
   Register constructor = this_function;  // Re-use dead this_function register.
   builder()->StoreAccumulatorInRegister(constructor);
 
-  RegisterList args =
-      register_allocator()->NewRegisterList(expr->arguments()->length());
-  VisitArguments(expr->arguments(), args);
-
-  // The new target is loaded into the accumulator from the
-  // {new.target} variable.
-  VisitForAccumulatorValue(super->new_target_var());
-
-  // Call construct.
-  builder()->SetExpressionPosition(expr);
-  // TODO(turbofan): For now we do gather feedback on super constructor
-  // calls, utilizing the existing machinery to inline the actual call
-  // target and the JSCreate for the implicit receiver allocation. This
-  // is not an ideal solution for super constructor calls, but it gets
-  // the job done for now. In the long run we might want to revisit this
-  // and come up with a better way.
-  int const feedback_slot_index = feedback_index(expr->CallFeedbackICSlot());
-  builder()->New(constructor, args, feedback_slot_index);
+  ZoneList<Expression*>* args = expr->arguments();
+
+  // When a super call contains a spread, a CallSuper AST node is only created
+  // if there is exactly one spread, and it is the last argument.
+  if (!args->is_empty() && args->last()->IsSpread()) {
+    // Prepare the spread arguments.
+    RegisterList spread_prepare_args =
+        register_allocator()->NewRegisterList(args->length());
+    VisitArguments(args, spread_prepare_args);
+
+    builder()->CallRuntime(Runtime::kSpreadIterablePrepareVarargs,
+                           spread_prepare_args);
+
+    // Call ReflectConstruct to do the actual super constructor call.
+    RegisterList reflect_construct_args =
+        register_allocator()->NewRegisterList(4);
+    builder()
+        ->StoreAccumulatorInRegister(reflect_construct_args[2])
+        .LoadUndefined()
+        .StoreAccumulatorInRegister(reflect_construct_args[0])
+        .MoveRegister(constructor, reflect_construct_args[1]);
+    VisitForRegisterValue(super->new_target_var(), reflect_construct_args[3]);
+    builder()->CallJSRuntime(Context::REFLECT_CONSTRUCT_INDEX,
+                             reflect_construct_args);
+  } else {
+    RegisterList args_regs =
+        register_allocator()->NewRegisterList(args->length());
+    VisitArguments(args, args_regs);
+    // The new target is loaded into the accumulator from the
+    // {new.target} variable.
+    VisitForAccumulatorValue(super->new_target_var());
+
+    // Call construct.
+    builder()->SetExpressionPosition(expr);
+    // TODO(turbofan): For now we do gather feedback on super constructor
+    // calls, utilizing the existing machinery to inline the actual call
+    // target and the JSCreate for the implicit receiver allocation. This
+    // is not an ideal solution for super constructor calls, but it gets
+    // the job done for now. In the long run we might want to revisit this
+    // and come up with a better way.
+    int const feedback_slot_index = feedback_index(expr->CallFeedbackICSlot());
+    builder()->New(constructor, args_regs, feedback_slot_index);
+  }
 }
 
 void BytecodeGenerator::VisitCallNew(CallNew* expr) {
@@ -2813,7 +2838,7 @@ void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* expr) {
   builder()->BinaryOperation(expr->op(), lhs, feedback_index(slot));
 }
 
-void BytecodeGenerator::VisitSpread(Spread* expr) { UNREACHABLE(); }
+void BytecodeGenerator::VisitSpread(Spread* expr) { Visit(expr->expression()); }
 
 void BytecodeGenerator::VisitEmptyParentheses(EmptyParentheses* expr) {
   UNREACHABLE();

src/parsing/parser-base.h

@@ -3152,7 +3152,6 @@ ParserBase<Impl>::ParseLeftHandSideExpression(bool* ok) {
 
         bool is_super_call = result->IsSuperCallReference();
         if (spread_pos.IsValid()) {
-          args = impl()->PrepareSpreadArguments(args);
           result = impl()->SpreadCall(result, args, pos);
         } else {
           result = factory()->NewCall(result, args, pos, is_possibly_eval);
@@ -3244,7 +3243,6 @@ ParserBase<Impl>::ParseMemberWithNewPrefixesExpression(bool* is_async,
       ExpressionListT args = ParseArguments(&spread_pos, CHECK_OK);
 
       if (spread_pos.IsValid()) {
-        args = impl()->PrepareSpreadArguments(args);
         result = impl()->SpreadCallNew(result, args, new_pos);
       } else {
         result = factory()->NewCallNew(result, args, new_pos);

src/parsing/parser.cc

@@ -244,9 +244,7 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
 
     body = new (zone()) ZoneList<Statement*>(call_super ? 2 : 1, zone());
     if (call_super) {
-      // $super_constructor = %_GetSuperConstructor(<this-function>)
-      // %reflect_construct(
-      //     $super_constructor, InternalArray(...args), new.target)
+      // Create a SuperCallReference and handle in BytecodeGenerator.
       auto constructor_args_name = ast_value_factory()->empty_string();
       bool is_duplicate;
       bool is_rest = true;
@@ -256,29 +254,13 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
           ast_value_factory());
 
       ZoneList<Expression*>* args =
-          new (zone()) ZoneList<Expression*>(2, zone());
-      VariableProxy* this_function_proxy =
-          NewUnresolved(ast_value_factory()->this_function_string(), pos);
-      ZoneList<Expression*>* tmp =
           new (zone()) ZoneList<Expression*>(1, zone());
-      tmp->Add(this_function_proxy, zone());
-      Expression* super_constructor = factory()->NewCallRuntime(
-          Runtime::kInlineGetSuperConstructor, tmp, pos);
-      args->Add(super_constructor, zone());
       Spread* spread_args = factory()->NewSpread(
           factory()->NewVariableProxy(constructor_args), pos, pos);
-      ZoneList<Expression*>* spread_args_expr =
-          new (zone()) ZoneList<Expression*>(1, zone());
-      spread_args_expr->Add(spread_args, zone());
-      args->AddAll(*PrepareSpreadArguments(spread_args_expr), zone());
-      VariableProxy* new_target_proxy =
-          NewUnresolved(ast_value_factory()->new_target_string(), pos);
-      args->Add(new_target_proxy, zone());
-      Expression* call = factory()->NewCallRuntime(
-          Context::REFLECT_CONSTRUCT_INDEX, args, pos);
-      if (requires_class_field_init) {
-        call = CallClassFieldInitializer(scope(), call);
-      }
+
+      args->Add(spread_args, zone());
+      Expression* super_call_ref = NewSuperCallReference(pos);
+      Expression* call = factory()->NewCall(super_call_ref, args, pos);
       body->Add(factory()->NewReturnStatement(call, pos), zone());
     }
 
@@ -4076,6 +4058,23 @@ Expression* Parser::SpreadCall(Expression* function,
     // Super calls
     // $super_constructor = %_GetSuperConstructor(<this-function>)
     // %reflect_construct($super_constructor, args, new.target)
+
+    bool only_last_arg_is_spread = false;
+    for (int i = 0; i < args->length(); i++) {
+      if (args->at(i)->IsSpread()) {
+        if (i == args->length() - 1) {
+          only_last_arg_is_spread = true;
+        }
+        break;
+      }
+    }
+
+    if (only_last_arg_is_spread) {
+      // Handle in BytecodeGenerator.
+      Expression* super_call_ref = NewSuperCallReference(pos);
+      return factory()->NewCall(super_call_ref, args, pos);
+    }
+    args = PrepareSpreadArguments(args);
     ZoneList<Expression*>* tmp = new (zone()) ZoneList<Expression*>(1, zone());
     tmp->Add(function->AsSuperCallReference()->this_function_var(), zone());
     Expression* super_constructor = factory()->NewCallRuntime(
@@ -4085,6 +4084,7 @@ Expression* Parser::SpreadCall(Expression* function,
     return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args,
                                      pos);
   } else {
+    args = PrepareSpreadArguments(args);
     if (function->IsProperty()) {
       // Method calls
       if (function->AsProperty()->IsSuperAccess()) {
@@ -4115,6 +4115,7 @@ Expression* Parser::SpreadCall(Expression* function,
 
 Expression* Parser::SpreadCallNew(Expression* function,
                                   ZoneList<Expression*>* args, int pos) {
+  args = PrepareSpreadArguments(args);
   args->InsertAt(0, function, zone());
 
   return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args, pos);

src/parsing/preparser.h

@@ -953,11 +953,6 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE void MarkCollectedTailCallExpressions() {}
   V8_INLINE void MarkTailPosition(PreParserExpression expression) {}
 
-  V8_INLINE PreParserExpressionList
-  PrepareSpreadArguments(PreParserExpressionList list) {
-    return list;
-  }
-
   V8_INLINE PreParserExpression SpreadCall(PreParserExpression function,
                                            PreParserExpressionList args,
                                            int pos);

src/runtime/runtime-array.cc

@@ -630,11 +630,9 @@ RUNTIME_FUNCTION(Runtime_ArrayIndexOf) {
   return Smi::FromInt(-1);
 }
 
-RUNTIME_FUNCTION(Runtime_SpreadIterablePrepare) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(Object, spread, 0);
+namespace {
 
+bool MustIterate(Isolate* isolate, Handle<Object> spread) {
   if (spread->IsJSArray()) {
     // Check that the spread arg has fast elements
     Handle<JSArray> spread_array = Handle<JSArray>::cast(spread);
@@ -648,29 +646,77 @@ RUNTIME_FUNCTION(Runtime_SpreadIterablePrepare) {
     // If IsArrayIteratorLookupChainIntact(), then we know that the initial
     // ArrayIterator is being used. If the map of the prototype has changed,
     // then take the slow path.
-
     if (isolate->is_initial_array_prototype(array_proto) &&
         isolate->IsArrayIteratorLookupChainIntact() &&
         isolate->is_initial_array_iterator_prototype_map(iterator_map)) {
       if (IsFastPackedElementsKind(array_kind)) {
-        return *spread;
+        return false;
       }
       if (IsFastHoleyElementsKind(array_kind) &&
           isolate->IsFastArrayConstructorPrototypeChainIntact()) {
-        return *spread;
+        return false;
       }
     }
   }
+  return true;
+}
 
-  Handle<JSFunction> spread_iterable_function = isolate->spread_iterable();
+}  //  namespace
 
-  Handle<Object> spreaded;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, spreaded,
-      Execution::Call(isolate, spread_iterable_function,
-                      isolate->factory()->undefined_value(), 1, &spread));
+RUNTIME_FUNCTION(Runtime_SpreadIterablePrepare) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(1, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(Object, spread, 0);
+
+  // Iterate over the spread if we need to.
+  if (MustIterate(isolate, spread)) {
+    Handle<JSFunction> spread_iterable_function = isolate->spread_iterable();
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, spread,
+        Execution::Call(isolate, spread_iterable_function,
+                        isolate->factory()->undefined_value(), 1, &spread));
+  }
+
+  return *spread;
+}
+
+RUNTIME_FUNCTION(Runtime_SpreadIterablePrepareVarargs) {
+  HandleScope scope(isolate);
+  DCHECK_LE(1, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(Object, spread, args.length() - 1);
+
+  // Iterate over the spread if we need to.
+  if (MustIterate(isolate, spread)) {
+    Handle<JSFunction> spread_iterable_function = isolate->spread_iterable();
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, spread,
+        Execution::Call(isolate, spread_iterable_function,
+                        isolate->factory()->undefined_value(), 1, &spread));
+  }
+
+  if (args.length() == 1) return *spread;
+
+  JSArray* spread_array = JSArray::cast(*spread);
+  uint32_t spread_length;
+  CHECK(spread_array->length()->ToArrayIndex(&spread_length));
+
+  // Append each of the individual args to the result.
+  int result_length = args.length() - 1 + spread_length;
+  Handle<FixedArray> result = isolate->factory()->NewFixedArray(result_length);
+  for (int i = 0; i < args.length() - 1; i++) {
+    result->set(i, *args.at<Object>(i));
+  }
+
+  // Append element of the spread to the result.
+  for (uint32_t i = 0; i < spread_length; i++) {
+    LookupIterator it(isolate, spread, i);
+    Handle<Object> element = spread_array->GetDataProperty(&it);
+    result->set(args.length() - 1 + i, *element);
+  }
 
-  return *spreaded;
+  Handle<JSArray> r = isolate->factory()->NewJSArrayWithElements(
+      result, FAST_ELEMENTS, result_length);
+  return *r;
 }
 
 }  // namespace internal

src/runtime/runtime.h

@@ -56,7 +56,8 @@ namespace internal {
   F(ArraySpeciesConstructor, 1, 1)   \
   F(ArrayIncludes_Slow, 3, 1)        \
   F(ArrayIndexOf, 3, 1)              \
-  F(SpreadIterablePrepare, 1, 1)
+  F(SpreadIterablePrepare, 1, 1)     \
+  F(SpreadIterablePrepareVarargs, -1, 1)
 
 #define FOR_EACH_INTRINSIC_ATOMICS(F)           \
   F(ThrowNotIntegerSharedTypedArrayError, 1, 1) \

test/cctest/interpreter/bytecode_expectations/SuperCallAndSpread.golden

+#
+# Autogenerated by generate-bytecode-expectations.
+#
+
+---
+wrap: no
+test function name: test
+
+---
+snippet: "
+  var test;
+  (function() {
+    class A {
+      constructor(...args) { this.baseArgs = args; }
+    }
+    class B extends A {}
+    test = new B(1, 2, 3).constructor;
+  })();
+"
+frame size: 9
+parameter count: 1
+bytecode array length: 40
+bytecodes: [
+                B(CreateRestParameter),
+                B(Star), R(2),
+                B(Mov), R(closure), R(1),
+                B(Mov), R(new_target), R(0),
+                B(Ldar), R(new_target),
+  /*   93 E> */ B(StackCheck),
+  /*   93 S> */ B(CallRuntime), U16(Runtime::k_GetSuperConstructor), R(1), U8(1),
+                B(Star), R(3),
+                B(CallRuntime), U16(Runtime::kSpreadIterablePrepareVarargs), R(2), U8(1),
+                B(Star), R(7),
+                B(LdaUndefined),
+                B(Star), R(5),
+                B(Mov), R(3), R(6),
+                B(Mov), R(0), R(8),
+  /*   93 E> */ B(CallJSRuntime), U8(150), R(5), U8(4),
+  /*   93 S> */ B(Return),
+]
+constant pool: [
+]
+handlers: [
+]
+
+---
+snippet: "
+  var test;
+  (function() {
+    class A {
+      constructor(...args) { this.baseArgs = args; }
+    }
+    class B extends A {
+      constructor(...args) { super(1, ...args); }
+    }
+    test = new B(1, 2, 3).constructor;
+  })();
+"
+frame size: 10
+parameter count: 1
+bytecode array length: 80
+bytecodes: [
+                B(CreateRestParameter),
+                B(Star), R(2),
+                B(Mov), R(closure), R(1),
+                B(Mov), R(new_target), R(0),
+                B(Ldar), R(new_target),
+  /*  128 E> */ B(StackCheck),
+  /*  140 S> */ B(CallRuntime), U16(Runtime::k_GetSuperConstructor), R(1), U8(1),
+                B(Star), R(3),
+                B(LdaSmi), U8(1),
+                B(Star), R(4),
+                B(Mov), R(2), R(5),
+  /*  152 E> */ B(CallRuntime), U16(Runtime::kSpreadIterablePrepareVarargs), R(4), U8(2),
+                B(Star), R(8),
+                B(LdaUndefined),
+                B(Star), R(6),
+                B(Mov), R(3), R(7),
+                B(Mov), R(0), R(9),
+  /*  140 E> */ B(CallJSRuntime), U8(150), R(6), U8(4),
+                B(Star), R(3),
+                B(Ldar), R(this),
+                B(JumpIfNotHole), U8(4),
+                B(Jump), U8(11),
+                B(LdaConstant), U8(0),
+                B(Star), R(4),
+  /*  140 E> */ B(CallRuntime), U16(Runtime::kThrowReferenceError), R(4), U8(1),
+                B(Mov), R(3), R(this),
+                B(Ldar), R(this),
+                B(JumpIfNotHole), U8(11),
+                B(LdaConstant), U8(0),
+                B(Star), R(3),
+                B(CallRuntime), U16(Runtime::kThrowReferenceError), R(3), U8(1),
+  /*  159 S> */ B(Return),
+]
+constant pool: [
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["this"],
+]
+handlers: [
+]
+
+---
+snippet: "
+  var test;
+  (function() {
+    class A {
+      constructor(...args) { this.baseArgs = args; }
+    }
+    class B extends A {
+      constructor(...args) { super(1, ...args, 1); }
+    }
+    test = new B(1, 2, 3).constructor;
+  })();
+"
+frame size: 13
+parameter count: 1
+bytecode array length: 96
+bytecodes: [
+                B(CreateRestParameter),
+                B(Star), R(2),
+                B(Mov), R(closure), R(1),
+                B(Mov), R(new_target), R(0),
+                B(Ldar), R(new_target),
+  /*  128 E> */ B(StackCheck),
+  /*  140 S> */ B(LdaUndefined),
+                B(Star), R(3),
+  /*  140 E> */ B(CallRuntime), U16(Runtime::k_GetSuperConstructor), R(1), U8(1),
+                B(Star), R(4),
+                B(LdaUndefined),
+                B(Star), R(7),
+                B(CreateArrayLiteral), U8(0), U8(0), U8(9),
+                B(Star), R(8),
+                B(LdaUndefined),
+                B(Star), R(11),
+                B(Mov), R(2), R(12),
+  /*  152 E> */ B(CallJSRuntime), U8(154), R(11), U8(2),
+                B(Star), R(9),
+                B(CreateArrayLiteral), U8(1), U8(1), U8(9),
+                B(Star), R(10),
+                B(CallJSRuntime), U8(153), R(7), U8(4),
+                B(Star), R(5),
+                B(Mov), R(0), R(6),
+  /*  140 E> */ B(CallJSRuntime), U8(150), R(3), U8(4),
+                B(Star), R(3),
+                B(Ldar), R(this),
+                B(JumpIfNotHole), U8(4),
+                B(Jump), U8(11),
+                B(LdaConstant), U8(2),
+                B(Star), R(4),
+  /*  140 E> */ B(CallRuntime), U16(Runtime::kThrowReferenceError), R(4), U8(1),
+                B(Mov), R(3), R(this),
+                B(Ldar), R(this),
+                B(JumpIfNotHole), U8(11),
+                B(LdaConstant), U8(2),
+                B(Star), R(3),
+                B(CallRuntime), U16(Runtime::kThrowReferenceError), R(3), U8(1),
+  /*  162 S> */ B(Return),
+]
+constant pool: [
+  FIXED_ARRAY_TYPE,
+  FIXED_ARRAY_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["this"],
+]
+handlers: [
+]
+

test/cctest/interpreter/test-bytecode-generator.cc

@@ -2246,6 +2246,48 @@ TEST(Modules) {
                      LoadGolden("Modules.golden")));
 }
 
+TEST(SuperCallAndSpread) {
+  InitializedIgnitionHandleScope scope;
+  BytecodeExpectationsPrinter printer(CcTest::isolate());
+  printer.set_wrap(false);
+  printer.set_test_function_name("test");
+  const char* snippets[] = {
+      "var test;\n"
+      "(function() {\n"
+      "  class A {\n"
+      "    constructor(...args) { this.baseArgs = args; }\n"
+      "  }\n"
+      "  class B extends A {}\n"
+      "  test = new B(1, 2, 3).constructor;\n"
+      "})();\n",
+
+      "var test;\n"
+      "(function() {\n"
+      "  class A {\n"
+      "    constructor(...args) { this.baseArgs = args; }\n"
+      "  }\n"
+      "  class B extends A {\n"
+      "    constructor(...args) { super(1, ...args); }\n"
+      "  }\n"
+      "  test = new B(1, 2, 3).constructor;\n"
+      "})();\n",
+
+      "var test;\n"
+      "(function() {\n"
+      "  class A {\n"
+      "    constructor(...args) { this.baseArgs = args; }\n"
+      "  }\n"
+      "  class B extends A {\n"
+      "    constructor(...args) { super(1, ...args, 1); }\n"
+      "  }\n"
+      "  test = new B(1, 2, 3).constructor;\n"
+      "})();\n",
+  };
+
+  CHECK(CompareTexts(BuildActual(printer, snippets),
+                     LoadGolden("SuperCallAndSpread.golden")));
+}
+
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8