Add support for yield expressions

src/runtime.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 2403 matching lines @@
   generator->set_function(function);
   generator->set_context(Context::cast(frame->context()));
   generator->set_continuation(0);
   generator->set_operand_stack(isolate->heap()->empty_fixed_array());
 
   return generator;
 }
 
 
 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.
   generator_object->set_context(Context::cast(frame->context()));
 
   // The return value is the hole for a suspend return, and anything else for a
   // resume return.
   return isolate->heap()->the_hole_value();
 }
 
 
 // Note that this function is the slow path for resuming generators.  It is only
 // called if the suspended activation had operands on the stack, stack handlers
 // needing rewinding, or if the resume should throw an exception.  The fast path
 // is handled directly in FullCodeGenerator::EmitGeneratorResume(), which is
 // inlined into GeneratorNext, GeneratorSend, and GeneratorThrow.
 // 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.

 
   ASSERT_EQ(frame->function(), generator_object->function());
 
   STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting <= 0);
   STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed <= 0);
 
   Address pc = generator_object->function()->code()->instruction_start();
   int offset = generator_object->continuation();
   ASSERT(offset > 0);
   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();
   return isolate->ThrowIllegalOperation();
 }
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_ThrowGeneratorStateError) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 1);
@@ skipping 10823 matching lines @@
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     isolate->counters()->gc_last_resort_from_js()->Increment();
     isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags,
                                        "Runtime::PerformGC");
   }
 }
 
 
 } }  // namespace v8::internal