Reland "[async-iteration] implement spec-change to yield in async g…

…enerators"

Per https://github.com/tc39/proposal-async-iteration/pull/102/files:

AsyncGeneratorResolve no longer unwraps a value component. Instead, the value is
unwrapped before the builtin call via Await, allowing Promise rejections to
affect the generator control flow.

Thus, all `yield <expr>` implicitly become `yield await <expr>`.

Additionally, `return <expr>` becomes `return await <expr>`. Finally, when the
generator is resumed with `.return()`, the parameter passed to .return() is
awaited before generator execution properly continues).

BUG=v8:6187, v8:5855
R=littledan@chromium.org, neis@chromium.org, adamk@chromium.org
TBR=rmcilroy@chromium.org, neis@chromium.org

Cq-Include-Trybots: master.tryserver.v8:v8_linux_noi18n_rel_ng
Change-Id: Id7718028fd555481f9f4ca0dbecfa788e3057c48
Reviewed-on: https://chromium-review.googlesource.com/594500
Reviewed-by: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Cr-Commit-Position: refs/heads/master@{#47058}

src/ast/ast-numbering.cc

@@ -95,6 +95,7 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
   int suspend_count_;
   AstProperties properties_;
   LanguageMode language_mode_;
+  FunctionKind function_kind_;
   // The slot cache allows us to reuse certain feedback slots.
   FeedbackSlotCache slot_cache_;
   BailoutReason disable_fullcodegen_reason_;
@@ -218,7 +219,13 @@ void AstNumberingVisitor::VisitSuspend(Suspend* node) {
   Visit(node->expression());
 }
 
-void AstNumberingVisitor::VisitYield(Yield* node) { VisitSuspend(node); }
+void AstNumberingVisitor::VisitYield(Yield* node) {
+  node->set_suspend_id(suspend_count_++);
+  if (IsAsyncGeneratorFunction(function_kind_)) {
+    node->set_await_return_value_suspend_id(suspend_count_++);
+  }
+  Visit(node->expression());
+}
 
 void AstNumberingVisitor::VisitYieldStar(YieldStar* node) {
   VisitSuspend(node);
@@ -581,6 +588,7 @@ bool AstNumberingVisitor::Renumber(FunctionLiteral* node) {
     DisableFullCodegen(kClassConstructorFunction);
   }
 
+  function_kind_ = node->kind();
   LanguageModeScope language_mode_scope(this, node->language_mode());
 
   if (collect_type_profile_) {

src/ast/ast.h

@@ -2275,10 +2275,24 @@ class Suspend : public Expression {
 };
 
 class Yield final : public Suspend {
+ public:
+  inline int await_return_value_suspend_id() const {
+    DCHECK_NE(await_return_value_suspend_id_, -1);
+    return await_return_value_suspend_id_;
+  }
+  void set_await_return_value_suspend_id(int id) {
+    await_return_value_suspend_id_ = id;
+  }
+
  private:
   friend class AstNodeFactory;
   Yield(Expression* expression, int pos, OnAbruptResume on_abrupt_resume)
-      : Suspend(kYield, expression, pos, on_abrupt_resume) {}
+      : Suspend(kYield, expression, pos, on_abrupt_resume),
+        await_return_value_suspend_id_(-1) {}
+
+  // TODO(caitp): remove from class once `await` handled by AsyncGeneratorReturn
+  // stub.
+  int await_return_value_suspend_id_;
 };
 
 class YieldStar final : public Suspend {

src/bootstrapper.cc

@@ -1487,6 +1487,18 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     info->set_internal_formal_parameter_count(1);
     info->set_length(1);
     native_context()->set_async_generator_await_reject_shared_fun(*info);
+
+    code = BUILTIN_CODE(isolate, AsyncGeneratorReturnResolveClosure);
+    info = factory->NewSharedFunctionInfo(factory->empty_string(), code, false);
+    info->set_internal_formal_parameter_count(1);
+    info->set_length(1);
+    native_context()->set_async_generator_return_resolve_shared_fun(*info);
+
+    code = BUILTIN_CODE(isolate, AsyncGeneratorReturnRejectClosure);
+    info = factory->NewSharedFunctionInfo(factory->empty_string(), code, false);
+    info->set_internal_formal_parameter_count(1);
+    info->set_length(1);
+    native_context()->set_async_generator_return_reject_shared_fun(*info);
   }
 
   {  // --- A r r a y ---

src/builtins/builtins-async-generator-gen.cc

@@ -418,8 +418,8 @@ TF_BUILTIN(AsyncGeneratorResumeNext, AsyncGeneratorBuiltinsAssembler) {
            LoadGeneratorState(generator));
   VARIABLE(var_next, MachineRepresentation::kTagged,
            LoadFirstAsyncGeneratorRequestFromQueue(generator));
-  Variable* labels[] = {&var_state, &var_next};
-  Label start(this, 2, labels);
+  Variable* loop_variables[] = {&var_state, &var_next};
+  Label start(this, 2, loop_variables);
   Goto(&start);
   BIND(&start);
 
@@ -453,10 +453,10 @@ TF_BUILTIN(AsyncGeneratorResumeNext, AsyncGeneratorBuiltinsAssembler) {
            &fulfill_promise, &reject_promise);
 
     BIND(&fulfill_promise);
-    CallBuiltin(Builtins::kAsyncGeneratorResolve, context, generator,
-                LoadValueFromAsyncGeneratorRequest(next), TrueConstant());
-    var_next.Bind(LoadFirstAsyncGeneratorRequestFromQueue(generator));
-    Goto(&start);
+    // Unwrap Promise values for "return" in state "suspendedStart".
+    // This simulates `return await request.[[Completion]].[[Value]]`
+    TailCallBuiltin(Builtins::kAsyncGeneratorReturnProcessor, context,
+                    generator);
 
     BIND(&reject_promise);
     CallBuiltin(Builtins::kAsyncGeneratorReject, context, generator,
@@ -495,19 +495,35 @@ TF_BUILTIN(AsyncGeneratorResolve, AsyncGeneratorBuiltinsAssembler) {
                   HasInstanceType(generator, JS_ASYNC_GENERATOR_OBJECT_TYPE));
   CSA_ASSERT(this, IsGeneratorNotSuspendedForAwait(generator));
 
+  // If this assertion fails, the `value` component was not Awaited as it should
+  // have been, per https://github.com/tc39/proposal-async-iteration/pull/102/.
+  CSA_SLOW_ASSERT(this, TaggedDoesntHaveInstanceType(value, JS_PROMISE_TYPE));
+
   Node* const next = TakeFirstAsyncGeneratorRequestFromQueue(generator);
   Node* const promise = LoadPromiseFromAsyncGeneratorRequest(next);
 
-  Node* const wrapper = AllocateAndInitJSPromise(context);
-  CallBuiltin(Builtins::kResolveNativePromise, context, wrapper, value);
+  // Let iteratorResult be CreateIterResultObject(value, done).
+  Node* const iter_result = Allocate(JSIteratorResult::kSize);
+  {
+    Node* map = LoadContextElement(LoadNativeContext(context),
+                                   Context::ITERATOR_RESULT_MAP_INDEX);
+    StoreMapNoWriteBarrier(iter_result, map);
+    StoreObjectFieldRoot(iter_result, JSIteratorResult::kPropertiesOrHashOffset,
+                         Heap::kEmptyFixedArrayRootIndex);
+    StoreObjectFieldRoot(iter_result, JSIteratorResult::kElementsOffset,
+                         Heap::kEmptyFixedArrayRootIndex);
+    StoreObjectFieldNoWriteBarrier(iter_result, JSIteratorResult::kValueOffset,
+                                   value);
+    StoreObjectFieldNoWriteBarrier(iter_result, JSIteratorResult::kDoneOffset,
+                                   done);
+  }
 
-  Node* const on_fulfilled =
-      CreateUnwrapClosure(LoadNativeContext(context), done);
+  // Perform Call(promiseCapability.[[Resolve]], undefined, «iteratorResult»).
+  CallBuiltin(Builtins::kResolveNativePromise, context, promise, iter_result);
 
   // Per spec, AsyncGeneratorResolve() returns undefined. However, for the
   // benefit of %TraceExit(), return the Promise.
-  Return(CallBuiltin(Builtins::kPerformNativePromiseThen, context, wrapper,
-                     on_fulfilled, UndefinedConstant(), promise));
+  Return(promise);
 }
 
 TF_BUILTIN(AsyncGeneratorReject, AsyncGeneratorBuiltinsAssembler) {
@@ -523,5 +539,65 @@ TF_BUILTIN(AsyncGeneratorReject, AsyncGeneratorBuiltinsAssembler) {
                      TrueConstant()));
 }
 
+TF_BUILTIN(AsyncGeneratorReturnProcessor, AsyncGeneratorBuiltinsAssembler) {
+  Node* const generator = Parameter(Descriptor::kGenerator);
+  Node* const req = LoadFirstAsyncGeneratorRequestFromQueue(generator);
+
+  CSA_SLOW_ASSERT(this, SmiEqual(LoadResumeTypeFromAsyncGeneratorRequest(req),
+                                 SmiConstant(JSGeneratorObject::kReturn)));
+
+  ContextInitializer init_closure_context = [&](Node* context) {
+    StoreContextElementNoWriteBarrier(context, AwaitContext::kGeneratorSlot,
+                                      generator);
+  };
+
+  const bool kIsPredictedAsCaught = false;
+
+  Node* const context = Parameter(Descriptor::kContext);
+  Node* const outer_promise = LoadPromiseFromAsyncGeneratorRequest(req);
+  Node* const value = LoadValueFromAsyncGeneratorRequest(req);
+  Node* const promise = Await(
+      context, generator, value, outer_promise, AwaitContext::kLength,
+      init_closure_context, Context::ASYNC_GENERATOR_RETURN_RESOLVE_SHARED_FUN,
+      Context::ASYNC_GENERATOR_RETURN_REJECT_SHARED_FUN, kIsPredictedAsCaught);
+
+  CSA_SLOW_ASSERT(this, IsGeneratorNotSuspendedForAwait(generator));
+  StoreObjectField(generator, JSAsyncGeneratorObject::kAwaitedPromiseOffset,
+                   promise);
+  Return(UndefinedConstant());
+}
+
+TF_BUILTIN(AsyncGeneratorReturnResolveClosure,
+           AsyncGeneratorBuiltinsAssembler) {
+  Node* const context = Parameter(Descriptor::kContext);
+  Node* const value = Parameter(Descriptor::kValue);
+  Node* const generator =
+      LoadContextElement(context, AwaitContext::kGeneratorSlot);
+
+  CSA_SLOW_ASSERT(this, IsGeneratorSuspendedForAwait(generator));
+  ClearAwaitedPromise(generator);
+
+  //  Return ! AsyncGeneratorResolve(_F_.[[Generator]], _value_, *true*).
+  CallBuiltin(Builtins::kAsyncGeneratorResolve, context, generator, value,
+              TrueConstant());
+
+  TailCallBuiltin(Builtins::kAsyncGeneratorResumeNext, context, generator);
+}
+
+TF_BUILTIN(AsyncGeneratorReturnRejectClosure, AsyncGeneratorBuiltinsAssembler) {
+  Node* const context = Parameter(Descriptor::kContext);
+  Node* const value = Parameter(Descriptor::kValue);
+  Node* const generator =
+      LoadContextElement(context, AwaitContext::kGeneratorSlot);
+
+  CSA_SLOW_ASSERT(this, IsGeneratorSuspendedForAwait(generator));
+  ClearAwaitedPromise(generator);
+
+  // Return ! AsyncGeneratorReject(_F_.[[Generator]], _reason_).
+  CallBuiltin(Builtins::kAsyncGeneratorReject, context, generator, value);
+
+  TailCallBuiltin(Builtins::kAsyncGeneratorResumeNext, context, generator);
+}
+
 }  // namespace internal
 }  // namespace v8

src/builtins/builtins-definitions.h

@@ -1050,6 +1050,7 @@ namespace internal {
                                                                                \
   TFS(AsyncGeneratorResolve, kGenerator, kValue, kDone)                        \
   TFS(AsyncGeneratorReject, kGenerator, kValue)                                \
+  TFS(AsyncGeneratorReturnProcessor, kGenerator)                               \
   TFS(AsyncGeneratorResumeNext, kGenerator)                                    \
                                                                                \
   /* AsyncGeneratorFunction( p1, p2, ... pn, body ) */                         \
@@ -1074,6 +1075,8 @@ namespace internal {
   TFJ(AsyncGeneratorAwaitUncaught, 1, kAwaited)                                \
   TFJ(AsyncGeneratorAwaitResolveClosure, 1, kValue)                            \
   TFJ(AsyncGeneratorAwaitRejectClosure, 1, kValue)                             \
+  TFJ(AsyncGeneratorReturnResolveClosure, 1, kValue)                           \
+  TFJ(AsyncGeneratorReturnRejectClosure, 1, kValue)                            \
                                                                                \
   /* Async-from-Sync Iterator */                                               \
                                                                                \

src/code-stub-assembler.cc

@@ -1128,6 +1128,15 @@ Node* CodeStubAssembler::DoesntHaveInstanceType(Node* object,
   return Word32NotEqual(LoadInstanceType(object), Int32Constant(instance_type));
 }
 
+Node* CodeStubAssembler::TaggedDoesntHaveInstanceType(Node* any_tagged,
+                                                      InstanceType type) {
+  /* return Phi <TaggedIsSmi(val), DoesntHaveInstanceType(val, type)> */
+  Node* tagged_is_smi = TaggedIsSmi(any_tagged);
+  return Select(tagged_is_smi, [=]() { return tagged_is_smi; },
+                [=]() { return DoesntHaveInstanceType(any_tagged, type); },
+                MachineRepresentation::kBit);
+}
+
 Node* CodeStubAssembler::LoadProperties(Node* object) {
   return LoadObjectField(object, JSObject::kPropertiesOrHashOffset);
 }

src/code-stub-assembler.h

@@ -393,6 +393,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   // Compare the instance the type of the object against the provided one.
   Node* HasInstanceType(Node* object, InstanceType type);
   Node* DoesntHaveInstanceType(Node* object, InstanceType type);
+  Node* TaggedDoesntHaveInstanceType(Node* any_tagged, InstanceType type);
   // Load the properties backing store of a JSObject.
   Node* LoadProperties(Node* object);
   // Load the elements backing store of a JSObject.

src/contexts.h

@@ -217,6 +217,10 @@ enum ContextLookupFlags {
     async_generator_await_reject_shared_fun)                                   \
   V(ASYNC_GENERATOR_AWAIT_RESOLVE_SHARED_FUN, SharedFunctionInfo,              \
     async_generator_await_resolve_shared_fun)                                  \
+  V(ASYNC_GENERATOR_RETURN_RESOLVE_SHARED_FUN, SharedFunctionInfo,             \
+    async_generator_return_resolve_shared_fun)                                 \
+  V(ASYNC_GENERATOR_RETURN_REJECT_SHARED_FUN, SharedFunctionInfo,              \
+    async_generator_return_reject_shared_fun)                                  \
   V(ATOMICS_OBJECT, JSObject, atomics_object)                                  \
   V(BOOLEAN_FUNCTION_INDEX, JSFunction, boolean_function)                      \
   V(BOUND_FUNCTION_WITH_CONSTRUCTOR_MAP_INDEX, Map,                            \

src/interpreter/bytecode-generator.cc

@@ -1579,7 +1579,7 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) {
 
 void BytecodeGenerator::VisitTryFinallyStatement(TryFinallyStatement* stmt) {
   // We can't know whether the finally block will override ("catch") an
-  // exception thrown in the try bblock, so we just adopt the outer prediction.
+  // exception thrown in the try block, so we just adopt the outer prediction.
   TryFinallyBuilder try_control_builder(builder(), catch_prediction());
 
   // We keep a record of all paths that enter the finally-block to be able to
@@ -2662,6 +2662,7 @@ void BytecodeGenerator::VisitYield(Yield* expr) {
     builder()->LoadAccumulatorWithRegister(input);
     if (IsAsyncGeneratorFunction(function_kind())) {
       // Async generator methods will produce the iter result object.
+      BuildAwait(expr->await_return_value_suspend_id());
       execution_control()->AsyncReturnAccumulator();
     } else {
       execution_control()->ReturnAccumulator();
@@ -2912,7 +2913,7 @@ void BytecodeGenerator::VisitYieldStar(YieldStar* expr) {
   builder()->LoadAccumulatorWithRegister(output_value);
 }
 
-void BytecodeGenerator::VisitAwait(Await* expr) {
+void BytecodeGenerator::BuildAwait(int suspend_id) {
   // Rather than HandlerTable::UNCAUGHT, async functions use
   // HandlerTable::ASYNC_AWAIT to communicate that top-level exceptions are
   // transformed into promise rejections. This is necessary to prevent emitting
@@ -2921,8 +2922,6 @@ void BytecodeGenerator::VisitAwait(Await* expr) {
   // HandlerTable::UNCAUGHT.
   DCHECK(catch_prediction() != HandlerTable::UNCAUGHT);
 
-  builder()->SetExpressionPosition(expr);
-  VisitForAccumulatorValue(expr->expression());
   {
     // Await(operand) and suspend.
     RegisterAllocationScope register_scope(this);
@@ -2959,7 +2958,7 @@ void BytecodeGenerator::VisitAwait(Await* expr) {
     builder()->CallJSRuntime(await_builtin_context_index, args);
   }
 
-  BuildSuspendPoint(expr->suspend_id());
+  BuildSuspendPoint(suspend_id);
 
   Register input = register_allocator()->NewRegister();
   Register resume_mode = register_allocator()->NewRegister();
@@ -2977,14 +2976,19 @@ void BytecodeGenerator::VisitAwait(Await* expr) {
   // Resume with "throw" completion (rethrow the received value).
   // TODO(leszeks): Add a debug-only check that the accumulator is
   // JSGeneratorObject::kThrow.
-  builder()->SetExpressionPosition(expr);
   builder()->LoadAccumulatorWithRegister(input).ReThrow();
 
   // Resume with next.
   builder()->Bind(&resume_next);
+  builder()->LoadAccumulatorWithRegister(input);
+}
+
+void BytecodeGenerator::VisitAwait(Await* expr) {
+  builder()->SetExpressionPosition(expr);
+  VisitForAccumulatorValue(expr->expression());
+  BuildAwait(expr->suspend_id());
   BuildIncrementBlockCoverageCounterIfEnabled(expr,
                                               SourceRangeKind::kContinuation);
-  builder()->LoadAccumulatorWithRegister(input);
 }
 
 void BytecodeGenerator::VisitThrow(Throw* expr) {

src/interpreter/bytecode-generator.h

@@ -140,7 +140,8 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   void BuildGeneratorPrologue();
   void BuildSuspendPoint(int suspend_id);
 
-  void BuildAbruptResume(Suspend* expr);
+  void BuildAwait(int suspend_id);
+
   void BuildGetIterator(Expression* iterable, IteratorType hint,
                         FeedbackSlot load_slot, FeedbackSlot call_slot,
                         FeedbackSlot async_load_slot,

src/parsing/parser-base.h

@@ -1341,9 +1341,17 @@ class ParserBase {
   // depending on the current function type.
   inline StatementT BuildReturnStatement(ExpressionT expr, int pos,
                                          int end_pos = kNoSourcePosition) {
-    return is_async_function()
-               ? factory()->NewAsyncReturnStatement(expr, pos, end_pos)
-               : factory()->NewReturnStatement(expr, pos, end_pos);
+    if (impl()->IsEmptyExpression(expr)) {
+      expr = impl()->GetLiteralUndefined(kNoSourcePosition);
+    } else if (is_async_generator()) {
+      // In async generators, if there is an explicit operand to the return
+      // statement, await the operand.
+      expr = factory()->NewAwait(expr, kNoSourcePosition);
+    }
+    if (is_async_function()) {
+      return factory()->NewAsyncReturnStatement(expr, pos, end_pos);
+    }
+    return factory()->NewReturnStatement(expr, pos, end_pos);
   }
 
   // Validation per ES6 object literals.
@@ -2943,6 +2951,12 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseYieldExpression(
     return impl()->RewriteYieldStar(expression, pos);
   }
 
+  if (is_async_generator()) {
+    // Per https://github.com/tc39/proposal-async-iteration/pull/102, the yield
+    // operand must be Await-ed in async generators.
+    expression = factory()->NewAwait(expression, pos);
+  }
+
   // Hackily disambiguate o from o.next and o [Symbol.iterator]().
   // TODO(verwaest): Come up with a better solution.
   ExpressionT yield =
@@ -5221,8 +5235,6 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseReturnStatement(
       tok == Token::RBRACE || tok == Token::EOS) {
     if (IsDerivedConstructor(function_state_->kind())) {
       return_value = impl()->ThisExpression(loc.beg_pos);
-    } else {
-      return_value = impl()->GetLiteralUndefined(position());
     }
   } else {
     return_value = ParseExpression(true, CHECK_OK);

src/parsing/parser.cc

@@ -4526,11 +4526,14 @@ Expression* Parser::RewriteYieldStar(Expression* iterable, int pos) {
       VariableProxy* output_proxy = factory()->NewVariableProxy(var_output);
       Expression* literal = factory()->NewStringLiteral(
           ast_value_factory()->value_string(), nopos);
-      Assignment* assign = factory()->NewAssignment(
-          Token::ASSIGN, output_proxy,
+
+      Expression* value = factory()->NewAwait(
           factory()->NewProperty(factory()->NewVariableProxy(var_output),
                                  literal, nopos),
           nopos);
+
+      Assignment* assign =
+          factory()->NewAssignment(Token::ASSIGN, output_proxy, value, nopos);
       loop_body->statements()->Add(
           factory()->NewExpressionStatement(assign, nopos), zone());
     }