Add support for spread args.

Summary:
Allow the spread element in arguments using a simple naive implementation.
We add a `HermesInternal.apply` function, spread the arguments into
an array using the spread array mechanisms, and then use HermesInternal.apply
to actually run the call.

Note that because spread args can be used in constructors, we need to
convey the use of `new` to `HermesInternal.apply` with one of the arguments.

Reviewed By: tmikov

Differential Revision: D17334064

fbshipit-source-id: f358475122bac76cc1a6533ac8ccd6d62851d312

include/hermes/BCGen/HBC/BytecodeFileFormat.h

@@ -29,7 +29,7 @@ const static uint64_t DELTA_MAGIC = ~MAGIC;
 
 // Bytecode version generated by this version of the compiler.
 // Updated: Oct 11, 2019
-const static uint32_t BYTECODE_VERSION = 69;
+const static uint32_t BYTECODE_VERSION = 70;
 
 /// Property cache index which indicates no caching.
 static constexpr uint8_t PROPERTY_CACHING_DISABLED = 0;

include/hermes/Inst/Builtins.def

@@ -45,6 +45,7 @@ BUILTIN_METHOD(HermesInternal, generatorSetDelegated)
 BUILTIN_METHOD(HermesInternal, copyDataProperties)
 BUILTIN_METHOD(HermesInternal, copyRestArgs)
 BUILTIN_METHOD(HermesInternal, arraySpread)
+BUILTIN_METHOD(HermesInternal, apply)
 BUILTIN_METHOD(HermesInternal, exportAll)
 BUILTIN_METHOD(HermesInternal, exponentiationOperator)
 

include/hermes/VM/Callable.h

@@ -255,6 +255,18 @@ class Callable : public JSObject {
     return selfHandle->getVT()->call(selfHandle, runtime);
   }
 
+  /// Call the callable in contruct mode with arguments already on the stack.
+  /// Checks the return value of the called function. If it is an object, then
+  /// it is returned, else the `this` value is returned.
+  static CallResult<HermesValue>
+  construct(Handle<Callable> selfHandle, Runtime *runtime, Handle<> thisVal) {
+    auto result = call(selfHandle, runtime);
+    if (LLVM_UNLIKELY(result == ExecutionStatus::EXCEPTION)) {
+      return ExecutionStatus::EXCEPTION;
+    }
+    return result->isObject() ? result : thisVal.getHermesValue();
+  }
+
   /// Create a new object and construct the new object of the given type
   /// by invoking \p selfHandle with construct=true.
   /// \param selfHandle the Callable from which to construct the new object.
@@ -283,6 +295,16 @@ class Callable : public JSObject {
   /// creation has been delayed by lazy objects.
   static void defineLazyProperties(Handle<Callable> fn, Runtime *runtime);
 
+  /// Create an object by calling newObject on \p selfHandle.
+  /// The object can then be used as the "this" argument when calling
+  /// \p selfHandle to construct an object.
+  /// Retrieves the "prototype" property from \p selfHandle,
+  /// and calls newObject() on it if it's an object,
+  /// else calls newObject() on the built-in Object prototype object.
+  static CallResult<HermesValue> createThisForConstruct(
+      Handle<Callable> selfHandle,
+      Runtime *runtime);
+
  protected:
   Callable(
       Runtime *runtime,
@@ -307,17 +329,6 @@ class Callable : public JSObject {
       Handle<Callable> selfHandle,
       Runtime *runtime,
       Handle<JSObject> parentHandle);
-
- private:
-  /// Create an object by calling newObject on \p selfHandle.
-  /// The object can then be used as the "this" argument when calling
-  /// \p selfHandle to construct an object.
-  /// Retrieves the "prototype" property from \p selfHandle,
-  /// and calls newObject() on it if it's an object,
-  /// else calls newObject() on the built-in Object prototype object.
-  static CallResult<HermesValue> createThisForConstruct(
-      Handle<Callable> selfHandle,
-      Runtime *runtime);
 };
 
 /// A function produced by Function.prototype.bind(). It packages a function

include/hermes/VM/NativeFunctions.def

@@ -132,6 +132,7 @@ NATIVE_FUNCTION(generatorPrototypeReturnOrThrow)
 NATIVE_FUNCTION(hermesInternalCopyDataProperties)
 NATIVE_FUNCTION(hermesInternalCopyRestArgs)
 NATIVE_FUNCTION(hermesInternalArraySpread)
+NATIVE_FUNCTION(hermesInternalApply)
 NATIVE_FUNCTION(hermesInternalDetachArrayBuffer)
 NATIVE_FUNCTION(hermesInternalEnsureObject)
 NATIVE_FUNCTION(hermesInternalExportAll)

lib/AST/SemanticValidator.cpp

@@ -459,7 +459,9 @@ void SemanticValidator::visit(ArrayPatternNode *AP) {
 
 void SemanticValidator::visit(SpreadElementNode *S, Node *parent) {
   if (!isa<ESTree::ObjectExpressionNode>(parent) &&
-      !isa<ESTree::ArrayExpressionNode>(parent))
+      !isa<ESTree::ArrayExpressionNode>(parent) &&
+      !isa<ESTree::CallExpressionNode>(parent) &&
+      !isa<ESTree::NewExpressionNode>(parent))
     sm_.error(S->getSourceRange(), "spread operator is not supported");
   visitESTreeChildren(*this, S);
 }

lib/IRGen/ESTreeIRGen-expr.cpp

@@ -206,7 +206,7 @@ void ESTreeIRGen::genExpressionBranch(
   Builder.createCondBranchInst(condVal, onTrue, onFalse);
 }
 
-Value *ESTreeIRGen::genArrayExpr(ESTree::ArrayExpressionNode *Expr) {
+Value *ESTreeIRGen::genArrayFromElements(ESTree::NodeList &list) {
   LLVM_DEBUG(dbgs() << "Initializing a new array\n");
   AllocArrayInst::ArrayValueList elements;
 
@@ -215,7 +215,7 @@ Value *ESTreeIRGen::genArrayExpr(ESTree::ArrayExpressionNode *Expr) {
   // element (which will result in the final array having variable length).
   unsigned minElements = 0;
   bool variableLength = false;
-  for (auto &E : Expr->_elements) {
+  for (auto &E : list) {
     if (isa<ESTree::SpreadElementNode>(&E)) {
       variableLength = true;
       continue;
@@ -242,7 +242,7 @@ Value *ESTreeIRGen::genArrayExpr(ESTree::ArrayExpressionNode *Expr) {
   bool consecutive = true;
   auto codeGenOpts = Mod->getContext().getCodeGenerationSettings();
   AllocArrayInst *allocArrayInst = nullptr;
-  for (auto &E : Expr->_elements) {
+  for (auto &E : list) {
     Value *value{nullptr};
     bool isSpread = false;
     if (!isa<ESTree::EmptyNode>(&E)) {
@@ -310,10 +310,9 @@ Value *ESTreeIRGen::genArrayExpr(ESTree::ArrayExpressionNode *Expr) {
     assert(
         !variableLength &&
         "variable length arrays must allocate their own arrays");
-    allocArrayInst =
-        Builder.createAllocArrayInst(elements, Expr->_elements.size());
+    allocArrayInst = Builder.createAllocArrayInst(elements, list.size());
   }
-  if (count > 0 && isa<ESTree::EmptyNode>(&Expr->_elements.back())) {
+  if (count > 0 && isa<ESTree::EmptyNode>(&list.back())) {
     // Last element is an elision, VM cannot derive the length properly.
     // We have to explicitly set it.
     Builder.createStorePropertyInst(
@@ -322,6 +321,10 @@ Value *ESTreeIRGen::genArrayExpr(ESTree::ArrayExpressionNode *Expr) {
   return allocArrayInst;
 }
 
+Value *ESTreeIRGen::genArrayExpr(ESTree::ArrayExpressionNode *Expr) {
+  return genArrayFromElements(Expr->_elements);
+}
+
 Value *ESTreeIRGen::genCallExpr(ESTree::CallExpressionNode *call) {
   LLVM_DEBUG(dbgs() << "IRGen 'call' statement/expression.\n");
 
@@ -350,12 +353,29 @@ Value *ESTreeIRGen::genCallExpr(ESTree::CallExpressionNode *call) {
     callee = genExpression(call->_callee);
   }
 
-  CallInst::ArgumentList args;
+  bool hasSpread = false;
   for (auto &arg : call->_arguments) {
-    args.push_back(genExpression(&arg));
+    if (isa<ESTree::SpreadElementNode>(&arg)) {
+      hasSpread = true;
+    }
+  }
+
+  if (!hasSpread) {
+    CallInst::ArgumentList args;
+    for (auto &arg : call->_arguments) {
+      args.push_back(genExpression(&arg));
+    }
+
+    return Builder.createCallInst(callee, thisVal, args);
   }
 
-  return Builder.createCallInst(callee, thisVal, args);
+  // Otherwise, there exists a spread argument, so the number of arguments
+  // is variable.
+  // Generate IR for this by creating an array and populating it with the
+  // arguments, then calling HermesInternal.apply.
+  auto *args = genArrayFromElements(call->_arguments);
+  return genHermesInternalCall(
+      "apply", Builder.getLiteralUndefined(), {callee, args, thisVal});
 }
 
 Value *ESTreeIRGen::genCallEvalExpr(ESTree::CallExpressionNode *call) {
@@ -1246,17 +1266,32 @@ Value *ESTreeIRGen::genMetaProperty(ESTree::MetaPropertyNode *MP) {
 Value *ESTreeIRGen::genNewExpr(ESTree::NewExpressionNode *N) {
   LLVM_DEBUG(dbgs() << "IRGen 'new' statement/expression.\n");
 
-  // Implement the new operator.
-  // http://www.ecma-international.org/ecma-262/7.0/index.html#sec-new-operator
-
   Value *callee = genExpression(N->_callee);
 
-  ConstructInst::ArgumentList args;
+  bool hasSpread = false;
   for (auto &arg : N->_arguments) {
-    args.push_back(genExpression(&arg));
+    if (isa<ESTree::SpreadElementNode>(&arg)) {
+      hasSpread = true;
+    }
   }
 
-  return Builder.createConstructInst(callee, args);
+  if (!hasSpread) {
+    ConstructInst::ArgumentList args;
+    for (auto &arg : N->_arguments) {
+      args.push_back(genExpression(&arg));
+    }
+
+    return Builder.createConstructInst(callee, args);
+  }
+
+  // Otherwise, there exists a spread argument, so the number of arguments
+  // is variable.
+  // Generate IR for this by creating an array and populating it with the
+  // arguments, then calling HermesInternal.apply.
+  auto *args = genArrayFromElements(N->_arguments);
+
+  return genHermesInternalCall(
+      "apply", Builder.getLiteralUndefined(), {callee, args});
 }
 
 Value *ESTreeIRGen::genLogicalExpression(

lib/IRGen/ESTreeIRGen.h

@@ -483,6 +483,11 @@ class ESTreeIRGen {
       BasicBlock *onTrue,
       BasicBlock *onFalse);
 
+  /// Convert the \p input into an array, spreading SpreadElements
+  /// using for-or iteration semantics.
+  /// Allows sharing spread code between genArrayExpr and genCallExpr.
+  Value *genArrayFromElements(ESTree::NodeList &list);
+
   Value *genObjectExpr(ESTree::ObjectExpressionNode *Expr);
   Value *genArrayExpr(ESTree::ArrayExpressionNode *Expr);
   Value *genCallExpr(ESTree::CallExpressionNode *call);

lib/VM/JSLib/HermesInternal.cpp

@@ -793,6 +793,54 @@ hermesInternalArraySpread(void *, Runtime *runtime, NativeArgs args) {
   return nextIndex.getHermesValue();
 }
 
+/// \code
+///   HermesInternal.apply = function(fn, argArray, thisVal(opt)) {}
+/// /endcode
+/// Faster version of Function.prototype.apply which does not use its `this`
+/// argument.
+/// `argArray` must be a JSArray with no getters.
+/// Equivalent to fn.apply(thisVal, argArray) if thisVal is provided.
+/// If thisVal is not provided, equivalent to running `new fn` and passing the
+/// arguments in argArray.
+CallResult<HermesValue>
+hermesInternalApply(void *, Runtime *runtime, NativeArgs args) {
+  GCScopeMarkerRAII marker{runtime};
+
+  Handle<Callable> fn = args.dyncastArg<Callable>(0);
+  if (LLVM_UNLIKELY(!fn)) {
+    return runtime->raiseTypeErrorForValue(
+        args.getArgHandle(0), " is not a function");
+  }
+
+  Handle<JSArray> argArray = args.dyncastArg<JSArray>(1);
+  if (LLVM_UNLIKELY(!argArray)) {
+    return runtime->raiseTypeError("args must be an array");
+  }
+
+  uint32_t len = JSArray::getLength(*argArray);
+
+  bool isConstructor = args.getArgCount() == 2;
+
+  MutableHandle<> thisVal{runtime};
+  if (isConstructor) {
+    auto thisValRes = Callable::createThisForConstruct(fn, runtime);
+    if (LLVM_UNLIKELY(thisValRes == ExecutionStatus::EXCEPTION)) {
+      return ExecutionStatus::EXCEPTION;
+    }
+    thisVal = *thisValRes;
+  } else {
+    thisVal = args.getArg(2);
+  }
+
+  ScopedNativeCallFrame newFrame{
+      runtime, len, *fn, isConstructor, thisVal.getHermesValue()};
+  for (uint32_t i = 0; i < len; ++i) {
+    newFrame->getArgRef(i) = argArray->at(runtime, i);
+  }
+  return isConstructor ? Callable::construct(fn, runtime, thisVal)
+                       : Callable::call(fn, runtime);
+}
+
 #ifdef HERMESVM_PLATFORM_LOGGING
 static void logGCStats(Runtime *runtime, const char *msg) {
   // The GC stats can exceed the android logcat length limit, of
@@ -1040,6 +1088,7 @@ Handle<JSObject> createHermesInternalObject(Runtime *runtime) {
       P::copyDataProperties, hermesInternalCopyDataProperties, 3);
   defineInternMethod(P::copyRestArgs, hermesInternalCopyRestArgs, 1);
   defineInternMethod(P::arraySpread, hermesInternalArraySpread, 2);
+  defineInternMethod(P::apply, hermesInternalApply, 2);
   defineInternMethod(P::ttiReached, hermesInternalTTIReached);
   defineInternMethod(P::ttrcReached, hermesInternalTTRCReached);
   defineInternMethod(P::exportAll, hermesInternalExportAll);

test/BCGen/HBC/es6/spread-arguments.js

@@ -0,0 +1,49 @@
+// Copyright (c) Facebook, Inc. and its affiliates.
+//
+// This source code is licensed under the MIT license found in the LICENSE
+// file in the root directory of this source tree.
+//
+// RUN: %hermesc -dump-bytecode %s | %FileCheck %s --match-full-lines
+
+function foo(fn, x) {
+  fn(...x);
+  new fn(...x);
+}
+
+// CHECK-LABEL: Function<foo>(3 params, 28 registers, 2 symbols):
+// CHECK-NEXT: Offset in debug table: src 0x7, vars 0x0
+// CHECK-NEXT:     CreateEnvironment r0
+// CHECK-NEXT:     LoadParam         r1, 1
+// CHECK-NEXT:     LoadParam         r2, 2
+// CHECK-NEXT:     LoadConstZero     r3
+// CHECK-NEXT:     LoadConstUndefined r4
+// CHECK-NEXT:     StoreToEnvironment r0, 0, r1
+// CHECK-NEXT:     StoreToEnvironment r0, 1, r2
+// CHECK-NEXT:     LoadFromEnvironment r5, r0, 0
+// CHECK-NEXT:     Mov               r6, r3
+// CHECK-NEXT:     LoadFromEnvironment r7, r0, 1
+// CHECK-NEXT:     NewArray          r8, 0
+// CHECK-NEXT:     Mov               r9, r6
+// CHECK-NEXT:     Mov               r20, r8
+// CHECK-NEXT:     Mov               r19, r7
+// CHECK-NEXT:     Mov               r18, r9
+// CHECK-NEXT:     CallBuiltin       r10, "HermesInternal.arraySpread", 4
+// CHECK-NEXT:     Mov               r6, r10
+// CHECK-NEXT:     Mov               r20, r5
+// CHECK-NEXT:     Mov               r19, r8
+// CHECK-NEXT:     Mov               r18, r4
+// CHECK-NEXT:     CallBuiltin       r11, "HermesInternal.apply", 4
+// CHECK-NEXT:     LoadFromEnvironment r11, r0, 0
+// CHECK-NEXT:     Mov               r12, r3
+// CHECK-NEXT:     LoadFromEnvironment r13, r0, 1
+// CHECK-NEXT:     NewArray          r14, 0
+// CHECK-NEXT:     Mov               r15, r12
+// CHECK-NEXT:     Mov               r20, r14
+// CHECK-NEXT:     Mov               r19, r13
+// CHECK-NEXT:     Mov               r18, r15
+// CHECK-NEXT:     CallBuiltin       r16, "HermesInternal.arraySpread", 4
+// CHECK-NEXT:     Mov               r12, r16
+// CHECK-NEXT:     Mov               r20, r11
+// CHECK-NEXT:     Mov               r19, r14
+// CHECK-NEXT:     CallBuiltin       r17, "HermesInternal.apply", 3
+// CHECK-NEXT:     Ret               r4