[interpreter] Move desugaring of spread super call to bytecode generator

This patch moves the desugaring from the parser to the bytecode
generator for super calls that have a spread at a non last position.

This allows us to have the post super() call behavior, such as
initializing instance fields in one place in VisitCallSuper.

Bug: v8:7642
Change-Id: I00a693beb7078a63282359c1121b66bb62c157c8
Reviewed-on: https://chromium-review.googlesource.com/1009907
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52596}

src/interpreter/bytecode-generator.cc

@@ -2303,6 +2303,8 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
 void BytecodeGenerator::BuildArrayLiteralElementsInsertion(
     Register array, int first_spread_index, ZoneList<Expression*>* elements,
     bool skip_constants) {
+  DCHECK_LT(first_spread_index, elements->length());
+
   Register index = register_allocator()->NewRegister();
   int array_index = 0;
 
@@ -3575,6 +3577,12 @@ void BytecodeGenerator::VisitCall(Call* expr) {
 void BytecodeGenerator::VisitCallSuper(Call* expr) {
   RegisterAllocationScope register_scope(this);
   SuperCallReference* super = expr->expression()->AsSuperCallReference();
+  ZoneList<Expression*>* args = expr->arguments();
+
+  int first_spread_index = 0;
+  for (; first_spread_index < args->length(); first_spread_index++) {
+    if (args->at(first_spread_index)->IsSpread()) break;
+  }
 
   // Prepare the constructor to the super call.
   Register this_function = VisitForRegisterValue(super->this_function_var());
@@ -3583,29 +3591,52 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
       ->LoadAccumulatorWithRegister(this_function)
       .GetSuperConstructor(constructor);
 
-  ZoneList<Expression*>* args = expr->arguments();
-  RegisterList args_regs = register_allocator()->NewGrowableRegisterList();
-  VisitArguments(args, &args_regs);
-  // The new target is loaded into the accumulator from the
-  // {new.target} variable.
-  VisitForAccumulatorValue(super->new_target_var());
-  builder()->SetExpressionPosition(expr);
+  if (first_spread_index < expr->arguments()->length() - 1) {
+    // We rewrite something like
+    //    super(1, ...x, 2)
+    // to
+    //    %reflect_construct(constructor, [1, ...x, 2], new_target)
+    // That is, we implement (non-last-arg) spreads in super calls via our
+    // mechanism for spreads in array literals.
+
+    // First generate the array containing all arguments.
+    Register array = register_allocator()->NewRegister();
+    int literal_index = feedback_index(feedback_spec()->AddLiteralSlot());
+    builder()
+        ->CreateEmptyArrayLiteral(literal_index)
+        .StoreAccumulatorInRegister(array);
+    BuildArrayLiteralElementsInsertion(array, first_spread_index, args, false);
+
+    // Now pass that array to %reflect_construct.
+    RegisterList construct_args = register_allocator()->NewRegisterList(3);
+    builder()->MoveRegister(constructor, construct_args[0]);
+    builder()->MoveRegister(array, construct_args[1]);
+    VisitForRegisterValue(super->new_target_var(), construct_args[2]);
+    builder()->CallJSRuntime(Context::REFLECT_CONSTRUCT_INDEX, construct_args);
+  } else {
+    RegisterList args_regs = register_allocator()->NewGrowableRegisterList();
+    VisitArguments(args, &args_regs);
+    // The new target is loaded into the accumulator from the
+    // {new.target} variable.
+    VisitForAccumulatorValue(super->new_target_var());
+    builder()->SetExpressionPosition(expr);
 
-  int feedback_slot_index = feedback_index(feedback_spec()->AddCallICSlot());
+    int feedback_slot_index = feedback_index(feedback_spec()->AddCallICSlot());
 
-  // 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 (expr->only_last_arg_is_spread()) {
-    builder()->ConstructWithSpread(constructor, args_regs, feedback_slot_index);
-  } else {
-    // Call construct.
-    // 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.
-    builder()->Construct(constructor, args_regs, feedback_slot_index);
+    if (first_spread_index == expr->arguments()->length() - 1) {
+      builder()->ConstructWithSpread(constructor, args_regs,
+                                     feedback_slot_index);
+    } else {
+      DCHECK_EQ(first_spread_index, expr->arguments()->length());
+      // Call construct.
+      // 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.
+      builder()->Construct(constructor, args_regs, feedback_slot_index);
+    }
   }
 
   // The derived constructor has the correct bit set always, so we

src/parsing/parser.cc

@@ -3543,52 +3543,35 @@ Expression* Parser::SpreadCall(Expression* function,
                                ZoneList<Expression*>* args_list, int pos,
                                Call::PossiblyEval is_possibly_eval) {
   // Handle this case in BytecodeGenerator.
-  if (OnlyLastArgIsSpread(args_list)) {
+  if (OnlyLastArgIsSpread(args_list) || function->IsSuperCallReference()) {
     return factory()->NewCall(function, args_list, pos);
   }
 
-  if (function->IsSuperCallReference()) {
-    // Super calls
-    // $super_constructor = %_GetSuperConstructor(<this-function>)
-    // %reflect_construct($super_constructor, args, new.target)
-    ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(3, zone());
-    ZoneList<Expression*>* tmp = new (zone()) ZoneList<Expression*>(1, zone());
-    tmp->Add(function->AsSuperCallReference()->this_function_var(), zone());
-    args->Add(factory()->NewCallRuntime(Runtime::kInlineGetSuperConstructor,
-                                        tmp, pos),
-              zone());
-    args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
-    args->Add(function->AsSuperCallReference()->new_target_var(), zone());
-    return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args,
-                                     pos);
-  } else {
-    ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(3, zone());
-    if (function->IsProperty()) {
-      // Method calls
-      if (function->AsProperty()->IsSuperAccess()) {
-        Expression* home = ThisExpression(kNoSourcePosition);
-        args->Add(function, zone());
-        args->Add(home, zone());
-      } else {
-        Variable* temp = NewTemporary(ast_value_factory()->empty_string());
-        VariableProxy* obj = factory()->NewVariableProxy(temp);
-        Assignment* assign_obj = factory()->NewAssignment(
-            Token::ASSIGN, obj, function->AsProperty()->obj(),
-            kNoSourcePosition);
-        function = factory()->NewProperty(
-            assign_obj, function->AsProperty()->key(), kNoSourcePosition);
-        args->Add(function, zone());
-        obj = factory()->NewVariableProxy(temp);
-        args->Add(obj, zone());
-      }
+  ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(3, zone());
+  if (function->IsProperty()) {
+    // Method calls
+    if (function->AsProperty()->IsSuperAccess()) {
+      Expression* home = ThisExpression(kNoSourcePosition);
+      args->Add(function, zone());
+      args->Add(home, zone());
     } else {
-      // Non-method calls
+      Variable* temp = NewTemporary(ast_value_factory()->empty_string());
+      VariableProxy* obj = factory()->NewVariableProxy(temp);
+      Assignment* assign_obj = factory()->NewAssignment(
+          Token::ASSIGN, obj, function->AsProperty()->obj(), kNoSourcePosition);
+      function = factory()->NewProperty(
+          assign_obj, function->AsProperty()->key(), kNoSourcePosition);
       args->Add(function, zone());
-      args->Add(factory()->NewUndefinedLiteral(kNoSourcePosition), zone());
+      obj = factory()->NewVariableProxy(temp);
+      args->Add(obj, zone());
     }
-    args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
-    return factory()->NewCallRuntime(Context::REFLECT_APPLY_INDEX, args, pos);
+  } else {
+    // Non-method calls
+    args->Add(function, zone());
+    args->Add(factory()->NewUndefinedLiteral(kNoSourcePosition), zone());
   }
+  args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
+  return factory()->NewCallRuntime(Context::REFLECT_APPLY_INDEX, args, pos);
 }
 
 Expression* Parser::SpreadCallNew(Expression* function,

test/cctest/interpreter/bytecode_expectations/SuperCallAndSpread.golden

@@ -91,50 +91,57 @@ snippet: "
     test = new B(1, 2, 3).constructor;
   })();
 "
-frame size: 12
+frame size: 13
 parameter count: 1
-bytecode array length: 124
+bytecode array length: 137
 bytecodes: [
                 B(CreateRestParameter),
                 B(Star), R(2),
                 B(Mov), R(closure), R(1),
   /*  128 E> */ B(StackCheck),
                 B(Mov), R(2), R(3),
-  /*  140 S> */ B(CallRuntime), U16(Runtime::k_GetSuperConstructor), R(closure), U8(1),
-                B(Star), R(4),
-                B(CreateArrayLiteral), U8(0), U8(0), U8(37),
-                B(Star), R(5),
-                B(LdaConstant), U8(1),
-  /*  152 S> */ B(Star), R(6),
-                B(LdaNamedProperty), R(2), U8(2), U8(6),
-                B(Star), R(11),
-                B(CallProperty0), R(11), R(2), U8(8),
-                B(Mov), R(2), R(10),
+  /*  140 S> */ B(Ldar), R(closure),
+                B(GetSuperConstructor), R(5),
+                B(CreateEmptyArrayLiteral), U8(0),
+                B(Star), R(6),
+                B(LdaZero),
+                B(Star), R(7),
+                B(LdaSmi), I8(1),
+                B(StaKeyedProperty), R(6), R(7), U8(1),
+                B(LdaConstant), U8(0),
+  /*  152 S> */ B(Star), R(7),
+                B(LdaNamedProperty), R(2), U8(1), U8(8),
+                B(Star), R(12),
+                B(CallProperty0), R(12), R(2), U8(10),
+                B(Mov), R(2), R(11),
+                B(Mov), R(1), R(4),
                 B(JumpIfJSReceiver), U8(7),
                 B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
+                B(Star), R(10),
+                B(LdaNamedProperty), R(10), U8(2), U8(12),
                 B(Star), R(9),
-                B(LdaNamedProperty), R(9), U8(3), U8(10),
+                B(CallProperty0), R(9), R(10), U8(14),
                 B(Star), R(8),
-                B(CallProperty0), R(8), R(9), U8(12),
-                B(Star), R(7),
                 B(JumpIfJSReceiver), U8(7),
-                B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(7), U8(1),
-                B(LdaNamedProperty), R(7), U8(4), U8(14),
+                B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(8), U8(1),
+                B(LdaNamedProperty), R(8), U8(3), U8(16),
                 B(JumpIfToBooleanTrue), U8(21),
-                B(LdaNamedProperty), R(7), U8(5), U8(16),
+                B(LdaNamedProperty), R(8), U8(4), U8(18),
+                B(Star), R(8),
+                B(StaInArrayLiteral), R(6), R(7), U8(3),
+                B(Ldar), R(7),
+                B(Inc), U8(5),
                 B(Star), R(7),
-                B(StaInArrayLiteral), R(5), R(6), U8(1),
-                B(Ldar), R(6),
-                B(Inc), U8(3),
-                B(Star), R(6),
                 B(JumpLoop), U8(35), I8(0),
                 B(LdaSmi), I8(1),
-                B(StaInArrayLiteral), R(5), R(6), U8(1),
-                B(Ldar), R(6),
-                B(Inc), U8(3),
-                B(Star), R(6),
-                B(Mov), R(0), R(6),
-  /*  140 E> */ B(CallJSRuntime), U8(%reflect_construct), R(4), U8(3),
+                B(StaInArrayLiteral), R(6), R(7), U8(3),
+                B(Ldar), R(7),
+                B(Inc), U8(5),
+                B(Star), R(7),
+                B(Mov), R(5), R(8),
+                B(Mov), R(6), R(9),
+                B(Mov), R(0), R(10),
+  /*  140 E> */ B(CallJSRuntime), U8(%reflect_construct), R(8), U8(3),
                 B(Star), R(4),
                 B(Ldar), R(this),
   /*  140 E> */ B(ThrowSuperAlreadyCalledIfNotHole),
@@ -144,7 +151,6 @@ bytecodes: [
   /*  162 S> */ B(Return),
 ]
 constant pool: [
-  TUPLE2_TYPE,
   Smi [1],
   SYMBOL_TYPE,
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],

test/mjsunit/regress/regress-7642.js

+// Copyright 2018 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.
+
+// Flags: --harmony-public-fields
+
+const a = [2];
+const b = [4];
+
+let log;
+
+class C {
+  constructor(...args) {
+    log = args;
+  }
+}
+
+class D extends C {
+  field = 42;
+  constructor() { super(1) };
+}
+assertEquals(42, (new D).field);
+assertEquals([1], log);
+
+class E extends C {
+  field = 42;
+  constructor() { super(...a) };
+}
+assertEquals(42, (new E).field);
+assertEquals([2], log);
+
+class F extends C {
+  field = 42;
+  constructor() { super(1, ...a) };
+}
+assertEquals(42, (new F).field);
+assertEquals([1, 2], log);
+
+class G extends C {
+  field = 42;
+  constructor() { super(1, ...a, 3) };
+}
+assertEquals(42, (new G).field);
+assertEquals([1, 2, 3], log);
+
+class H extends C {
+  field = 42;
+  constructor() { super(1, ...a, 3, ...b) };
+}
+assertEquals(42, (new H).field);
+assertEquals([1, 2, 3, 4], log);
+
+class I extends C {
+  field = 42;
+  constructor() { super(1, ...a, 3, ...b, 5) };
+}
+assertEquals(42, (new I).field);
+assertEquals([1, 2, 3, 4, 5], log);