Bug 1115868 - Implement Generator.prototype.return. r=wingo
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 13 Jan 2015 15:02:58 +0100
changeset 223585 2ecbda2b89b03fd3f3ee748c13bd7d796213c0f7
parent 223584 f397b355946761ebc137d080166ab09642ec00f8
child 223586 86fa41494e58f9f87535d89f460a368f2185958e
push id28098
push userkwierso@gmail.com
push dateWed, 14 Jan 2015 00:52:19 +0000
treeherdermozilla-central@e978b8bc5c45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswingo
bugs1115868
milestone38.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 1115868 - Implement Generator.prototype.return. r=wingo
js/src/builtin/Generator.js
js/src/jit-test/tests/generators/return-break-continue.js
js/src/jit-test/tests/generators/return.js
js/src/jit-test/tests/generators/throw-closes.js
js/src/jit-test/tests/generators/wrappers.js
js/src/jit/JitFrames.cpp
js/src/jit/VMFunctions.cpp
js/src/tests/ecma_6/Generators/objects.js
js/src/tests/ecma_6/Generators/runtime.js
js/src/vm/GeneratorObject.cpp
js/src/vm/GeneratorObject.h
js/src/vm/Interpreter.cpp
--- a/js/src/builtin/Generator.js
+++ b/js/src/builtin/Generator.js
@@ -43,16 +43,38 @@ function StarGeneratorThrow(val) {
         return resumeGenerator(this, val, 'throw');
     } catch (e) {
         if (!StarGeneratorObjectIsClosed(this))
             GeneratorSetClosed(this);
         throw e;
     }
 }
 
+function StarGeneratorReturn(val) {
+    if (!IsSuspendedStarGenerator(this)) {
+        if (!IsObject(this) || !IsStarGeneratorObject(this))
+            return callFunction(CallStarGeneratorMethodIfWrapped, this, val, "StarGeneratorReturn");
+
+        if (StarGeneratorObjectIsClosed(this))
+            return { value: val, done: true };
+
+        if (GeneratorIsRunning(this))
+            ThrowError(JSMSG_NESTING_GENERATOR);
+    }
+
+    try {
+        var rval = { value: val, done: true };
+        return resumeGenerator(this, rval, 'close');
+    } catch (e) {
+        if (!StarGeneratorObjectIsClosed(this))
+            GeneratorSetClosed(this);
+        throw e;
+    }
+}
+
 function LegacyGeneratorNext(val) {
     if (!IsObject(this) || !IsLegacyGeneratorObject(this))
         return callFunction(CallLegacyGeneratorMethodIfWrapped, this, val, "LegacyGeneratorNext");
 
     if (LegacyGeneratorObjectIsClosed(this))
         ThrowStopIteration();
 
     if (GeneratorIsRunning(this))
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/generators/return-break-continue.js
@@ -0,0 +1,66 @@
+load(libdir + "iteration.js");
+
+// break in finally.
+function *f1() {
+    L: try {
+        yield 1;
+    } finally {
+        break L;
+    }
+    return 2;
+}
+it = f1();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(4), 2, true);
+assertIteratorDone(it);
+
+// continue in finally, followed by return.
+function *f2() {
+    do try {
+        yield 1;
+    } catch (e) {
+        assertEq(0, 1);
+    } finally {
+        continue;
+    } while (0);
+    return 2;
+}
+it = f2();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(4), 2, true);
+assertIteratorDone(it);
+
+// continue in finally, followed by yield.
+function *f3() {
+    do try {
+        yield 1;
+    } catch (e) {
+        assertEq(0, 1);
+    } finally {
+        continue;
+    } while (0);
+    yield 2;
+}
+it = f3();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(4), 2, false);
+assertIteratorDone(it);
+
+// continue in finally.
+function *f4() {
+    var i = 0;
+    while (true) {
+        try {
+            yield i++;
+        } finally {
+            if (i < 3)
+                continue;
+        }
+    }
+}
+it = f4();
+assertIteratorNext(it, 0);
+assertIteratorResult(it.return(-1), 1, false);
+assertIteratorResult(it.return(-2), 2, false);
+assertIteratorResult(it.return(-3), -3, true);
+assertIteratorDone(it);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/generators/return.js
@@ -0,0 +1,181 @@
+// |jit-test| error:done
+
+load(libdir + "iteration.js");
+
+function *f1() {
+    yield 1;
+    yield 2;
+}
+
+// Return after initial yield.
+var it = f1();
+assertIteratorResult(it.return(3), 3, true);
+assertIteratorResult(it.return(Math), Math, true);
+assertIteratorResult(it.return(), undefined, true);
+assertIteratorDone(it);
+
+// Return after other yield.
+it = f1();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(null), null, true);
+assertIteratorDone(it);
+
+// Finally blocks should run and can override the return value.
+function *f2() {
+    try {
+        yield 1;
+        yield 2;
+    } finally {
+        return 9;
+    }
+}
+it = f2();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(3), 9, true);
+assertIteratorDone(it);
+
+// Yield in finally block can override the return, but we should still
+// return the correct value after that.
+function *f3() {
+    try {
+        try {
+            yield 1;
+            yield 2;
+        } finally {
+            yield 3;
+        }
+    } finally {
+        yield 4;
+    }
+}
+it = f3();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(9), 3, false);
+assertIteratorNext(it, 4);
+assertIteratorDone(it, 9);
+assertIteratorDone(it, undefined);
+
+// Finally block can throw.
+function *f4() {
+    try {
+        yield 1;
+        yield 2;
+    } finally {
+        throw 3;
+    }
+}
+it = f4();
+assertIteratorNext(it, 1);
+assertThrowsValue(() => it.return(8), 3);
+assertIteratorDone(it);
+
+function *f5() {}
+it = f5();
+assertIteratorDone(it);
+assertIteratorResult(it.return(3), 3, true);
+assertIteratorDone(it);
+
+function *f6() {
+    try {
+        yield 1;
+        yield 2;
+    } finally {
+        try {
+            return 9;
+        } finally {
+            yield 3;
+        }
+    }
+}
+it = f6();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(5), 3, false);
+assertIteratorDone(it, 9);
+assertIteratorDone(it);
+
+// If we yield in a finally block, a second .return() can override
+// the first one.
+function *f7() {
+    try {
+        yield 1;
+        yield 2;
+    } finally {
+        try {
+            yield 3;
+        } finally {
+            yield 4;
+        }
+    }
+}
+it = f7();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(5), 3, false);
+assertIteratorResult(it.return(6), 4, false);
+assertIteratorDone(it, 6);
+assertIteratorDone(it);
+
+// If we yield in a finally block, .throw() should work.
+function *f8() {
+    try {
+        yield 1;
+        yield 2;
+    } finally {
+        yield 3;
+    }
+}
+it = f8();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(5), 3, false);
+assertThrowsValue(() => it.throw(4), 4);
+assertIteratorDone(it);
+
+// If the generator is already running, we should throw a TypeError.
+function *f9() {
+    try {
+        yield 1;
+        yield 2;
+    } finally {
+        it.return(4);
+        yield 3;
+    }
+}
+it = f9();
+assertIteratorNext(it, 1);
+assertThrowsInstanceOf(() => it.return(5), TypeError);
+assertIteratorDone(it);
+assertIteratorDone(it);
+
+// Second return overrides first one and closes the generator.
+function *f10() {
+    try {
+        yield 1;
+    } finally {
+        yield 2;
+    }
+}
+it = f10();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(-1), 2, false);
+assertIteratorResult(it.return(-2), -2, true);
+assertIteratorDone(it);
+
+function *f11() {
+    try {
+        try {
+            yield 1;
+        } finally {
+            throw 2;
+        }
+    } catch(e) {
+        yield e;
+    } finally {
+        yield 3;
+    }
+}
+it = f11();
+assertIteratorNext(it, 1);
+assertIteratorResult(it.return(9), 2, false);
+assertIteratorNext(it, 3);
+assertIteratorDone(it);
+
+throw "done";
--- a/js/src/jit-test/tests/generators/throw-closes.js
+++ b/js/src/jit-test/tests/generators/throw-closes.js
@@ -22,16 +22,30 @@ function *h() {
     yield 1;
     yield 2;
 }
 var i = h();
 assertIteratorNext(i, 1);
 assertThrowsValue(() => i.throw(4), 4);
 assertIteratorDone(i);
 
+// Star generator, return() throws.
+function *h2() {
+    try {
+	yield 1;
+	yield 2;
+    } finally {
+	throw 6;
+    }
+}
+var i = h2();
+assertIteratorNext(i, 1);
+assertThrowsValue(() => i.return(4), 6);
+assertIteratorDone(i);
+
 // Legacy generator, throw() throws.
 function l1() {
     yield 1;
     yield 2;
 }
 var i = l1();
 assertEq(i.next(), 1);
 assertThrowsValue(() => i.throw(5), 5);
--- a/js/src/jit-test/tests/generators/wrappers.js
+++ b/js/src/jit-test/tests/generators/wrappers.js
@@ -20,15 +20,18 @@ it = gen3();
 g.eval("function *gen4() { yield 5; yield 6; }; var it4 = gen4();");
 
 // StarGenerator.next
 assertIteratorResult(it.next.call(g.it4), 5, false)
 
 // StarGenerator.throw
 assertThrowsValue(() => it.throw.call(g.it4, 8), 8);
 
+// StarGenerator.return
+assertIteratorResult(it.return.call(g.it4, 8), 8, true);
+
 // Other objects should throw.
 try {
     it.next.call([]);
     assertEq(0, 1);
 } catch (e) {
     assertEq(e.toString().contains("called on incompatible Array"), true);
 }
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -501,17 +501,17 @@ HandleClosingGeneratorReturn(JSContext *
         return;
     RootedValue exception(cx);
     if (!cx->getPendingException(&exception))
         return;
     if (!exception.isMagic(JS_GENERATOR_CLOSING))
         return;
 
     cx->clearPendingException();
-    frame.baselineFrame()->setReturnValue(UndefinedValue());
+    SetReturnValueForClosingGenerator(cx, frame.baselineFrame());
 
     if (unwoundScopeToPc) {
         if (frame.baselineFrame()->isDebuggee())
             frame.baselineFrame()->setOverridePc(unwoundScopeToPc);
         pc = unwoundScopeToPc;
     }
 
     ForcedReturn(cx, frame, pc, rfe, calledDebugEpilogue);
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -923,17 +923,17 @@ GeneratorThrowOrClose(JSContext *cx, Bas
     // Set the frame's pc to the current resume pc, so that frame iterators
     // work. This function always returns false, so we're guaranteed to enter
     // the exception handler where we will clear the pc.
     JSScript *script = frame->script();
     uint32_t offset = script->yieldOffsets()[genObj->yieldIndex()];
     frame->setOverridePc(script->offsetToPC(offset));
 
     MOZ_ALWAYS_TRUE(DebugAfterYield(cx, frame));
-    MOZ_ALWAYS_FALSE(js::GeneratorThrowOrClose(cx, genObj, arg, resumeKind));
+    MOZ_ALWAYS_FALSE(js::GeneratorThrowOrClose(cx, frame, genObj, arg, resumeKind));
     return false;
 }
 
 bool
 StrictEvalPrologue(JSContext *cx, BaselineFrame *frame)
 {
     return frame->strictEvalPrologue(cx);
 }
--- a/js/src/tests/ecma_6/Generators/objects.js
+++ b/js/src/tests/ecma_6/Generators/objects.js
@@ -27,19 +27,24 @@ function TestGeneratorObject() {
 TestGeneratorObject();
 
 
 // Test the methods of generator objects.
 function TestGeneratorObjectMethods() {
   function* g() { yield 1; }
   var iter = g();
 
+  assertEq(iter.next.length, 1);
+  assertEq(iter.return.length, 1);
+  assertEq(iter.throw.length, 1);
+
   function TestNonGenerator(non_generator) {
     assertThrowsInstanceOf(function() { iter.next.call(non_generator); }, TypeError);
     assertThrowsInstanceOf(function() { iter.next.call(non_generator, 1); }, TypeError);
+    assertThrowsInstanceOf(function() { iter.return.call(non_generator, 1); }, TypeError);
     assertThrowsInstanceOf(function() { iter.throw.call(non_generator, 1); }, TypeError);
     assertThrowsInstanceOf(function() { iter.close.call(non_generator); }, TypeError);
   }
 
   TestNonGenerator(1);
   TestNonGenerator({});
   TestNonGenerator(function(){});
   TestNonGenerator(g);
--- a/js/src/tests/ecma_6/Generators/runtime.js
+++ b/js/src/tests/ecma_6/Generators/runtime.js
@@ -59,17 +59,17 @@ TestGeneratorFunctionPrototype();
 // Functions that we associate with generator objects are actually defined by
 // a common prototype.
 function TestGeneratorObjectPrototype() {
     assertEq(Object.getPrototypeOf(GeneratorObjectPrototype),
                Object.prototype);
     assertEq(Object.getPrototypeOf((function*(){yield 1}).prototype),
                GeneratorObjectPrototype);
 
-    var expected_property_names = ["next", "throw", "constructor"];
+    var expected_property_names = ["next", "return", "throw", "constructor"];
     if (!JS_HAS_SYMBOLS)
         expected_property_names.push(std_iterator);
     var found_property_names =
         Object.getOwnPropertyNames(GeneratorObjectPrototype);
 
     expected_property_names.sort();
     found_property_names.sort();
 
--- a/js/src/vm/GeneratorObject.cpp
+++ b/js/src/vm/GeneratorObject.cpp
@@ -62,18 +62,17 @@ bool
 GeneratorObject::suspend(JSContext *cx, HandleObject obj, AbstractFramePtr frame, jsbytecode *pc,
                          Value *vp, unsigned nvalues)
 {
     MOZ_ASSERT(*pc == JSOP_INITIALYIELD || *pc == JSOP_YIELD);
 
     Rooted<GeneratorObject*> genObj(cx, &obj->as<GeneratorObject>());
     MOZ_ASSERT(!genObj->hasExpressionStack());
 
-    if (*pc == JSOP_YIELD && genObj->isClosing()) {
-        MOZ_ASSERT(genObj->is<LegacyGeneratorObject>());
+    if (*pc == JSOP_YIELD && genObj->isClosing() && genObj->is<LegacyGeneratorObject>()) {
         RootedValue val(cx, ObjectValue(*frame.callee()));
         js_ReportValueError(cx, JSMSG_BAD_GENERATOR_YIELD, JSDVG_IGNORE_STACK, val, NullPtr());
         return false;
     }
 
     uint32_t yieldIndex = GET_UINT24(pc);
     genObj->setYieldIndex(yieldIndex);
     genObj->setScopeChain(*frame.scopeChain());
@@ -90,35 +89,70 @@ GeneratorObject::suspend(JSContext *cx, 
 
 bool
 GeneratorObject::finalSuspend(JSContext *cx, HandleObject obj)
 {
     Rooted<GeneratorObject*> genObj(cx, &obj->as<GeneratorObject>());
     MOZ_ASSERT(genObj->isRunning() || genObj->isClosing());
 
     bool closing = genObj->isClosing();
-    MOZ_ASSERT_IF(closing, genObj->is<LegacyGeneratorObject>());
     genObj->setClosed();
 
     if (genObj->is<LegacyGeneratorObject>() && !closing)
         return ThrowStopIteration(cx);
 
     return true;
 }
 
+void
+js::SetReturnValueForClosingGenerator(JSContext *cx, AbstractFramePtr frame)
+{
+    CallObject &callObj = frame.callObj();
+
+    // Get the generator object stored on the scope chain and close it.
+    Shape *shape = callObj.lookup(cx, cx->names().dotGenerator);
+    GeneratorObject &genObj = callObj.getSlot(shape->slot()).toObject().as<GeneratorObject>();
+    genObj.setClosed();
+
+    Value v;
+    if (genObj.is<StarGeneratorObject>()) {
+        // The return value is stored in the .genrval slot.
+        shape = callObj.lookup(cx, cx->names().dotGenRVal);
+        v = callObj.getSlot(shape->slot());
+    } else {
+        // Legacy generator .close() always returns |undefined|.
+        MOZ_ASSERT(genObj.is<LegacyGeneratorObject>());
+        v = UndefinedValue();
+    }
+
+    frame.setReturnValue(v);
+}
+
 bool
-js::GeneratorThrowOrClose(JSContext *cx, Handle<GeneratorObject*> genObj, HandleValue arg,
-                          uint32_t resumeKind)
+js::GeneratorThrowOrClose(JSContext *cx, AbstractFramePtr frame, Handle<GeneratorObject*> genObj,
+                          HandleValue arg, uint32_t resumeKind)
 {
     if (resumeKind == GeneratorObject::THROW) {
         cx->setPendingException(arg);
         genObj->setRunning();
     } else {
         MOZ_ASSERT(resumeKind == GeneratorObject::CLOSE);
-        MOZ_ASSERT(genObj->is<LegacyGeneratorObject>());
+
+        if (genObj->is<StarGeneratorObject>()) {
+            // Store the return value in the frame's CallObject so that we can
+            // return it after executing finally blocks (and potentially
+            // yielding again).
+            MOZ_ASSERT(arg.isObject());
+            CallObject &callObj = frame.callObj();
+            Shape *shape = callObj.lookup(cx, cx->names().dotGenRVal);
+            callObj.setSlot(shape->slot(), arg);
+        } else {
+            MOZ_ASSERT(arg.isUndefined());
+        }
+
         cx->setPendingException(MagicValue(JS_GENERATOR_CLOSING));
         genObj->setClosing();
     }
     return false;
 }
 
 bool
 GeneratorObject::resume(JSContext *cx, InterpreterActivation &activation,
@@ -134,18 +168,18 @@ GeneratorObject::resume(JSContext *cx, I
         return false;
 
     if (genObj->hasArgsObj())
         activation.regs().fp()->initArgsObj(genObj->argsObj());
 
     if (genObj->hasExpressionStack()) {
         uint32_t len = genObj->expressionStack().length();
         MOZ_ASSERT(activation.regs().spForStackDepth(len));
-        RootedObject array(cx, &genObj->expressionStack());
-        GetElements(cx, array, len, activation.regs().sp);
+        const Value *src = genObj->expressionStack().getDenseElements();
+        mozilla::PodCopy(activation.regs().sp, src, len);
         activation.regs().sp += len;
         genObj->clearExpressionStack();
     }
 
     JSScript *script = callee->nonLazyScript();
     uint32_t offset = script->yieldOffsets()[genObj->yieldIndex()];
     activation.regs().pc = script->offsetToPC(offset);
 
@@ -158,17 +192,17 @@ GeneratorObject::resume(JSContext *cx, I
 
     switch (resumeKind) {
       case NEXT:
         genObj->setRunning();
         return true;
 
       case THROW:
       case CLOSE:
-        return GeneratorThrowOrClose(cx, genObj, arg, resumeKind);
+        return GeneratorThrowOrClose(cx, activation.regs().fp(), genObj, arg, resumeKind);
 
       default:
         MOZ_CRASH("bad resumeKind");
     }
 }
 
 bool
 LegacyGeneratorObject::close(JSContext *cx, HandleObject obj)
@@ -209,16 +243,17 @@ const Class StarGeneratorObject::class_ 
     "Generator",
     JSCLASS_HAS_RESERVED_SLOTS(GeneratorObject::RESERVED_SLOTS)
 };
 
 static const JSFunctionSpec star_generator_methods[] = {
     JS_SELF_HOSTED_SYM_FN(iterator, "IteratorIdentity", 0, 0),
     JS_SELF_HOSTED_FN("next", "StarGeneratorNext", 1, 0),
     JS_SELF_HOSTED_FN("throw", "StarGeneratorThrow", 1, 0),
+    JS_SELF_HOSTED_FN("return", "StarGeneratorReturn", 1, 0),
     JS_FS_END
 };
 
 #define JSPROP_ROPERM   (JSPROP_READONLY | JSPROP_PERMANENT)
 
 static const JSFunctionSpec legacy_generator_methods[] = {
     JS_SELF_HOSTED_SYM_FN(iterator, "LegacyGeneratorIteratorShim", 0, 0),
     // "send" is an alias for "next".
--- a/js/src/vm/GeneratorObject.h
+++ b/js/src/vm/GeneratorObject.h
@@ -148,17 +148,17 @@ class GeneratorObject : public NativeObj
         setFixedSlot(YIELD_INDEX_SLOT, Int32Value(YIELD_INDEX_RUNNING));
     }
     void setClosing() {
         MOZ_ASSERT(isSuspended());
         setFixedSlot(YIELD_INDEX_SLOT, Int32Value(YIELD_INDEX_CLOSING));
     }
     void setYieldIndex(uint32_t yieldIndex) {
         MOZ_ASSERT_IF(yieldIndex == 0, getFixedSlot(YIELD_INDEX_SLOT).isUndefined());
-        MOZ_ASSERT_IF(yieldIndex != 0, isRunning());
+        MOZ_ASSERT_IF(yieldIndex != 0, isRunning() || isClosing());
         MOZ_ASSERT(yieldIndex < uint32_t(YIELD_INDEX_CLOSING));
         setFixedSlot(YIELD_INDEX_SLOT, Int32Value(yieldIndex));
         MOZ_ASSERT(isSuspended());
     }
     uint32_t yieldIndex() const {
         MOZ_ASSERT(isSuspended());
         return getFixedSlot(YIELD_INDEX_SLOT).toInt32();
     }
@@ -203,18 +203,19 @@ class LegacyGeneratorObject : public Gen
 };
 
 class StarGeneratorObject : public GeneratorObject
 {
   public:
     static const Class class_;
 };
 
-bool GeneratorThrowOrClose(JSContext *cx, Handle<GeneratorObject*> obj, HandleValue val,
-                           uint32_t resumeKind);
+bool GeneratorThrowOrClose(JSContext *cx, AbstractFramePtr frame, Handle<GeneratorObject*> obj,
+                           HandleValue val, uint32_t resumeKind);
+void SetReturnValueForClosingGenerator(JSContext *cx, AbstractFramePtr frame);
 
 } // namespace js
 
 template<>
 inline bool
 JSObject::is<js::GeneratorObject>() const
 {
     return is<js::LegacyGeneratorObject>() || is<js::StarGeneratorObject>();
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1116,17 +1116,17 @@ HandleError(JSContext *cx, InterpreterRe
         if (cx->isExceptionPending()) {
             RootedValue exception(cx);
             if (!cx->getPendingException(&exception))
                 return ErrorReturnContinuation;
 
             if (exception.isMagic(JS_GENERATOR_CLOSING)) {
                 cx->clearPendingException();
                 ok = true;
-                regs.fp()->clearReturnValue();
+                SetReturnValueForClosingGenerator(cx, regs.fp());
             }
         }
     } else {
         // We may be propagating a forced return from the interrupt
         // callback, which cannot easily force a return.
         if (MOZ_UNLIKELY(cx->isPropagatingForcedReturn())) {
             cx->clearPropagatingForcedReturn();
             ForcedReturn(cx, si, regs);