[ES7] yield star should not return if the inner iterator.throw returns { done: true }
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Apr 2016 07:35:48 +0000 (07:35 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Apr 2016 07:35:48 +0000 (07:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156576

Reviewed by Saam Barati.

This is slight generator fix in ES7. When calling generator.throw(),
the yield-star should call the throw() of the inner generator. At that
time, when the result of throw() is { done: true}, the generator should
not stop itself.

    function * gen()
    {
        yield * (function * () {
            try {
                yield 42;
            } catch (error) { }
        }());
        // Continue executing.
        yield 42;
    }

    let g = gen();
    g.next();
    shouldBe(g.throw().value, 42);

* builtins/GeneratorPrototype.js:
(generatorResume):
(next):
(return):
(throw):
* bytecode/BytecodeIntrinsicRegistry.cpp:
(JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry):
* bytecode/BytecodeIntrinsicRegistry.h:
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitDelegateYield):
* runtime/JSGeneratorFunction.h:
* tests/stress/generator-yield-star.js:
(gen):
* tests/stress/yield-star-throw-continue.js: Added.
(shouldBe):
(generator):
(shouldThrow):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199652 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/GeneratorPrototype.js
Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp
Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/runtime/JSGeneratorFunction.h
Source/JavaScriptCore/tests/stress/generator-yield-star.js
Source/JavaScriptCore/tests/stress/yield-star-throw-continue.js [new file with mode: 0644]

index 9dbb7f3..21d656d 100644 (file)
@@ -1,3 +1,49 @@
+2016-04-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [ES7] yield star should not return if the inner iterator.throw returns { done: true }
+        https://bugs.webkit.org/show_bug.cgi?id=156576
+
+        Reviewed by Saam Barati.
+
+        This is slight generator fix in ES7. When calling generator.throw(),
+        the yield-star should call the throw() of the inner generator. At that
+        time, when the result of throw() is { done: true}, the generator should
+        not stop itself.
+
+            function * gen()
+            {
+                yield * (function * () {
+                    try {
+                        yield 42;
+                    } catch (error) { }
+                }());
+                // Continue executing.
+                yield 42;
+            }
+
+            let g = gen();
+            g.next();
+            shouldBe(g.throw().value, 42);
+
+
+        * builtins/GeneratorPrototype.js:
+        (generatorResume):
+        (next):
+        (return):
+        (throw):
+        * bytecode/BytecodeIntrinsicRegistry.cpp:
+        (JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry):
+        * bytecode/BytecodeIntrinsicRegistry.h:
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::emitDelegateYield):
+        * runtime/JSGeneratorFunction.h:
+        * tests/stress/generator-yield-star.js:
+        (gen):
+        * tests/stress/yield-star-throw-continue.js: Added.
+        (shouldBe):
+        (generator):
+        (shouldThrow):
+
 2016-04-17  Jeremy Huddleston Sequoia  <jeremyhu@apple.com>
 
         Fix incorrect assumption that APPLE implies Mac.
index 597a882..69463c3 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Yusuke Suzuki <utatane.tea@gmail.com>.
+ * Copyright (C) 2015-2016 Yusuke Suzuki <utatane.tea@gmail.com>.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -29,15 +29,6 @@ function generatorResume(generator, sentValue, resumeMode)
 {
     "use strict";
 
-    // GeneratorState.
-    const Completed = -1;
-    const Executing = -2;
-
-    // ResumeMode.
-    const NormalMode = 0;
-    const ReturnMode = 1;
-    const ThrowMode = 2;
-
     let state = generator.@generatorState;
     let done = false;
     let value = @undefined;
@@ -45,26 +36,26 @@ function generatorResume(generator, sentValue, resumeMode)
     if (typeof state !== 'number')
         throw new @TypeError("|this| should be a generator");
 
-    if (state === Executing)
+    if (state === @GeneratorStateExecuting)
         throw new @TypeError("Generator is executing");
 
-    if (state === Completed) {
-        if (resumeMode === ThrowMode)
+    if (state === @GeneratorStateCompleted) {
+        if (resumeMode === @GeneratorResumeModeThrow)
             throw sentValue;
 
         done = true;
-        if (resumeMode === ReturnMode)
+        if (resumeMode === @GeneratorResumeModeReturn)
             value = sentValue;
     } else {
         try {
-            generator.@generatorState = Executing;
+            generator.@generatorState = @GeneratorStateExecuting;
             value = generator.@generatorNext.@call(generator.@generatorThis, generator, state, sentValue, resumeMode);
-            if (generator.@generatorState === Executing) {
-                generator.@generatorState = Completed;
+            if (generator.@generatorState === @GeneratorStateExecuting) {
+                generator.@generatorState = @GeneratorStateCompleted;
                 done = true;
             }
         } catch (error) {
-            generator.@generatorState = Completed;
+            generator.@generatorState = @GeneratorStateCompleted;
             throw error;
         }
     }
@@ -75,19 +66,19 @@ function next(value)
 {
     "use strict";
 
-    return @generatorResume(this, value, /* NormalMode */ 0);
+    return @generatorResume(this, value, @GeneratorResumeModeNormal);
 }
 
 function return(value)
 {
     "use strict";
 
-    return @generatorResume(this, value, /* ReturnMode */ 1);
+    return @generatorResume(this, value, @GeneratorResumeModeReturn);
 }
 
 function throw(exception)
 {
     "use strict";
 
-    return @generatorResume(this, exception, /* ThrowMode */ 2);
+    return @generatorResume(this, exception, @GeneratorResumeModeThrow);
 }
index d5854b0..7c75879 100644 (file)
@@ -28,6 +28,7 @@
 #include "BytecodeGenerator.h"
 #include "JSArrayIterator.h"
 #include "JSCJSValueInlines.h"
+#include "JSGeneratorFunction.h"
 #include "JSPromise.h"
 #include "Nodes.h"
 #include "StrongInlines.h"
@@ -58,6 +59,11 @@ BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry(VM& vm)
     m_symbolMatch.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->matchSymbol.impl())));
     m_symbolSearch.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->searchSymbol.impl())));
     m_symbolSpecies.set(m_vm, Symbol::create(m_vm, static_cast<SymbolImpl&>(*m_vm.propertyNames->speciesSymbol.impl())));
+    m_GeneratorResumeModeNormal.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorResumeMode::NormalMode)));
+    m_GeneratorResumeModeThrow.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorResumeMode::ThrowMode)));
+    m_GeneratorResumeModeReturn.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorResumeMode::ReturnMode)));
+    m_GeneratorStateCompleted.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorState::Completed)));
+    m_GeneratorStateExecuting.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorState::Executing)));
 }
 
 BytecodeIntrinsicNode::EmitterType BytecodeIntrinsicRegistry::lookup(const Identifier& ident) const
index 12ba09d..56d02ba 100644 (file)
@@ -61,6 +61,12 @@ class Identifier;
     macro(symbolMatch) \
     macro(symbolSearch) \
     macro(symbolSpecies) \
+    macro(GeneratorResumeModeNormal) \
+    macro(GeneratorResumeModeThrow) \
+    macro(GeneratorResumeModeReturn) \
+    macro(GeneratorStateCompleted) \
+    macro(GeneratorStateExecuting) \
+
 
 class BytecodeIntrinsicRegistry {
     WTF_MAKE_NONCOPYABLE(BytecodeIntrinsicRegistry);
index 11cf73c..8410e5f 100644 (file)
@@ -4406,6 +4406,7 @@ RegisterID* BytecodeGenerator::emitDelegateYield(RegisterID* argument, Throwable
             emitLabel(loopStart.get());
             emitLoopHint();
 
+            RefPtr<Label> branchOnResult = newLabel();
             {
                 emitYieldPoint(value.get());
 
@@ -4422,9 +4423,6 @@ RegisterID* BytecodeGenerator::emitDelegateYield(RegisterID* argument, Throwable
                     // Fallthrough to ThrowMode.
                 }
 
-                RefPtr<Label> returnSequence = newLabel();
-                RefPtr<Label> returnWithIteratorResult = newLabel();
-                RefPtr<RegisterID> returnIteratorResult = newTemporary();
                 // Throw.
                 {
                     RefPtr<Label> throwMethodFound = newLabel();
@@ -4438,8 +4436,10 @@ RegisterID* BytecodeGenerator::emitDelegateYield(RegisterID* argument, Throwable
                     CallArguments throwArguments(*this, nullptr, 1);
                     emitMove(throwArguments.thisRegister(), iterator.get());
                     emitMove(throwArguments.argumentRegister(0), generatorValueRegister());
-                    emitCall(returnIteratorResult.get(), throwMethod.get(), NoExpectedFunction, throwArguments, node->divot(), node->divotStart(), node->divotEnd());
-                    emitJump(returnWithIteratorResult.get());
+                    emitCall(value.get(), throwMethod.get(), NoExpectedFunction, throwArguments, node->divot(), node->divotStart(), node->divotEnd());
+
+                    emitJumpIfTrue(emitIsObject(newTemporary(), value.get()), branchOnResult.get());
+                    emitThrowTypeError(ASCIILiteral("Iterator result interface is not an object."));
                 }
 
                 // Return.
@@ -4450,38 +4450,31 @@ RegisterID* BytecodeGenerator::emitDelegateYield(RegisterID* argument, Throwable
                     emitJumpIfFalse(emitIsUndefined(newTemporary(), returnMethod.get()), returnMethodFound.get());
 
                     emitMove(value.get(), generatorValueRegister());
+
+                    RefPtr<Label> returnSequence = newLabel();
                     emitJump(returnSequence.get());
 
                     emitLabel(returnMethodFound.get());
                     CallArguments returnArguments(*this, nullptr, 1);
                     emitMove(returnArguments.thisRegister(), iterator.get());
                     emitMove(returnArguments.argumentRegister(0), generatorValueRegister());
-                    emitCall(returnIteratorResult.get(), returnMethod.get(), NoExpectedFunction, returnArguments, node->divot(), node->divotStart(), node->divotEnd());
-
-                    // Fallthrough to returnWithIteratorResult.
-                }
+                    emitCall(value.get(), returnMethod.get(), NoExpectedFunction, returnArguments, node->divot(), node->divotStart(), node->divotEnd());
 
-                emitLabel(returnWithIteratorResult.get());
-                {
                     RefPtr<Label> returnIteratorResultIsObject = newLabel();
-                    emitJumpIfTrue(emitIsObject(newTemporary(), returnIteratorResult.get()), returnIteratorResultIsObject.get());
+                    emitJumpIfTrue(emitIsObject(newTemporary(), value.get()), returnIteratorResultIsObject.get());
                     emitThrowTypeError(ASCIILiteral("Iterator result interface is not an object."));
 
                     emitLabel(returnIteratorResultIsObject.get());
                     RefPtr<Label> returnFromGenerator = newLabel();
-                    emitJumpIfTrue(emitGetById(newTemporary(), returnIteratorResult.get(), propertyNames().done), returnFromGenerator.get());
+                    emitJumpIfTrue(emitGetById(newTemporary(), value.get(), propertyNames().done), returnFromGenerator.get());
 
-                    emitGetById(value.get(), returnIteratorResult.get(), propertyNames().value);
+                    emitGetById(value.get(), value.get(), propertyNames().value);
                     emitJump(loopStart.get());
 
                     emitLabel(returnFromGenerator.get());
-                    emitGetById(value.get(), returnIteratorResult.get(), propertyNames().value);
-
-                    // Fallthrough to returnSequence.
-                }
+                    emitGetById(value.get(), value.get(), propertyNames().value);
 
-                emitLabel(returnSequence.get());
-                {
+                    emitLabel(returnSequence.get());
                     if (isInFinallyBlock())
                         emitPopScopes(scopeRegister(), 0);
                     emitReturn(value.get());
@@ -4493,12 +4486,12 @@ RegisterID* BytecodeGenerator::emitDelegateYield(RegisterID* argument, Throwable
             }
 
             emitLabel(nextElement.get());
-            {
-                emitIteratorNextWithValue(value.get(), iterator.get(), value.get(), node);
-                emitJumpIfTrue(emitGetById(newTemporary(), value.get(), propertyNames().done), loopDone.get());
-                emitGetById(value.get(), value.get(), propertyNames().value);
-                emitJump(loopStart.get());
-            }
+            emitIteratorNextWithValue(value.get(), iterator.get(), value.get(), node);
+
+            emitLabel(branchOnResult.get());
+            emitJumpIfTrue(emitGetById(newTemporary(), value.get(), propertyNames().done), loopDone.get());
+            emitGetById(value.get(), value.get(), propertyNames().value);
+            emitJump(loopStart.get());
         }
         emitLabel(loopDone.get());
     }
index ff1072a..36f61c5 100644 (file)
@@ -50,6 +50,11 @@ public:
         ThrowMode = 2
     };
 
+    enum class GeneratorState : int32_t {
+        Completed = -1,
+        Executing = -2,
+    };
+
     const static unsigned StructureFlags = Base::StructureFlags;
 
     DECLARE_EXPORT_INFO;
index 45abefa..748a835 100644 (file)
@@ -263,14 +263,17 @@ class CallSite {
     function *gen()
     {
         let iter = new Iterator();
-        yield * iter;
+        let result = yield * iter;
+        shouldBe(result, 42);
+        yield 21;
     }
 
     let g = gen();
     shouldBe(g.next(0).value, undefined);
     shouldBe(g.next(1).value, 1);
     shouldBe(g.next(2).value, 2);
-    shouldBe(g.throw(42).value, 42);
+    shouldBe(g.throw(42).value, 21);
+    shouldBe(g.next().done, true);
     shouldThrow(() => {
         g.throw(44);
     }, `44`);
diff --git a/Source/JavaScriptCore/tests/stress/yield-star-throw-continue.js b/Source/JavaScriptCore/tests/stress/yield-star-throw-continue.js
new file mode 100644 (file)
index 0000000..46efaa6
--- /dev/null
@@ -0,0 +1,73 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error(`bad value: ${String(actual)}`);
+}
+
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error("not thrown");
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+(function () {
+    function * generator() {
+        yield * (function * () {
+            try {
+                yield "foo";
+            } catch(e) {
+                return;
+            }
+        }());
+        // OK, continue executing.
+        yield "bar";
+    }
+    var iter = generator();
+    iter.next();
+    shouldBe(iter["throw"]().value, "bar");
+}());
+
+(function () {
+    function * generator() {
+        yield * (function * () {
+            try {
+                yield "foo";
+            } catch (e) {
+                throw e;
+            }
+        }());
+        // OK, continue executing.
+        yield "bar";
+    }
+    var iter = generator();
+    iter.next();
+    shouldThrow(() => {
+        iter["throw"](new Error("NG"));
+    }, `Error: NG`);
+}());
+
+(function () {
+    function * generator() {
+        yield * (function * () {
+            try {
+                yield "foo";
+            } catch (e) {
+            }
+            yield "cocoa";
+        }());
+        // OK, continue executing.
+        yield "bar";
+    }
+    var iter = generator();
+    iter.next();
+    shouldBe(iter["throw"]().value, "cocoa");
+    shouldBe(iter.next().value, "bar");
+}());