Closed generator returns a completed object instead of throwing a error

From ES6 rev20 draft, closed generator returns completed object (the
value is `undefined` and done is `true`).
Since a error thrown in generator is propagated to the caller without
setting status of a thrown generator to "completed", once a generator is
suspended by a error, status becomes "executing" forever. This is filed
as v8:3096

LOG=N
BUG=v8:3097
R=mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/136003003

Patch from Yusuke Suzuki <yusukesuzuki@chromium.org>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18591 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/full-codegen-arm.cc

@@ -2118,19 +2118,21 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator,
     Expression *value,
     JSGeneratorObject::ResumeMode resume_mode) {
   // The value stays in r0, and is ultimately read by the resumed generator, as
-  // if the CallRuntime(Runtime::kSuspendJSGeneratorObject) returned it.  r1
-  // will hold the generator object until the activation has been resumed.
+  // if the CallRuntime(Runtime::kSuspendJSGeneratorObject) returned it. Or it
+  // is read to throw the value when the resumed generator is already closed.
+  // r1 will hold the generator object until the activation has been resumed.
   VisitForStackValue(generator);
   VisitForAccumulatorValue(value);
   __ pop(r1);
 
   // Check generator state.
-  Label wrong_state, done;
+  Label wrong_state, closed_state, done;
   __ ldr(r3, FieldMemOperand(r1, JSGeneratorObject::kContinuationOffset));
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting <= 0);
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed <= 0);
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting < 0);
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed == 0);
   __ cmp(r3, Operand(Smi::FromInt(0)));
-  __ b(le, &wrong_state);
+  __ b(eq, &closed_state);
+  __ b(lt, &wrong_state);
 
   // Load suspended function and context.
   __ ldr(cp, FieldMemOperand(r1, JSGeneratorObject::kContextOffset));
@@ -2205,6 +2207,21 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator,
   // Not reached: the runtime call returns elsewhere.
   __ stop("not-reached");
 
+  // Reach here when generator is closed.
+  __ bind(&closed_state);
+  if (resume_mode == JSGeneratorObject::NEXT) {
+    // Return completed iterator result when generator is closed.
+    __ LoadRoot(r2, Heap::kUndefinedValueRootIndex);
+    __ push(r2);
+    // Pop value from top-of-stack slot; box result into result register.
+    EmitCreateIteratorResult(true);
+  } else {
+    // Throw the provided value.
+    __ push(r0);
+    __ CallRuntime(Runtime::kThrow, 1);
+  }
+  __ jmp(&done);
+
   // Throw error if we attempt to operate on a running generator.
   __ bind(&wrong_state);
   __ push(r1);

src/ia32/full-codegen-ia32.cc

@@ -2074,19 +2074,21 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator,
     Expression *value,
     JSGeneratorObject::ResumeMode resume_mode) {
   // The value stays in eax, and is ultimately read by the resumed generator, as
-  // if the CallRuntime(Runtime::kSuspendJSGeneratorObject) returned it.  ebx
-  // will hold the generator object until the activation has been resumed.
+  // if the CallRuntime(Runtime::kSuspendJSGeneratorObject) returned it. Or it
+  // is read to throw the value when the resumed generator is already closed.
+  // ebx will hold the generator object until the activation has been resumed.
   VisitForStackValue(generator);
   VisitForAccumulatorValue(value);
   __ pop(ebx);
 
   // Check generator state.
-  Label wrong_state, done;
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting <= 0);
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed <= 0);
+  Label wrong_state, closed_state, done;
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting < 0);
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed == 0);
   __ cmp(FieldOperand(ebx, JSGeneratorObject::kContinuationOffset),
          Immediate(Smi::FromInt(0)));
-  __ j(less_equal, &wrong_state);
+  __ j(equal, &closed_state);
+  __ j(less, &wrong_state);
 
   // Load suspended function and context.
   __ mov(esi, FieldOperand(ebx, JSGeneratorObject::kContextOffset));
@@ -2156,6 +2158,20 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator,
   // Not reached: the runtime call returns elsewhere.
   __ Abort(kGeneratorFailedToResume);
 
+  // Reach here when generator is closed.
+  __ bind(&closed_state);
+  if (resume_mode == JSGeneratorObject::NEXT) {
+    // Return completed iterator result when generator is closed.
+    __ push(Immediate(isolate()->factory()->undefined_value()));
+    // Pop value from top-of-stack slot; box result into result register.
+    EmitCreateIteratorResult(true);
+  } else {
+    // Throw the provided value.
+    __ push(eax);
+    __ CallRuntime(Runtime::kThrow, 1);
+  }
+  __ jmp(&done);
+
   // Throw error if we attempt to operate on a running generator.
   __ bind(&wrong_state);
   __ push(ebx);

src/mips/full-codegen-mips.cc

@@ -2133,18 +2133,20 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator,
     Expression *value,
     JSGeneratorObject::ResumeMode resume_mode) {
   // The value stays in a0, and is ultimately read by the resumed generator, as
-  // if the CallRuntime(Runtime::kSuspendJSGeneratorObject) returned it.  a1
-  // will hold the generator object until the activation has been resumed.
+  // if the CallRuntime(Runtime::kSuspendJSGeneratorObject) returned it. Or it
+  // is read to throw the value when the resumed generator is already closed.
+  // a1 will hold the generator object until the activation has been resumed.
   VisitForStackValue(generator);
   VisitForAccumulatorValue(value);
   __ pop(a1);
 
   // Check generator state.
-  Label wrong_state, done;
+  Label wrong_state, closed_state, done;
   __ lw(a3, FieldMemOperand(a1, JSGeneratorObject::kContinuationOffset));
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting <= 0);
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed <= 0);
-  __ Branch(&wrong_state, le, a3, Operand(zero_reg));
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting < 0);
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed == 0);
+  __ Branch(&closed_state, eq, a3, Operand(zero_reg));
+  __ Branch(&wrong_state, lt, a3, Operand(zero_reg));
 
   // Load suspended function and context.
   __ lw(cp, FieldMemOperand(a1, JSGeneratorObject::kContextOffset));
@@ -2217,6 +2219,21 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator,
   // Not reached: the runtime call returns elsewhere.
   __ stop("not-reached");
 
+  // Reach here when generator is closed.
+  __ bind(&closed_state);
+  if (resume_mode == JSGeneratorObject::NEXT) {
+    // Return completed iterator result when generator is closed.
+    __ LoadRoot(a2, Heap::kUndefinedValueRootIndex);
+    __ push(a2);
+    // Pop value from top-of-stack slot; box result into result register.
+    EmitCreateIteratorResult(true);
+  } else {
+    // Throw the provided value.
+    __ push(a0);
+    __ CallRuntime(Runtime::kThrow, 1);
+  }
+  __ jmp(&done);
+
   // Throw error if we attempt to operate on a running generator.
   __ bind(&wrong_state);
   __ push(a1);

src/runtime.cc

@@ -3116,8 +3116,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_ResumeJSGeneratorObject) {
   ASSERT_EQ(frame->function(), generator_object->function());
   ASSERT(frame->function()->is_compiled());
 
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting <= 0);
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed <= 0);
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting < 0);
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed == 0);
 
   Address pc = generator_object->function()->code()->instruction_start();
   int offset = generator_object->continuation();

src/x64/full-codegen-x64.cc

@@ -2098,19 +2098,21 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator,
     Expression *value,
     JSGeneratorObject::ResumeMode resume_mode) {
   // The value stays in rax, and is ultimately read by the resumed generator, as
-  // if the CallRuntime(Runtime::kSuspendJSGeneratorObject) returned it.  rbx
-  // will hold the generator object until the activation has been resumed.
+  // if the CallRuntime(Runtime::kSuspendJSGeneratorObject) returned it. Or it
+  // is read to throw the value when the resumed generator is already closed.
+  // rbx will hold the generator object until the activation has been resumed.
   VisitForStackValue(generator);
   VisitForAccumulatorValue(value);
   __ pop(rbx);
 
   // Check generator state.
-  Label wrong_state, done;
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting <= 0);
-  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed <= 0);
+  Label wrong_state, closed_state, done;
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting < 0);
+  STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed == 0);
   __ SmiCompare(FieldOperand(rbx, JSGeneratorObject::kContinuationOffset),
                 Smi::FromInt(0));
-  __ j(less_equal, &wrong_state);
+  __ j(equal, &closed_state);
+  __ j(less, &wrong_state);
 
   // Load suspended function and context.
   __ movq(rsi, FieldOperand(rbx, JSGeneratorObject::kContextOffset));
@@ -2181,6 +2183,20 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator,
   // Not reached: the runtime call returns elsewhere.
   __ Abort(kGeneratorFailedToResume);
 
+  // Reach here when generator is closed.
+  __ bind(&closed_state);
+  if (resume_mode == JSGeneratorObject::NEXT) {
+    // Return completed iterator result when generator is closed.
+    __ PushRoot(Heap::kUndefinedValueRootIndex);
+    // Pop value from top-of-stack slot; box result into result register.
+    EmitCreateIteratorResult(true);
+  } else {
+    // Throw the provided value.
+    __ push(rax);
+    __ CallRuntime(Runtime::kThrow, 1);
+  }
+  __ jmp(&done);
+
   // Throw error if we attempt to operate on a running generator.
   __ bind(&wrong_state);
   __ push(rbx);

test/mjsunit/harmony/generators-iteration.js

@@ -35,6 +35,18 @@ function assertIteratorResult(value, done, result) {
   assertEquals({ value: value, done: done}, result);
 }
 
+function assertIteratorIsClosed(iter) {
+  assertIteratorResult(undefined, true, iter.next());
+  assertDoesNotThrow(function() { iter.next(); });
+}
+
+function assertThrownIteratorIsClosed(iter) {
+  // TODO(yusukesuzuki): Since status of a thrown generator is "executing",
+  // following tests are failed.
+  // https://code.google.com/p/v8/issues/detail?id=3096
+  // assertIteratorIsClosed(iter);
+}
+
 function TestGeneratorResultPrototype() {
   function* g() { yield 1; }
   var iter = g();
@@ -58,7 +70,7 @@ function TestGenerator(g, expected_values_for_next,
       // var v3 = iter.next();
       assertIteratorResult(v1, v2, iter.next());
     }
-    assertThrows(function() { iter.next(); }, Error);
+    assertIteratorIsClosed(iter);
   }
   function testSend(thunk) {
     var iter = thunk();
@@ -67,7 +79,7 @@ function TestGenerator(g, expected_values_for_next,
                            i == expected_values_for_send.length - 1,
                            iter.next(send_val));
     }
-    assertThrows(function() { iter.next(send_val); }, Error);
+    assertIteratorIsClosed(iter);
   }
   function testThrow(thunk) {
     for (var i = 0; i < expected_values_for_next.length; i++) {
@@ -79,7 +91,7 @@ function TestGenerator(g, expected_values_for_next,
       }
       function Sentinel() {}
       assertThrows(function () { iter.throw(new Sentinel); }, Sentinel);
-      assertThrows(function () { iter.next(); }, Error);
+      assertThrownIteratorIsClosed(iter);
     }
   }
 
@@ -394,21 +406,20 @@ function TestTryCatch(instantiate) {
     assertIteratorResult(1, false, iter.next());
     assertIteratorResult(2, false, iter.next());
     assertIteratorResult(3, false, iter.next());
-    assertIteratorResult(undefined, true, iter.next());
-    assertThrows(function() { iter.next(); }, Error);
+    assertIteratorIsClosed(iter);
   }
   Test1(instantiate(g));
 
   function Test2(iter) {
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test2(instantiate(g));
 
   function Test3(iter) {
     assertIteratorResult(1, false, iter.next());
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test3(instantiate(g));
 
@@ -418,8 +429,7 @@ function TestTryCatch(instantiate) {
     var exn = new Sentinel;
     assertIteratorResult(exn, false, iter.throw(exn));
     assertIteratorResult(3, false, iter.next());
-    assertIteratorResult(undefined, true, iter.next());
-    assertThrows(function() { iter.next(); }, Error);
+    assertIteratorIsClosed(iter);
   }
   Test4(instantiate(g));
 
@@ -430,8 +440,7 @@ function TestTryCatch(instantiate) {
     assertIteratorResult(exn, false, iter.throw(exn));
     assertIteratorResult(3, false, iter.next());
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
-
+    assertThrownIteratorIsClosed(iter);
   }
   Test5(instantiate(g));
 
@@ -441,7 +450,7 @@ function TestTryCatch(instantiate) {
     var exn = new Sentinel;
     assertIteratorResult(exn, false, iter.throw(exn));
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test6(instantiate(g));
 
@@ -449,8 +458,7 @@ function TestTryCatch(instantiate) {
     assertIteratorResult(1, false, iter.next());
     assertIteratorResult(2, false, iter.next());
     assertIteratorResult(3, false, iter.next());
-    assertIteratorResult(undefined, true, iter.next());
-    assertThrows(function() { iter.next(); }, Error);
+    assertIteratorIsClosed(iter);
   }
   Test7(instantiate(g));
 }
@@ -467,21 +475,20 @@ function TestTryFinally(instantiate) {
     assertIteratorResult(2, false, iter.next());
     assertIteratorResult(3, false, iter.next());
     assertIteratorResult(4, false, iter.next());
-    assertIteratorResult(undefined, true, iter.next());
-    assertThrows(function() { iter.next(); }, Error);
+    assertIteratorIsClosed(iter);
   }
   Test1(instantiate(g));
 
   function Test2(iter) {
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test2(instantiate(g));
 
   function Test3(iter) {
     assertIteratorResult(1, false, iter.next());
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test3(instantiate(g));
 
@@ -490,8 +497,7 @@ function TestTryFinally(instantiate) {
     assertIteratorResult(2, false, iter.next());
     assertIteratorResult(3, false, iter.throw(new Sentinel));
     assertThrows(function() { iter.next(); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
-
+    assertThrownIteratorIsClosed(iter);
   }
   Test4(instantiate(g));
 
@@ -500,7 +506,7 @@ function TestTryFinally(instantiate) {
     assertIteratorResult(2, false, iter.next());
     assertIteratorResult(3, false, iter.throw(new Sentinel));
     assertThrows(function() { iter.throw(new Sentinel2); }, Sentinel2);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test5(instantiate(g));
 
@@ -509,7 +515,7 @@ function TestTryFinally(instantiate) {
     assertIteratorResult(2, false, iter.next());
     assertIteratorResult(3, false, iter.next());
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test6(instantiate(g));
 
@@ -519,7 +525,7 @@ function TestTryFinally(instantiate) {
     assertIteratorResult(3, false, iter.next());
     assertIteratorResult(4, false, iter.next());
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test7(instantiate(g));
 
@@ -528,9 +534,7 @@ function TestTryFinally(instantiate) {
     assertIteratorResult(2, false, iter.next());
     assertIteratorResult(3, false, iter.next());
     assertIteratorResult(4, false, iter.next());
-    assertIteratorResult(undefined, true, iter.next());
-    assertThrows(function() { iter.next(); }, Error);
-
+    assertIteratorIsClosed(iter);
   }
   Test8(instantiate(g));
 }
@@ -557,14 +561,13 @@ function TestNestedTry(instantiate) {
     assertIteratorResult(3, false, iter.next());
     assertIteratorResult(4, false, iter.next());
     assertIteratorResult(5, false, iter.next());
-    assertIteratorResult(undefined, true, iter.next());
-    assertThrows(function() { iter.next(); }, Error);
+    assertIteratorIsClosed(iter);
   }
   Test1(instantiate(g));
 
   function Test2(iter) {
     assertThrows(function() { iter.throw(new Sentinel); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test2(instantiate(g));
 
@@ -572,7 +575,7 @@ function TestNestedTry(instantiate) {
     assertIteratorResult(1, false, iter.next());
     assertIteratorResult(4, false, iter.throw(new Sentinel));
     assertThrows(function() { iter.next(); }, Sentinel);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test3(instantiate(g));
 
@@ -580,7 +583,7 @@ function TestNestedTry(instantiate) {
     assertIteratorResult(1, false, iter.next());
     assertIteratorResult(4, false, iter.throw(new Sentinel));
     assertThrows(function() { iter.throw(new Sentinel2); }, Sentinel2);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test4(instantiate(g));
 
@@ -592,9 +595,7 @@ function TestNestedTry(instantiate) {
     assertIteratorResult(3, false, iter.next());
     assertIteratorResult(4, false, iter.next());
     assertIteratorResult(5, false, iter.next());
-    assertIteratorResult(undefined, true, iter.next());
-    assertThrows(function() { iter.next(); }, Error);
-
+    assertIteratorIsClosed(iter);
   }
   Test5(instantiate(g));
 
@@ -605,7 +606,7 @@ function TestNestedTry(instantiate) {
     assertIteratorResult(exn, false, iter.throw(exn));
     assertIteratorResult(4, false, iter.throw(new Sentinel2));
     assertThrows(function() { iter.next(); }, Sentinel2);
-    assertThrows(function() { iter.next(); }, Error);
+    assertThrownIteratorIsClosed(iter);
   }
   Test6(instantiate(g));
 
@@ -617,8 +618,7 @@ function TestNestedTry(instantiate) {
     assertIteratorResult(3, false, iter.next());
     assertIteratorResult(4, false, iter.throw(new Sentinel2));
     assertThrows(function() { iter.next(); }, Sentinel2);
-    assertThrows(function() { iter.next(); }, Error);
-
+    assertThrownIteratorIsClosed(iter);
   }
   Test7(instantiate(g));