Add support for yield expressions

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index c942d029d45e0614ee8e5cd6cf84ebf84fc962b2..29d4558d3214bd0f2f0551b29ecf413f01f4d943 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -2421,51 +2421,52 @@ 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()));
+  JSFunction *function = JSFunction::cast(frame->function());

[Michael Starzinger, 2013/04/26 09:53:16]

style: The '*' sticks to the left. There are several other occurrences in the
code that I seemed to have missed in my earlier reviews.

[wingo, 2013/04/26 10:11:42]

Fixed this and other locations.

   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))

[Michael Starzinger, 2013/04/26 09:53:16]

nit: Conditional fits into one line.

[wingo, 2013/04/26 10:11:42]

Done.

+      return alloc;
+
+    for (int i = 0; i < operands_count; i++)

[Michael Starzinger, 2013/04/26 09:53:16]

style: Add curly braces around loop body.

[wingo, 2013/04/26 10:11:42]

Done.

+      // Since operand_stack is in NewSpace, no write barrier is needed.
+      operand_stack->set(i, frame->GetOperand(i), SKIP_WRITE_BARRIER);

[Michael Starzinger, 2013/04/26 09:53:16]

The allocation function above doesn't guarantee the FixedArray to be in
new-space. We need a write-barrier here. Or as Boromir would put it: "One does
not simply skip a write-barrier".

[wingo, 2013/04/26 10:11:42]

Good catch!

+    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.
@@ -2485,10 +2486,10 @@ 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();

[Michael Starzinger, 2013/04/26 09:53:16]

style: The '*' sticks to the left.

[wingo, 2013/04/26 10:11:42]

Done.

@@ -2504,18 +2505,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();