Bug 1015742 - refactor JSOP_SPREAD to work on iterator; r=jorendorff
authorArpad Borsos <arpad.borsos@googlemail.com>
Mon, 26 May 2014 19:41:37 +0200
changeset 206934 310d82551d3b537906f4a74c383c45a92f510881
parent 206933 b144d655a179191d81ea678b0ecb7db2d721fbc3
child 206935 cf93e150098beb52791b539b8d42347da6f8a3c2
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1015742
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1015742 - refactor JSOP_SPREAD to work on iterator; r=jorendorff
js/src/frontend/BytecodeEmitter.cpp
js/src/jit-test/tests/basic/spread-array.js
js/src/jit-test/tests/basic/spread-call-eval.js
js/src/jit-test/tests/basic/spread-call-funapply.js
js/src/jit-test/tests/basic/spread-call.js
js/src/jit/BaselineCompiler.cpp
js/src/jsapi.h
js/src/jsiter.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Opcodes.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4376,16 +4376,36 @@ EmitWith(ExclusiveContext *cx, BytecodeE
         return false;
     if (!EmitTree(cx, bce, pn->pn_right))
         return false;
     if (!LeaveNestedScope(cx, bce, &stmtInfo))
         return false;
     return true;
 }
 
+/**
+ * EmitIterator expects the iterable to already be on the stack.
+ * It will replace that stack value with the corresponding iterator
+ */
+static bool
+EmitIterator(ExclusiveContext *cx, BytecodeEmitter *bce)
+{
+    // Convert iterable to iterator.
+    if (Emit1(cx, bce, JSOP_DUP) < 0)                          // OBJ OBJ
+        return false;
+    if (!EmitAtomOp(cx, cx->names().std_iterator, JSOP_CALLPROP, bce)) // OBJ @@ITERATOR
+        return false;
+    if (Emit1(cx, bce, JSOP_SWAP) < 0)                         // @@ITERATOR OBJ
+        return false;
+    if (EmitCall(cx, bce, JSOP_CALL, 0) < 0)                   // ITER
+        return false;
+    CheckTypeSet(cx, bce, JSOP_CALL);
+    return true;
+}
+
 static bool
 EmitForOf(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn, ptrdiff_t top)
 {
     ParseNode *forHead = pn->pn_left;
     ParseNode *forBody = pn->pn_right;
 
     ParseNode *pn1 = forHead->pn_kid1;
     bool letDecl = pn1 && pn1->isKind(PNK_LEXICALSCOPE);
@@ -4404,26 +4424,18 @@ EmitForOf(ExclusiveContext *cx, Bytecode
 
     // For-of loops run with two values on the stack: the iterator and the
     // current result object.
 
     // Compile the object expression to the right of 'of'.
     if (!EmitTree(cx, bce, forHead->pn_kid3))
         return false;
 
-    // Convert iterable to iterator.
-    if (Emit1(cx, bce, JSOP_DUP) < 0)                          // OBJ OBJ
-        return false;
-    if (!EmitAtomOp(cx, cx->names().std_iterator, JSOP_CALLPROP, bce)) // OBJ @@ITERATOR
-        return false;
-    if (Emit1(cx, bce, JSOP_SWAP) < 0)                         // @@ITERATOR OBJ
-        return false;
-    if (EmitCall(cx, bce, JSOP_CALL, 0) < 0)                   // ITER
-        return false;
-    CheckTypeSet(cx, bce, JSOP_CALL);
+    if (!EmitIterator(cx, bce))
+        return false;
 
     // Push a dummy result so that we properly enter iteration midstream.
     if (Emit1(cx, bce, JSOP_UNDEFINED) < 0)                    // ITER RESULT
         return false;
 
     // Enter the block before the loop body, after evaluating the obj.
     StmtInfoBCE letStmt(cx);
     if (letDecl) {
@@ -6080,16 +6092,18 @@ EmitArray(ExclusiveContext *cx, Bytecode
             if (Emit1(cx, bce, JSOP_HOLE) < 0)
                 return false;
         } else {
             ParseNode *expr = pn2->isKind(PNK_SPREAD) ? pn2->pn_kid : pn2;
             if (!EmitTree(cx, bce, expr))
                 return false;
         }
         if (pn2->isKind(PNK_SPREAD)) {
+            if (!EmitIterator(cx, bce))
+                return false;
             if (Emit1(cx, bce, JSOP_SPREAD) < 0)
                 return false;
         } else if (afterSpread) {
             if (Emit1(cx, bce, JSOP_INITELEM_INC) < 0)
                 return false;
         } else {
             off = EmitN(cx, bce, JSOP_INITELEM_ARRAY, 3);
             if (off < 0)
--- a/js/src/jit-test/tests/basic/spread-array.js
+++ b/js/src/jit-test/tests/basic/spread-array.js
@@ -38,16 +38,13 @@ function* gen() {
     for (let i = 1; i < 4; i ++)
         yield i;
 }
 assertEqArray([...gen()], [1, 2, 3]);
 
 let a, b = [1, 2, 3];
 assertEqArray([...a=b], [1, 2, 3]);
 
-// According to the draft spec, null and undefined are to be treated as empty
-// arrays. However, they are not iterable. If the spec is not changed to be in
-// terms of iterables, these tests should be fixed.
-//assertEqArray([1, ...null, 2], [1, 2]);
-//assertEqArray([1, ...undefined, 2], [1, 2]);
+// 12.2.4.1.2 Runtime Semantics: ArrayAccumulation
+// If Type(spreadObj) is not Object, then throw a TypeError exception.
 assertThrowsInstanceOf(() => [...null], TypeError);
 assertThrowsInstanceOf(() => [...undefined], TypeError);
 
--- a/js/src/jit-test/tests/basic/spread-call-eval.js
+++ b/js/src/jit-test/tests/basic/spread-call-eval.js
@@ -43,15 +43,12 @@ assertEq(eval(...itr), 11);
 function* gen() {
     yield "a + b";
 }
 assertEq(eval(...gen()), 11);
 
 let c = ["C"], d = "D";
 assertEq(eval(...c=["c[0] + d"]), "c[0] + dD");
 
-// According to the draft spec, null and undefined are to be treated as empty
-// arrays. However, they are not iterable. If the spec is not changed to be in
-// terms of iterables, these tests should be fixed.
-//assertEq(eval("a + b", ...null), 11);
-//assertEq(eval("a + b", ...undefined), 11);
+// 12.2.4.1.2 Runtime Semantics: ArrayAccumulation
+// If Type(spreadObj) is not Object, then throw a TypeError exception.
 assertThrowsInstanceOf(() => eval("a + b", ...null), TypeError);
 assertThrowsInstanceOf(() => eval("a + b", ...undefined), TypeError);
--- a/js/src/jit-test/tests/basic/spread-call-funapply.js
+++ b/js/src/jit-test/tests/basic/spread-call-funapply.js
@@ -31,21 +31,18 @@ function checkCommon(f) {
       yield null;
       yield [1, 2, 3];
   }
   assertEqArray(f.apply(...gen()), [1, 2, 3]);
 
   let a;
   assertEqArray(f.apply(null, ...a=[[1, 2, 3]]), [1, 2, 3]);
 
-  // According to the draft spec, null and undefined are to be treated as empty
-  // arrays. However, they are not iterable. If the spec is not changed to be in
-  // terms of iterables, these tests should be fixed.
-  //assertEqArray(f.apply(null, ...null, [1, 2, 3]), [1, 2, 3]);
-  //assertEqArray(f.apply(null, ...undefined, [1, 2, 3]), [1, 2, 3]);
+  // 12.2.4.1.2 Runtime Semantics: ArrayAccumulation
+  // If Type(spreadObj) is not Object, then throw a TypeError exception.
   assertThrowsInstanceOf(() => f.apply(null, ...null, [1, 2, 3]), TypeError);
   assertThrowsInstanceOf(() => f.apply(null, ...undefined, [1, 2, 3]), TypeError);
 }
 
 function checkNormal(f) {
   checkCommon(f);
 
   assertEqArray(f.apply(null, ...[[]]), [undefined, undefined, undefined]);
--- a/js/src/jit-test/tests/basic/spread-call.js
+++ b/js/src/jit-test/tests/basic/spread-call.js
@@ -35,21 +35,18 @@ function checkCommon(f, makeFn) {
   function gen() {
       for (let i = 1; i < 4; i ++)
           yield i;
   }
   assertEqArray(makeFn("...arg")(f, gen()), [1, 2, 3]);
 
   assertEqArray(makeFn("...arg=[1, 2, 3]")(f), [1, 2, 3]);
 
-  // According to the draft spec, null and undefined are to be treated as empty
-  // arrays. However, they are not iterable. If the spec is not changed to be in
-  // terms of iterables, these tests should be fixed.
-  //assertEqArray(makeFn(1, ...null, 2, 3)(f), [1, 2, 3]);
-  //assertEqArray(makeFn(1, ...undefined, 2, 3)(f), [1, 2, 3]);
+  // 12.2.4.1.2 Runtime Semantics: ArrayAccumulation
+  // If Type(spreadObj) is not Object, then throw a TypeError exception.
   assertThrowsInstanceOf(makeFn("1, ...null, 2, 3"), TypeError);
   assertThrowsInstanceOf(makeFn("1, ...undefined, 2, 3"), TypeError);
 }
 
 function checkNormal(f, makeFn) {
   checkCommon(f, makeFn);
 
   assertEqArray(makeFn("...[]")(f), [undefined, undefined, undefined]);
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -2372,17 +2372,17 @@ BaselineCompiler::emit_JSOP_INITELEM_INC
 
 typedef bool (*SpreadFn)(JSContext *, HandleObject, HandleValue,
                          HandleValue, MutableHandleValue);
 static const VMFunction SpreadInfo = FunctionInfo<SpreadFn>(js::SpreadOperation);
 
 bool
 BaselineCompiler::emit_JSOP_SPREAD()
 {
-    // Load index and iterable in R0 and R1, but keep values on the stack for
+    // Load index and iterator in R0 and R1, but keep values on the stack for
     // the decompiler.
     frame.syncStack(0);
     masm.loadValue(frame.addressOfStackValue(frame.peek(-2)), R0);
     masm.loadValue(frame.addressOfStackValue(frame.peek(-1)), R1);
 
     prepareVMCall();
 
     pushArg(R1);
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -5019,16 +5019,23 @@ class MOZ_STACK_CLASS JS_PUBLIC_API(ForO
      * does not have a callable @@iterator init() will just return true instead
      * of throwing.  Callers should then check valueIsIterable() before
      * continuing with the iteration.
      */
     bool init(JS::HandleValue iterable,
               NonIterableBehavior nonIterableBehavior = ThrowOnNonIterable);
 
     /*
+     * This method assumes that |iterator| is already an iterator.  It will not
+     * check for, and call @@iterator.  Callers should make sure that the passed
+     * in value is in fact an iterator.
+     */
+    bool initWithIterator(JS::HandleValue aIterator);
+
+    /*
      * Get the next value from the iterator.  If false *done is true
      * after this call, do not examine val.
      */
     bool next(JS::MutableHandleValue val, bool *done);
 
     /*
      * If initialized with throwOnNonCallable = false, check whether
      * the value is iterable.
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -1345,16 +1345,24 @@ ForOfIterator::init(HandleValue iterable
 
     iterator = ToObject(cx, args.rval());
     if (!iterator)
         return false;
 
     return true;
 }
 
+bool
+ForOfIterator::initWithIterator(HandleValue aIterator)
+{
+    JSContext *cx = cx_;
+    RootedObject iteratorObj(cx, ToObject(cx, aIterator));
+    return iterator = iteratorObj;
+}
+
 inline bool
 ForOfIterator::nextFromOptimizedArray(MutableHandleValue vp, bool *done)
 {
     JS_ASSERT(index != NOT_ARRAY);
 
     if (!CheckForInterrupt(cx_))
         return false;
 
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -3169,20 +3169,20 @@ CASE(JSOP_INITELEM_INC)
 }
 END_CASE(JSOP_INITELEM_INC)
 
 CASE(JSOP_SPREAD)
 {
     HandleValue countVal = REGS.stackHandleAt(-2);
     RootedObject &arr = rootObject0;
     arr = &REGS.sp[-3].toObject();
-    HandleValue iterable = REGS.stackHandleAt(-1);
+    HandleValue iterator = REGS.stackHandleAt(-1);
     MutableHandleValue resultCountVal = REGS.stackHandleAt(-2);
 
-    if (!SpreadOperation(cx, arr, countVal, iterable, resultCountVal))
+    if (!SpreadOperation(cx, arr, countVal, iterator, resultCountVal))
         goto error;
 
     REGS.sp--;
 }
 END_CASE(JSOP_SPREAD)
 
 CASE(JSOP_GOSUB)
 {
@@ -3886,22 +3886,22 @@ js::InitGetterSetterOperation(JSContext 
     if (!ValueToId<CanGC>(cx, idval, &id))
         return false;
 
     return InitGetterSetterOperation(cx, pc, obj, id, val);
 }
 
 bool
 js::SpreadOperation(JSContext *cx, HandleObject arr, HandleValue countVal,
-                    HandleValue iterable, MutableHandleValue resultCountVal)
+                    HandleValue iterator, MutableHandleValue resultCountVal)
 {
     int32_t count = countVal.toInt32();
     ForOfIterator iter(cx);
-    RootedValue iterVal(cx, iterable);
-    if (!iter.init(iterVal))
+    RootedValue iterVal(cx, iterator);
+    if (!iter.initWithIterator(iterVal))
         return false;
     while (true) {
         bool done;
         if (!iter.next(&iterVal, &done))
             return false;
         if (done)
             break;
         if (count == INT32_MAX) {
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -686,27 +686,27 @@ 1234567890123456789012345678901234567890
      *   Category: Statements
      *   Type: Function
      *   Operands: uint16_t argc
      *   Stack: callee, this, args[0], ..., args[argc-1] => rval
      *   nuses: (argc+2)
      */ \
     macro(JSOP_NEW,       82, js_new_str,   NULL,         3, -1,  1,  JOF_UINT16|JOF_INVOKE|JOF_TYPESET) \
     /*
-     * Pops the top three values on the stack as 'iterable', 'index' and 'obj',
-     * iterates over 'iterable' and stores the iteration values as 'index + i'
+     * Pops the top three values on the stack as 'iterator', 'index' and 'obj',
+     * iterates over 'iterator' and stores the iteration values as 'index + i'
      * elements of 'obj', pushes 'obj' and 'index + iteration count' onto the
      * stack.
      *
      * This opcode is used in Array literals with spread and spreadcall
-     * arguments.
+     * arguments as well as in destructing assignment with rest element.
      *   Category: Literals
      *   Type: Array
      *   Operands:
-     *   Stack: obj, index, iterable => obj, (index + iteration count)
+     *   Stack: obj, index, iterator => obj, (index + iteration count)
      */ \
     macro(JSOP_SPREAD,    83, "spread",     NULL,         1,  3,  2,  JOF_BYTE|JOF_ELEM|JOF_SET) \
     \
     /*
      * Fast get op for function arguments and local variables.
      *
      * Pushes 'arguments[argno]' onto the stack.
      *   Category: Variables and Scopes