[generators] Improve yield* desugaring to save unnecessary try/catch and try/finally

Change-Id: Ia900c6c21d1ff330088a6566f8f6c7719c887ccf
Reviewed-on: https://chromium-review.googlesource.com/509256
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45466}

src/ast/ast.h

@@ -2518,15 +2518,18 @@ class RewritableExpression final : public Expression {
 // desired, must be done beforehand (see the parser).
 class Suspend final : public Expression {
  public:
-  enum OnException { kOnExceptionThrow, kOnExceptionRethrow };
+  // With {kNoControl}, the {Suspend} behaves like yield, except that it never
+  // throws and never causes the current generator to return. This is used to
+  // desugar yield*.
+  enum OnAbruptResume { kOnExceptionThrow, kOnExceptionRethrow, kNoControl };
 
   Expression* generator_object() const { return generator_object_; }
   Expression* expression() const { return expression_; }
-  OnException on_exception() const {
-    return OnExceptionField::decode(bit_field_);
+  OnAbruptResume on_abrupt_resume() const {
+    return OnAbruptResumeField::decode(bit_field_);
   }
   bool rethrow_on_exception() const {
-    return on_exception() == kOnExceptionRethrow;
+    return on_abrupt_resume() == kOnExceptionRethrow;
   }
 
   int suspend_id() const { return suspend_id_; }
@@ -2563,23 +2566,23 @@ class Suspend final : public Expression {
   friend class AstNodeFactory;
 
   Suspend(Expression* generator_object, Expression* expression, int pos,
-          OnException on_exception, SuspendFlags flags)
+          OnAbruptResume on_abrupt_resume, SuspendFlags flags)
       : Expression(pos, kSuspend),
         suspend_id_(-1),
         generator_object_(generator_object),
         expression_(expression) {
-    bit_field_ |=
-        OnExceptionField::encode(on_exception) | FlagsField::encode(flags);
+    bit_field_ |= OnAbruptResumeField::encode(on_abrupt_resume) |
+                  FlagsField::encode(flags);
   }
 
   int suspend_id_;
   Expression* generator_object_;
   Expression* expression_;
 
-  class OnExceptionField
-      : public BitField<OnException, Expression::kNextBitFieldIndex, 1> {};
+  class OnAbruptResumeField
+      : public BitField<OnAbruptResume, Expression::kNextBitFieldIndex, 2> {};
   class FlagsField
-      : public BitField<SuspendFlags, OnExceptionField::kNext,
+      : public BitField<SuspendFlags, OnAbruptResumeField::kNext,
                         static_cast<int>(SuspendFlags::kBitWidth)> {};
 };
 
@@ -3564,11 +3567,11 @@ class AstNodeFactory final BASE_EMBEDDED {
   }
 
   Suspend* NewSuspend(Expression* generator_object, Expression* expression,
-                      int pos, Suspend::OnException on_exception,
+                      int pos, Suspend::OnAbruptResume on_abrupt_resume,
                       SuspendFlags flags) {
     if (!expression) expression = NewUndefinedLiteral(pos);
     return new (zone_)
-        Suspend(generator_object, expression, pos, on_exception, flags);
+        Suspend(generator_object, expression, pos, on_abrupt_resume, flags);
   }
 
   Throw* NewThrow(Expression* exception, int pos) {

src/interpreter/bytecode-generator.cc

@@ -2567,54 +2567,55 @@ void BytecodeGenerator::BuildGeneratorResume(Suspend* expr,
           : Runtime::kInlineGeneratorGetInputOrDebugPos;
 
   DCHECK(generator.is_valid());
-  builder()
-      ->CallRuntime(get_generator_input, generator)
-      .StoreAccumulatorInRegister(input);
+  builder()->CallRuntime(get_generator_input, generator);
 
-  Register resume_mode = register_allocator()->NewRegister();
-  builder()
-      ->CallRuntime(Runtime::kInlineGeneratorGetResumeMode, generator)
-      .StoreAccumulatorInRegister(resume_mode);
+  if (expr->on_abrupt_resume() != Suspend::kNoControl) {
+    builder()->StoreAccumulatorInRegister(input);
 
-  // Now dispatch on resume mode.
+    Register resume_mode = register_allocator()->NewRegister();
+    builder()
+        ->CallRuntime(Runtime::kInlineGeneratorGetResumeMode, generator)
+        .StoreAccumulatorInRegister(resume_mode);
 
-  BytecodeLabel resume_with_next;
-  BytecodeLabel resume_with_throw;
+    // Now dispatch on resume mode.
+    BytecodeLabel resume_with_next;
+    BytecodeLabel resume_with_throw;
 
-  builder()
-      ->LoadLiteral(Smi::FromInt(JSGeneratorObject::kNext))
-      .CompareOperation(Token::EQ_STRICT, resume_mode)
-      .JumpIfTrue(ToBooleanMode::kAlreadyBoolean, &resume_with_next)
-      .LoadLiteral(Smi::FromInt(JSGeneratorObject::kThrow))
-      .CompareOperation(Token::EQ_STRICT, resume_mode)
-      .JumpIfTrue(ToBooleanMode::kAlreadyBoolean, &resume_with_throw);
-  // Fall through for resuming with return.
-
-  if (expr->is_async_generator()) {
-    // Async generator methods will produce the iter result object.
-    builder()->LoadAccumulatorWithRegister(input);
-    execution_control()->AsyncReturnAccumulator();
-  } else {
-    RegisterList args = register_allocator()->NewRegisterList(2);
     builder()
-        ->MoveRegister(input, args[0])
-        .LoadTrue()
-        .StoreAccumulatorInRegister(args[1])
-        .CallRuntime(Runtime::kInlineCreateIterResultObject, args);
-    execution_control()->ReturnAccumulator();
-  }
+        ->LoadLiteral(Smi::FromInt(JSGeneratorObject::kNext))
+        .CompareOperation(Token::EQ_STRICT, resume_mode)
+        .JumpIfTrue(ToBooleanMode::kAlreadyBoolean, &resume_with_next)
+        .LoadLiteral(Smi::FromInt(JSGeneratorObject::kThrow))
+        .CompareOperation(Token::EQ_STRICT, resume_mode)
+        .JumpIfTrue(ToBooleanMode::kAlreadyBoolean, &resume_with_throw);
+    // Fall through for resuming with return.
+
+    if (expr->is_async_generator()) {
+      // Async generator methods will produce the iter result object.
+      builder()->LoadAccumulatorWithRegister(input);
+      execution_control()->AsyncReturnAccumulator();
+    } else {
+      RegisterList args = register_allocator()->NewRegisterList(2);
+      builder()
+          ->MoveRegister(input, args[0])
+          .LoadTrue()
+          .StoreAccumulatorInRegister(args[1])
+          .CallRuntime(Runtime::kInlineCreateIterResultObject, args);
+      execution_control()->ReturnAccumulator();
+    }
 
-  builder()->Bind(&resume_with_throw);
-  builder()->SetExpressionPosition(expr);
-  builder()->LoadAccumulatorWithRegister(input);
-  if (expr->rethrow_on_exception()) {
-    builder()->ReThrow();
-  } else {
-    builder()->Throw();
-  }
+    builder()->Bind(&resume_with_throw);
+    builder()->SetExpressionPosition(expr);
+    builder()->LoadAccumulatorWithRegister(input);
+    if (expr->rethrow_on_exception()) {
+      builder()->ReThrow();
+    } else {
+      builder()->Throw();
+    }
 
-  builder()->Bind(&resume_with_next);
-  builder()->LoadAccumulatorWithRegister(input);
+    builder()->Bind(&resume_with_next);
+    builder()->LoadAccumulatorWithRegister(input);
+  }
 }
 
 void BytecodeGenerator::VisitSuspend(Suspend* expr) {

src/parsing/parser-base.h

@@ -1397,17 +1397,16 @@ class ParserBase {
     return factory()->NewReturnStatement(expr, pos);
   }
 
-  inline SuspendExpressionT BuildSuspend(ExpressionT generator,
-                                         ExpressionT expr, int pos,
-                                         Suspend::OnException on_exception,
-                                         SuspendFlags suspend_type) {
+  inline SuspendExpressionT BuildSuspend(
+      ExpressionT generator, ExpressionT expr, int pos,
+      Suspend::OnAbruptResume on_abrupt_resume, SuspendFlags suspend_type) {
     DCHECK_EQ(0,
               static_cast<int>(suspend_type & ~SuspendFlags::kSuspendTypeMask));
     if (V8_UNLIKELY(is_async_generator())) {
       suspend_type = static_cast<SuspendFlags>(suspend_type |
                                                SuspendFlags::kAsyncGenerator);
     }
-    return factory()->NewSuspend(generator, expr, pos, on_exception,
+    return factory()->NewSuspend(generator, expr, pos, on_abrupt_resume,
                                  suspend_type);
   }
 

src/parsing/parser.cc

@@ -4237,7 +4237,6 @@ void Parser::SetFunctionName(Expression* value, const AstRawString* name) {
   }
 }
 
-
 // Desugaring of yield*
 // ====================
 //
@@ -4282,17 +4281,9 @@ void Parser::SetFunctionName(Expression* value, const AstRawString* name) {
 //
 //       // From the generator to its user:
 //       // Forward output, receive new input, and determine resume mode.
-//       mode = kReturn;
-//       try {
-//         try {
-//           RawYield(output);  // See explanation above.
-//           mode = kNext;
-//         } catch (error) {
-//           mode = kThrow;
-//         }
-//       } finally {
-//         input = function.sent;
-//         continue;
+//       RawYield(output);  // See explanation above.
+//       mode = %GeneratorGetResumeMode();
+//       input = function.sent;
 //       }
 //     }
 //
@@ -4509,48 +4500,31 @@ Expression* Parser::RewriteYieldStar(Expression* generator,
         property, break_loop, factory()->NewEmptyStatement(nopos), nopos);
   }
 
-
-  // mode = kReturn;
-  Statement* set_mode_return;
-  {
-    Expression* mode_proxy = factory()->NewVariableProxy(var_mode);
-    Expression* kreturn =
-        factory()->NewSmiLiteral(JSGeneratorObject::kReturn, nopos);
-    Expression* assignment =
-        factory()->NewAssignment(Token::ASSIGN, mode_proxy, kreturn, nopos);
-    set_mode_return = factory()->NewExpressionStatement(assignment, nopos);
-  }
-
   // Yield(output);
   Statement* yield_output;
   {
     Expression* output_proxy = factory()->NewVariableProxy(var_output);
     Suspend* yield =
-        BuildSuspend(generator, output_proxy, nopos, Suspend::kOnExceptionThrow,
+        BuildSuspend(generator, output_proxy, nopos, Suspend::kNoControl,
                      SuspendFlags::kYieldStar);
     yield_output = factory()->NewExpressionStatement(yield, nopos);
   }
 
-  // mode = kNext;
-  Statement* set_mode_next;
+  // mode = %GeneratorGetResumeMode();
+  Statement* get_mode;
   {
     Expression* mode_proxy = factory()->NewVariableProxy(var_mode);
-    Expression* knext =
-        factory()->NewSmiLiteral(JSGeneratorObject::kNext, nopos);
-    Expression* assignment =
-        factory()->NewAssignment(Token::ASSIGN, mode_proxy, knext, nopos);
-    set_mode_next = factory()->NewExpressionStatement(assignment, nopos);
-  }
 
-  // mode = kThrow;
-  Statement* set_mode_throw;
-  {
-    Expression* mode_proxy = factory()->NewVariableProxy(var_mode);
-    Expression* kthrow =
-        factory()->NewSmiLiteral(JSGeneratorObject::kThrow, nopos);
+    ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
+    VariableProxy* generator = factory()->NewVariableProxy(
+        function_state_->generator_object_variable());
+    args->Add(generator, zone());
+    Expression* mode = factory()->NewCallRuntime(
+        Runtime::kInlineGeneratorGetResumeMode, args, pos);
+
     Expression* assignment =
-        factory()->NewAssignment(Token::ASSIGN, mode_proxy, kthrow, nopos);
-    set_mode_throw = factory()->NewExpressionStatement(assignment, nopos);
+        factory()->NewAssignment(Token::ASSIGN, mode_proxy, mode, nopos);
+    get_mode = factory()->NewExpressionStatement(assignment, nopos);
   }
 
   // input = function.sent;
@@ -4596,36 +4570,6 @@ Expression* Parser::RewriteYieldStar(Expression* generator,
 
   // Now put things together.
 
-  // try { ... } catch(e) { ... }
-  Statement* try_catch;
-  {
-    Block* try_block = factory()->NewBlock(nullptr, 2, false, nopos);
-    try_block->statements()->Add(yield_output, zone());
-    try_block->statements()->Add(set_mode_next, zone());
-
-    Block* catch_block = factory()->NewBlock(nullptr, 1, false, nopos);
-    catch_block->statements()->Add(set_mode_throw, zone());
-
-    Scope* catch_scope = NewHiddenCatchScopeWithParent(scope());
-
-    try_catch = factory()->NewTryCatchStatementForDesugaring(
-        try_block, catch_scope, catch_block, nopos);
-  }
-
-  // try { ... } finally { ... }
-  Statement* try_finally;
-  {
-    Block* try_block = factory()->NewBlock(nullptr, 1, false, nopos);
-    try_block->statements()->Add(try_catch, zone());
-
-    Block* finally = factory()->NewBlock(nullptr, 2, false, nopos);
-    finally->statements()->Add(get_input, zone());
-    finally->statements()->Add(factory()->NewContinueStatement(loop, nopos),
-                               zone());
-
-    try_finally = factory()->NewTryFinallyStatement(try_block, finally, nopos);
-  }
-
   // switch (mode) { ... }
   SwitchStatement* switch_mode = factory()->NewSwitchStatement(nullptr, nopos);
   {
@@ -4665,7 +4609,6 @@ Expression* Parser::RewriteYieldStar(Expression* generator,
     Block* loop_body = factory()->NewBlock(nullptr, 5, false, nopos);
     loop_body->statements()->Add(switch_mode, zone());
     loop_body->statements()->Add(if_done, zone());
-    loop_body->statements()->Add(set_mode_return, zone());
 
     if (is_async_generator()) {
       // AsyncGeneratorYield does not yield the original iterator result,
@@ -4682,7 +4625,9 @@ Expression* Parser::RewriteYieldStar(Expression* generator,
           factory()->NewExpressionStatement(assign, nopos), zone());
     }
 
-    loop_body->statements()->Add(try_finally, zone());
+    loop_body->statements()->Add(yield_output, zone());
+    loop_body->statements()->Add(get_input, zone());
+    loop_body->statements()->Add(get_mode, zone());
 
     loop->Initialize(factory()->NewBooleanLiteral(true, nopos), loop_body);
   }

src/parsing/preparser.h

@@ -644,7 +644,7 @@ class PreParserFactory {
   }
   PreParserExpression NewSuspend(PreParserExpression generator_object,
                                  PreParserExpression expression, int pos,
-                                 Suspend::OnException on_exception,
+                                 Suspend::OnAbruptResume on_abrupt_resume,
                                  SuspendFlags flags) {
     return PreParserExpression::Default();
   }

test/cctest/interpreter/test-bytecode-generator.cc

@@ -2348,6 +2348,10 @@ TEST(Generators) {
 
       "function* f() { for (let x of [42]) yield x }\n"
       "f();\n",
+
+      "function* g() { yield 42 }\n"
+      "function* f() { yield* g() }\n"
+      "f();\n",
   };
 
   CHECK(CompareTexts(BuildActual(printer, snippets),