Calling a generator function returns a generator object

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index f34bf59a2208e4dd8f0b0416951e548a4374f9d1..1759e7d31a648ce32b83a130c6746a6f2365ec8b 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -486,17 +486,16 @@ class Parser::BlockState BASE_EMBEDDED {
 
 Parser::FunctionState::FunctionState(Parser* parser,
                                      Scope* scope,
-                                     bool is_generator,
                                      Isolate* isolate)
     : next_materialized_literal_index_(JSFunction::kLiteralsPrefixSize),
       next_handler_index_(0),
       expected_property_count_(0),
-      is_generator_(is_generator),
       only_simple_this_property_assignments_(false),
       this_property_assignments_(isolate->factory()->empty_fixed_array()),
       parser_(parser),
       outer_function_state_(parser->current_function_state_),
       outer_scope_(parser->top_scope_),
+      generator_iterator_variable_(NULL),
       saved_ast_node_id_(isolate->ast_node_id()),
       factory_(isolate, parser->zone()) {
   parser->top_scope_ = scope;
@@ -644,9 +643,8 @@ FunctionLiteral* Parser::DoParseProgram(CompilationInfo* info,
     }
     ParsingModeScope parsing_mode(this, mode);
 
-    bool is_generator = false;
     // Enters 'scope'.
-    FunctionState function_state(this, scope, is_generator, isolate());
+    FunctionState function_state(this, scope, isolate());
 
     top_scope_->SetLanguageMode(info->language_mode());
     ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(16, zone());
@@ -760,8 +758,7 @@ FunctionLiteral* Parser::ParseLazy(Utf16CharacterStream* source,
       scope = Scope::DeserializeScopeChain(info()->closure()->context(), scope,
                                            zone());
     }
-    bool is_generator = false;  // Top scope is not a generator.
-    FunctionState function_state(this, scope, is_generator, isolate());
+    FunctionState function_state(this, scope, isolate());
     ASSERT(scope->language_mode() != STRICT_MODE || !info()->is_classic_mode());
     ASSERT(scope->language_mode() != EXTENDED_MODE ||
            info()->is_extended_mode());
@@ -1035,6 +1032,26 @@ void* Parser::ParseSourceElements(ZoneList<Statement*>* processor,
   // SourceElements ::
   //   (ModuleElement)* <end_token>
 
+  // For generators, allocate and yield an iterator on function entry.

[Michael Starzinger, 2013/04/08 13:14:05]

As discussed offline, it scares me a little bit to model all of this at the AST
level. I agree that modeling it on some high-level IR would be great, but this
introduces AST nodes that have absolutely no correspondence to the source code.

[rossberg, 2013/04/09 09:37:44]

I think it's perfectly fine. With the more high-level features in ES6+ the AST
will generally move away further from being a direct representation of the
surface syntax. We already do similar things for other constructs, e.g. 'try',
let blocks, and modules, all of which introduce internal statements.

Perhaps the issue is less with the approach as such but with calling the thing
'AST'.

[rossberg, 2013/04/09 09:37:44]

I don't understand, though, why this code is here and not in
ParseFunctionLiteral. That seems odd.

[Michael Starzinger, 2013/04/09 10:40:14]

I am convinced, please ignore my initial comment.

+  if (is_generator()) {
+    ZoneList<Expression*>* arguments =
+        new(zone()) ZoneList<Expression*>(1, zone());
+    arguments->Add(factory()->NewThisFunction(), zone());
+    CallRuntime* allocation = factory()->NewCallRuntime(
+        isolate()->factory()->empty_string(),
+        Runtime::FunctionForId(Runtime::kCreateJSGeneratorIterator),
+        arguments);
+    VariableProxy* init_proxy = factory()->NewVariableProxy(
+        current_function_state_->generator_iterator_variable());
+    Assignment* assignment = factory()->NewAssignment(
+        Token::INIT_VAR, init_proxy, allocation, RelocInfo::kNoPosition);
+    VariableProxy* get_proxy = factory()->NewVariableProxy(
+        current_function_state_->generator_iterator_variable());
+    Yield* yield = factory()->NewYield(
+        get_proxy, assignment, false, RelocInfo::kNoPosition);
+    processor->Add(factory()->NewExpressionStatement(yield), zone());
+  }
+
   // Allocate a target stack to use for this set of source
   // elements. This way, all scripts and functions get their own
   // target stack thus avoiding illegal breaks and continues across
@@ -3105,8 +3122,10 @@ Expression* Parser::ParseYieldExpression(bool* ok) {
   int position = scanner().peek_location().beg_pos;
   Expect(Token::YIELD, CHECK_OK);
   bool is_yield_star = Check(Token::MUL);
+  Expression* iterator = factory()->NewVariableProxy(
+      current_function_state_->generator_iterator_variable());
   Expression* expression = ParseAssignmentExpression(false, CHECK_OK);
-  return factory()->NewYield(expression, is_yield_star, position);
+  return factory()->NewYield(iterator, expression, is_yield_star, position);
 }
 
 
@@ -4390,10 +4409,20 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
       : FunctionLiteral::kNotGenerator;
   AstProperties ast_properties;
   // Parse function body.
-  { FunctionState function_state(this, scope, is_generator, isolate());
+  { FunctionState function_state(this, scope, isolate());
     top_scope_->SetScopeName(function_name);
-    if (is_generator)
+
+    // Generators allocate their variables on the heap, to avoid copying on
+    // suspend and resume.  The iterator object also gets a local, which
+    // indicates this FunctionState is a generator, while also connecting the
+    // definition of the iterator with its uses in "yield" expressions.
+    if (is_generator) {
       top_scope_->ForceContextAllocation();
+      Handle<String> tempname = isolate()->factory()->InternalizeOneByteString(
+          STATIC_ASCII_VECTOR(".generator_iterator"));

[Michael Starzinger, 2013/04/08 13:14:05]

The current generator instance needs to be stored in the context, so I totally
see the need for having a context slot. But do we really need a temporary
variable for this and model all of this on the AST level? Can we just assign a
fixed context slot to hold the current generator instance?

[rossberg, 2013/04/09 09:37:44]

Again, I think this is fine. Hard-coding context slots seems far more hacky and
unmodular to me than introducing a temp var on the syntax level.

[Michael Starzinger, 2013/04/09 10:40:14]

I am convinced, please ignore my initial comment.

+      Variable* temp = top_scope_->DeclarationScope()->NewTemporary(tempname);
+      function_state.set_generator_iterator_variable(temp);
+    }
 
     //  FormalParameterList ::
     //    '(' (Identifier)*[','] ')'