Add support for yield expressions

This CL extends the generator suspend and resume implementation to
capture values on the operand stack.

It factors out some helpers to measure and access the operand stack into
the JavaScriptFrame class.  It also refactors the suspend and resume
helpers to avoid handle allocation.

BUG=v8:2355
TEST=mjsunit/harmony/generators-iteration

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

Patch from Andy Wingo <wingo@igalia.com>.

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

diff --git a/src/frames-inl.h b/src/frames-inl.h
index 83b37a5..6ca1f1f 100644 (file)
--- a/src/frames-inl.h
+++ b/src/frames-inl.h
@@ -213,6 +213,33 @@ Object* JavaScriptFrame::GetParameter(int index) const {
 }
 
 
+inline Address JavaScriptFrame::GetOperandSlot(int index) const {
+  Address base = fp() + JavaScriptFrameConstants::kLocal0Offset;
+  ASSERT(IsAddressAligned(base, kPointerSize));
+  ASSERT(type() == JAVA_SCRIPT);
+  ASSERT(index < ComputeOperandsCount());
+  // Operand stack grows down.
+  return base - index * kPointerSize;
+}
+
+
+inline Object* JavaScriptFrame::GetOperand(int index) const {
+  return Memory::Object_at(GetOperandSlot(index));
+}
+
+
+inline int JavaScriptFrame::ComputeOperandsCount() const {
+  Address base = fp() + JavaScriptFrameConstants::kLocal0Offset;
+  // Base points to low address of first operand and stack grows down, so add
+  // kPointerSize to get the actual stack size.
+  intptr_t stack_size_in_bytes = (base + kPointerSize) - sp();
+  ASSERT(IsAligned(stack_size_in_bytes, kPointerSize));
+  ASSERT(type() == JAVA_SCRIPT);
+  ASSERT(stack_size_in_bytes >= 0);
+  return stack_size_in_bytes >> kPointerSizeLog2;
+}
+
+
 inline Object* JavaScriptFrame::receiver() const {
   return GetParameter(-1);
 }

diff --git a/src/frames.h b/src/frames.h
index 11e8d28..678191b 100644 (file)
--- a/src/frames.h
+++ b/src/frames.h
@@ -536,6 +536,11 @@ class JavaScriptFrame: public StandardFrame {
     return GetNumberOfIncomingArguments();
   }
 
+  // Access the operand stack.
+  inline Address GetOperandSlot(int index) const;
+  inline Object* GetOperand(int index) const;
+  inline int ComputeOperandsCount() const;
+
   // Debugger access.
   void SetParameterValue(int index, Object* value) const;
 

diff --git a/src/runtime.cc b/src/runtime.cc
index ebe88fe..333d22a 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -2437,51 +2437,51 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateJSGeneratorObject) {
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_SuspendJSGeneratorObject) {
-  HandleScope scope(isolate);
+  NoHandleAllocation ha(isolate);
   ASSERT(args.length() == 1);
-  CONVERT_ARG_HANDLE_CHECKED(JSGeneratorObject, generator_object, 0);
+  CONVERT_ARG_CHECKED(JSGeneratorObject, generator_object, 0);
 
   JavaScriptFrameIterator stack_iterator(isolate);
-  JavaScriptFrame *frame = stack_iterator.frame();
-  Handle<JSFunction> function(JSFunction::cast(frame->function()));
+  JavaScriptFrame* frame = stack_iterator.frame();
+  JSFunction* function = JSFunction::cast(frame->function());
   RUNTIME_ASSERT(function->shared()->is_generator());
+  ASSERT_EQ(function, generator_object->function());
 
-  intptr_t offset = frame->pc() - function->code()->instruction_start();
-  ASSERT(*function == generator_object->function());
-  ASSERT(offset > 0 && Smi::IsValid(offset));
-  generator_object->set_continuation(static_cast<int>(offset));
+  // We expect there to be at least two values on the operand stack: the return
+  // value of the yield expression, and the argument to this runtime call.
+  // Neither of those should be saved.
+  int operands_count = frame->ComputeOperandsCount();
+  ASSERT(operands_count >= 2);
+  operands_count -= 2;
 
-  // Generator functions force context allocation for locals, so Local0 points
-  // to the bottom of the operand stack.  Assume the stack grows down.
-  //
-  // TODO(wingo): Move these magical calculations to frames.h when the
-  // generators implementation has stabilized.
-  intptr_t stack_size_in_bytes =
-      (frame->fp() + JavaScriptFrameConstants::kLocal0Offset) -
-      (frame->sp() - kPointerSize);
-  ASSERT(IsAddressAligned(frame->fp(), kPointerSize));
-  ASSERT(IsAligned(stack_size_in_bytes, kPointerSize));
-  ASSERT(stack_size_in_bytes >= 0);
-  ASSERT(Smi::IsValid(stack_size_in_bytes));
-  intptr_t stack_size = stack_size_in_bytes >> kPointerSizeLog2;
-
-  // We expect there to be at least two values on the stack: the return value of
-  // the yield expression, and the argument to this runtime call.  Neither of
-  // those should be saved.
-  ASSERT(stack_size >= 2);
-  stack_size -= 2;
-
-  if (stack_size == 0) {
+  if (operands_count == 0) {
     ASSERT_EQ(generator_object->operand_stack(),
               isolate->heap()->empty_fixed_array());
     // If there are no operands on the stack, there shouldn't be a handler
     // active either.
     ASSERT(!frame->HasHandler());
   } else {
-    // TODO(wingo): Save the operand stack and/or the stack handlers.
-    UNIMPLEMENTED();
+    if (frame->HasHandler()) {
+      // TODO(wingo): Unwind the stack handlers.
+      UNIMPLEMENTED();
+    }
+
+    FixedArray* operand_stack;
+    MaybeObject* alloc = isolate->heap()->AllocateFixedArray(operands_count);
+    if (!alloc->To(&operand_stack)) return alloc;
+
+    for (int i = 0; i < operands_count; i++) {
+      operand_stack->set(i, frame->GetOperand(i));
+    }
+    generator_object->set_operand_stack(operand_stack);
   }
 
+  // Set continuation down here to avoid side effects if the operand stack
+  // allocation fails.
+  intptr_t offset = frame->pc() - function->code()->instruction_start();
+  ASSERT(offset > 0 && Smi::IsValid(offset));
+  generator_object->set_continuation(offset);
+
   // It's possible for the context to be other than the initial context even if
   // there is no stack handler active.  For example, this is the case in the
   // body of a "with" statement.  Therefore we always save the context.
@@ -2501,13 +2501,13 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SuspendJSGeneratorObject) {
 // EmitGeneratorResumeResume is called in any case, as it needs to reconstruct
 // the stack frame and make space for arguments and operands.
 RUNTIME_FUNCTION(MaybeObject*, Runtime_ResumeJSGeneratorObject) {
-  HandleScope scope(isolate);
+  NoHandleAllocation ha(isolate);
   ASSERT(args.length() == 3);
-  CONVERT_ARG_HANDLE_CHECKED(JSGeneratorObject, generator_object, 0);
-  CONVERT_ARG_HANDLE_CHECKED(Object, value, 1);
+  CONVERT_ARG_CHECKED(JSGeneratorObject, generator_object, 0);
+  CONVERT_ARG_CHECKED(Object, value, 1);
   CONVERT_SMI_ARG_CHECKED(resume_mode_int, 2);
   JavaScriptFrameIterator stack_iterator(isolate);
-  JavaScriptFrame *frame = stack_iterator.frame();
+  JavaScriptFrame* frame = stack_iterator.frame();
 
   ASSERT_EQ(frame->function(), generator_object->function());
 
@@ -2520,18 +2520,26 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_ResumeJSGeneratorObject) {
   frame->set_pc(pc + offset);
   generator_object->set_continuation(JSGeneratorObject::kGeneratorExecuting);
 
-  if (generator_object->operand_stack()->length() != 0) {
-    // TODO(wingo): Copy operand stack.  Rewind handlers.
-    UNIMPLEMENTED();
+  FixedArray* operand_stack = generator_object->operand_stack();
+  int operands_count = operand_stack->length();
+  if (operands_count != 0) {
+    // TODO(wingo): Rewind stack handlers.  However until
+    // SuspendJSGeneratorObject unwinds them, we won't see frames with stack
+    // handlers here.
+    for (int i = 0; i < operands_count; i++) {
+      ASSERT_EQ(frame->GetOperand(i), isolate->heap()->the_hole_value());
+      Memory::Object_at(frame->GetOperandSlot(i)) = operand_stack->get(i);
+    }
+    generator_object->set_operand_stack(isolate->heap()->empty_fixed_array());
   }
 
   JSGeneratorObject::ResumeMode resume_mode =
       static_cast<JSGeneratorObject::ResumeMode>(resume_mode_int);
   switch (resume_mode) {
     case JSGeneratorObject::SEND:
-      return *value;
+      return value;
     case JSGeneratorObject::THROW:
-      return isolate->Throw(*value);
+      return isolate->Throw(value);
   }
 
   UNREACHABLE();
@@ -2544,7 +2552,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_ThrowGeneratorStateError) {
   ASSERT(args.length() == 1);
   CONVERT_ARG_HANDLE_CHECKED(JSGeneratorObject, generator, 0);
   int continuation = generator->continuation();
-  const char *message = continuation == JSGeneratorObject::kGeneratorClosed ?
+  const char* message = continuation == JSGeneratorObject::kGeneratorClosed ?
       "generator_finished" : "generator_running";
   Vector< Handle<Object> > argv = HandleVector<Object>(NULL, 0);
   Handle<Object> error = isolate->factory()->NewError(message, argv);

diff --git a/test/mjsunit/harmony/generators-iteration.js b/test/mjsunit/harmony/generators-iteration.js
index bc0bde0..ba0ae10 100644 (file)
--- a/test/mjsunit/harmony/generators-iteration.js
+++ b/test/mjsunit/harmony/generators-iteration.js
@@ -234,7 +234,7 @@ TestGenerator(
     [1, 2, undefined]);
 
 TestGenerator(
-    function* g() {
+    function* g19() {
       var x = 1;
       yield x;
       with({x:2}) { yield x; }
@@ -244,6 +244,32 @@ TestGenerator(
     "foo",
     [1, 2, 1, undefined]);
 
+TestGenerator(
+    function* g20() { yield (1 + (yield 2) + 3); },
+    [2, NaN, undefined],
+    "foo",
+    [2, "1foo3", undefined]);
+
+TestGenerator(
+    function* g21() { return (1 + (yield 2) + 3); },
+    [2, NaN],
+    "foo",
+    [2, "1foo3"]);
+
+TestGenerator(
+    function* g22() { yield (1 + (yield 2) + 3); yield (4 + (yield 5) + 6); },
+    [2, NaN, 5, NaN, undefined],
+    "foo",
+    [2, "1foo3", 5, "4foo6", undefined]);
+
+TestGenerator(
+    function* g23() {
+      return (yield (1 + (yield 2) + 3)) + (yield (4 + (yield 5) + 6));
+    },
+    [2, NaN, 5, NaN, NaN],
+    "foo",
+    [2, "1foo3", 5, "4foo6", "foofoo"]);
+
 function TestRecursion() {
   function TestNextRecursion() {
     function* g() { yield iter.next(); }