From 757f400246dc45f889cdc0848094c417e44b8854 Mon Sep 17 00:00:00 2001 From: "wingo@igalia.com" Date: Wed, 12 Nov 2014 14:28:53 +0000 Subject: [PATCH] Leaving a generator via an exception causes it to close R=rossberg@chromium.org BUG=v8:3096 LOG=Y Review URL: https://codereview.chromium.org/717123002 Cr-Commit-Position: refs/heads/master@{#25297} git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25297 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 31 +-------------- src/arm64/full-codegen-arm64.cc | 32 +-------------- src/generator.js | 36 +++++++++++++++-- src/ia32/full-codegen-ia32.cc | 30 +------------- src/messages.js | 3 +- src/runtime/runtime-generator.cc | 12 +++--- src/runtime/runtime.h | 2 +- src/x64/full-codegen-x64.cc | 30 +------------- test/mjsunit/es6/generators-iteration.js | 5 +-- test/mjsunit/es6/generators-states.js | 67 ++++++++++++++++++++++++++++++++ 10 files changed, 112 insertions(+), 136 deletions(-) create mode 100644 test/mjsunit/es6/generators-states.js diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 32f6a2f..fbf3fec 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -2195,15 +2195,6 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, VisitForAccumulatorValue(value); __ pop(r1); - // Check generator state. - Label wrong_state, closed_state, done; - __ ldr(r3, FieldMemOperand(r1, JSGeneratorObject::kContinuationOffset)); - STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting < 0); - STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed == 0); - __ cmp(r3, Operand(Smi::FromInt(0))); - __ b(eq, &closed_state); - __ b(lt, &wrong_state); - // Load suspended function and context. __ ldr(cp, FieldMemOperand(r1, JSGeneratorObject::kContextOffset)); __ ldr(r4, FieldMemOperand(r1, JSGeneratorObject::kFunctionOffset)); @@ -2226,7 +2217,7 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, // Enter a new JavaScript frame, and initialize its slots as they were when // the generator was suspended. - Label resume_frame; + Label resume_frame, done; __ bind(&push_frame); __ bl(&resume_frame); __ jmp(&done); @@ -2286,26 +2277,6 @@ 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); - __ CallRuntime(Runtime::kThrowGeneratorStateError, 1); - __ bind(&done); context()->Plug(result_register()); } diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc index 11df6ae..710956f 100644 --- a/src/arm64/full-codegen-arm64.cc +++ b/src/arm64/full-codegen-arm64.cc @@ -4894,7 +4894,6 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, Expression *value, JSGeneratorObject::ResumeMode resume_mode) { ASM_LOCATION("FullCodeGenerator::EmitGeneratorResume"); - Register value_reg = x0; Register generator_object = x1; Register the_hole = x2; Register operand_stack_size = w3; @@ -4908,15 +4907,6 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, VisitForAccumulatorValue(value); __ Pop(generator_object); - // Check generator state. - Label wrong_state, closed_state, done; - __ Ldr(x10, FieldMemOperand(generator_object, - JSGeneratorObject::kContinuationOffset)); - STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting < 0); - STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed == 0); - __ CompareAndBranch(x10, Smi::FromInt(0), eq, &closed_state); - __ CompareAndBranch(x10, Smi::FromInt(0), lt, &wrong_state); - // Load suspended function and context. __ Ldr(cp, FieldMemOperand(generator_object, JSGeneratorObject::kContextOffset)); @@ -4942,7 +4932,7 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, // Enter a new JavaScript frame, and initialize its slots as they were when // the generator was suspended. - Label resume_frame; + Label resume_frame, done; __ Bl(&resume_frame); __ B(&done); @@ -4987,26 +4977,6 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, // Not reached: the runtime call returns elsewhere. __ Unreachable(); - // Reach here when generator is closed. - __ Bind(&closed_state); - if (resume_mode == JSGeneratorObject::NEXT) { - // Return completed iterator result when generator is closed. - __ LoadRoot(x10, Heap::kUndefinedValueRootIndex); - __ Push(x10); - // Pop value from top-of-stack slot; box result into result register. - EmitCreateIteratorResult(true); - } else { - // Throw the provided value. - __ Push(value_reg); - __ CallRuntime(Runtime::kThrow, 1); - } - __ B(&done); - - // Throw error if we attempt to operate on a running generator. - __ Bind(&wrong_state); - __ Push(generator_object); - __ CallRuntime(Runtime::kThrowGeneratorStateError, 1); - __ Bind(&done); context()->Plug(result_register()); } diff --git a/src/generator.js b/src/generator.js index 5fab8d3..19e7db8 100644 --- a/src/generator.js +++ b/src/generator.js @@ -20,8 +20,23 @@ function GeneratorObjectNext(value) { ['[Generator].prototype.next', this]); } - if (DEBUG_IS_ACTIVE) %DebugPrepareStepInIfStepping(this); - return %_GeneratorNext(this, value); + var continuation = %GeneratorGetContinuation(this); + if (continuation > 0) { + // Generator is suspended. + if (DEBUG_IS_ACTIVE) %DebugPrepareStepInIfStepping(this); + try { + return %_GeneratorNext(this, value); + } catch (e) { + %GeneratorClose(this); + throw e; + } + } else if (continuation == 0) { + // Generator is already closed. + return { value: void 0, done: true }; + } else { + // Generator is running. + throw MakeTypeError('generator_running', []); + } } function GeneratorObjectThrow(exn) { @@ -30,7 +45,22 @@ function GeneratorObjectThrow(exn) { ['[Generator].prototype.throw', this]); } - return %_GeneratorThrow(this, exn); + var continuation = %GeneratorGetContinuation(this); + if (continuation > 0) { + // Generator is suspended. + try { + return %_GeneratorThrow(this, exn); + } catch (e) { + %GeneratorClose(this); + throw e; + } + } else if (continuation == 0) { + // Generator is already closed. + throw exn; + } else { + // Generator is running. + throw MakeTypeError('generator_running', []); + } } function GeneratorObjectIterator() { diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 5138d57..672b46b 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -2125,15 +2125,6 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, VisitForAccumulatorValue(value); __ pop(ebx); - // Check generator state. - 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(equal, &closed_state); - __ j(less, &wrong_state); - // Load suspended function and context. __ mov(esi, FieldOperand(ebx, JSGeneratorObject::kContextOffset)); __ mov(edi, FieldOperand(ebx, JSGeneratorObject::kFunctionOffset)); @@ -2155,7 +2146,7 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, // Enter a new JavaScript frame, and initialize its slots as they were when // the generator was suspended. - Label resume_frame; + Label resume_frame, done; __ bind(&push_frame); __ call(&resume_frame); __ jmp(&done); @@ -2202,25 +2193,6 @@ 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); - __ CallRuntime(Runtime::kThrowGeneratorStateError, 1); - __ bind(&done); context()->Plug(result_register()); } diff --git a/src/messages.js b/src/messages.js index 6687b52..8a4e5e8 100644 --- a/src/messages.js +++ b/src/messages.js @@ -9,9 +9,8 @@ var kMessages = { cyclic_proto: ["Cyclic __proto__ value"], code_gen_from_strings: ["%0"], constructor_special_method: ["Class constructor may not be an accessor"], - generator_running: ["Generator is already running"], - generator_finished: ["Generator has already finished"], // TypeError + generator_running: ["Generator is already running"], unexpected_token: ["Unexpected token ", "%0"], unexpected_token_number: ["Unexpected number"], unexpected_token_string: ["Unexpected string"], diff --git a/src/runtime/runtime-generator.cc b/src/runtime/runtime-generator.cc index 9c2add7..ff07acd 100644 --- a/src/runtime/runtime-generator.cc +++ b/src/runtime/runtime-generator.cc @@ -135,16 +135,14 @@ RUNTIME_FUNCTION(Runtime_ResumeJSGeneratorObject) { } -RUNTIME_FUNCTION(Runtime_ThrowGeneratorStateError) { +RUNTIME_FUNCTION(Runtime_GeneratorClose) { HandleScope scope(isolate); DCHECK(args.length() == 1); CONVERT_ARG_HANDLE_CHECKED(JSGeneratorObject, generator, 0); - int continuation = generator->continuation(); - const char* message = continuation == JSGeneratorObject::kGeneratorClosed - ? "generator_finished" - : "generator_running"; - Vector > argv = HandleVector(NULL, 0); - THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewError(message, argv)); + + generator->set_continuation(JSGeneratorObject::kGeneratorClosed); + + return isolate->heap()->undefined_value(); } diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index b4ace19..d353fda 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -477,7 +477,7 @@ namespace internal { F(CreateJSGeneratorObject, 0, 1) \ F(SuspendJSGeneratorObject, 1, 1) \ F(ResumeJSGeneratorObject, 3, 1) \ - F(ThrowGeneratorStateError, 1, 1) \ + F(GeneratorClose, 1, 1) \ \ /* Arrays */ \ F(ArrayConstructor, -1, 1) \ diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index ec9afa4..ac31cfd 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -2157,15 +2157,6 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, VisitForAccumulatorValue(value); __ Pop(rbx); - // Check generator state. - 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(equal, &closed_state); - __ j(less, &wrong_state); - // Load suspended function and context. __ movp(rsi, FieldOperand(rbx, JSGeneratorObject::kContextOffset)); __ movp(rdi, FieldOperand(rbx, JSGeneratorObject::kFunctionOffset)); @@ -2187,7 +2178,7 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, // Enter a new JavaScript frame, and initialize its slots as they were when // the generator was suspended. - Label resume_frame; + Label resume_frame, done; __ bind(&push_frame); __ call(&resume_frame); __ jmp(&done); @@ -2234,25 +2225,6 @@ 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); - __ CallRuntime(Runtime::kThrowGeneratorStateError, 1); - __ bind(&done); context()->Plug(result_register()); } diff --git a/test/mjsunit/es6/generators-iteration.js b/test/mjsunit/es6/generators-iteration.js index b6fcdaa..faeb683 100644 --- a/test/mjsunit/es6/generators-iteration.js +++ b/test/mjsunit/es6/generators-iteration.js @@ -41,10 +41,7 @@ function assertIteratorIsClosed(iter) { } 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); + assertIteratorIsClosed(iter); } function TestGeneratorResultPrototype() { diff --git a/test/mjsunit/es6/generators-states.js b/test/mjsunit/es6/generators-states.js new file mode 100644 index 0000000..0a2173a --- /dev/null +++ b/test/mjsunit/es6/generators-states.js @@ -0,0 +1,67 @@ +// Copyright 2014 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Test generator states. + +function Foo() {} +function Bar() {} + +function assertIteratorResult(value, done, result) { + assertEquals({ value: value, done: done}, result); +} + +function assertIteratorIsClosed(iter) { + assertIteratorResult(undefined, true, iter.next()); + // Next and throw on a closed iterator. + assertDoesNotThrow(function() { iter.next(); }); + assertThrows(function() { iter.throw(new Bar); }, Bar); +} + +var iter; +function* nextGenerator() { yield iter.next(); } +function* throwGenerator() { yield iter.throw(new Bar); } + +// Throw on a suspendedStart iterator. +iter = nextGenerator(); +assertThrows(function() { iter.throw(new Foo) }, Foo) +assertThrows(function() { iter.throw(new Foo) }, Foo) +assertIteratorIsClosed(iter); + +// The same. +iter = throwGenerator(); +assertThrows(function() { iter.throw(new Foo) }, Foo) +assertThrows(function() { iter.throw(new Foo) }, Foo) +assertIteratorIsClosed(iter); + +// Next on an executing iterator raises a TypeError. +iter = nextGenerator(); +assertThrows(function() { iter.next() }, TypeError) +assertIteratorIsClosed(iter); + +// Throw on an executing iterator raises a TypeError. +iter = throwGenerator(); +assertThrows(function() { iter.next() }, TypeError) +assertIteratorIsClosed(iter); + +// Next on an executing iterator doesn't change the state of the +// generator. +iter = (function* () { + try { + iter.next(); + yield 1; + } catch (e) { + try { + // This next() should raise the same exception, because the first + // next() left the iter in the executing state. + iter.next(); + yield 2; + } catch (e) { + yield 3; + } + } + yield 4; +})(); +assertIteratorResult(3, false, iter.next()); +assertIteratorResult(4, false, iter.next()); +assertIteratorIsClosed(iter); -- 2.7.4