Desugar async/await to create the resulting Promise upfront

src/parsing/parser.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/parsing/parser.h"
 
 #include <memory>
 
 #include "src/api.h"
 #include "src/ast/ast-expression-rewriter.h"
@@ skipping 2617 matching lines @@
         // ES6 14.6.1 Static Semantics: IsInTailPosition
         function_state_->AddImplicitTailCallExpression(return_value);
       }
     }
   }
   ExpectSemicolon(CHECK_OK);
 
   if (is_generator()) {
     return_value = BuildIteratorResult(return_value, true);
   } else if (is_async_function()) {
-    return_value = BuildPromiseResolve(return_value, return_value->position());
+    return_value = BuildResolvePromise(return_value, return_value->position());
   }
 
   result = factory()->NewReturnStatement(return_value, loc.beg_pos);
 
   DeclarationScope* decl_scope = GetDeclarationScope();
   if (decl_scope->is_script_scope() || decl_scope->is_eval_scope()) {
     ReportMessageAt(loc, MessageTemplate::kIllegalReturn);
     *ok = false;
     return NULL;
   }
@@ skipping 1309 matching lines @@
   AddFormalParameter(parameters, expr, initializer, end_pos, is_rest);
 }
 
 void Parser::DesugarAsyncFunctionBody(const AstRawString* function_name,
                                       Scope* scope, ZoneList<Statement*>* body,
                                       ExpressionClassifier* classifier,
                                       FunctionKind kind,
                                       FunctionBodyType body_type,
                                       bool accept_IN, int pos, bool* ok) {
   // function async_function() {
-  //   try {
+  //   BuildRejectPromiseOnException({
   //     .generator_object = %CreateGeneratorObject();

[neis, 2016/08/24 09:48:51]

The .generator_object assignment is actually outside the try block. Can you fix
the comment?

[Dan Ehrenberg, 2016/08/24 18:03:22]

Fixed

   //     ... function body ...
-  //   } catch (e) {
-  //     return Promise.reject(e);
-  //   }
+  //     return %ResolvePromise(.promise, expr), .promise;
+  //   })
   // }
   scope->ForceContextAllocation();
   Variable* temp =
       NewTemporary(ast_value_factory()->dot_generator_object_string());
   function_state_->set_generator_object_variable(temp);
 
   Expression* init_generator_variable = factory()->NewAssignment(
       Token::INIT, factory()->NewVariableProxy(temp),
       BuildCreateJSGeneratorObject(pos, kind), kNoSourcePosition);
   body->Add(factory()->NewExpressionStatement(init_generator_variable,
                                               kNoSourcePosition),
             zone());
 
   Block* try_block = factory()->NewBlock(NULL, 8, true, kNoSourcePosition);

[neis, 2016/08/24 09:48:51]

This shouldn't be called try_block anymore.

 
   ZoneList<Statement*>* inner_body = try_block->statements();

[neis, 2016/08/24 09:48:51]

Can you get rid of inner_body and just inline it?  While reading this code I got
confused several times by thinking inner_body is something else.

 
   Expression* return_value = nullptr;
   if (body_type == FunctionBodyType::kNormal) {
     ParseStatementList(inner_body, Token::RBRACE, CHECK_OK_VOID);
     return_value = factory()->NewUndefinedLiteral(kNoSourcePosition);
   } else {
     return_value =
         ParseAssignmentExpression(accept_IN, classifier, CHECK_OK_VOID);
     RewriteNonPattern(classifier, CHECK_OK_VOID);
   }
 
-  return_value = BuildPromiseResolve(return_value, return_value->position());
+  return_value = BuildResolvePromise(return_value, return_value->position());
   inner_body->Add(
       factory()->NewReturnStatement(return_value, return_value->position()),
       zone());
-  body->Add(BuildRejectPromiseOnException(try_block), zone());
+  Statement* block = BuildRejectPromiseOnException(try_block, CHECK_OK_VOID);
+  body->Add(block, zone());
   scope->set_end_position(scanner()->location().end_pos);
 }
 
 DoExpression* Parser::ParseDoExpression(bool* ok) {
   // AssignmentExpression ::
   //     do '{' StatementList '}'
   int pos = peek_position();
 
   Expect(Token::DO, CHECK_OK);
   Variable* result = NewTemporary(ast_value_factory()->dot_result_string());
@@ skipping 539 matching lines @@
       param_scope = block_state.FinalizedBlockScope();
       if (param_scope != nullptr) {
         CheckConflictingVarDeclarations(param_scope, CHECK_OK);
       }
       init_block->statements()->Add(param_block, zone());
     }
   }
   return init_block;
 }
 
-Block* Parser::BuildRejectPromiseOnException(Block* block) {
-  // try { <block> } catch (error) { return Promise.reject(error); }
-  Block* try_block = block;
+Block* Parser::BuildRejectPromiseOnException(Block* inner_block, bool* ok) {

[neis, 2016/08/24 09:48:51]

s/inner_block/block/ to match the comment.

[Dan Ehrenberg, 2016/08/24 18:03:22]

Changed the name in the other direction

+  // var .promise = %CreatePromise();
+  // var .debug_is_active = %_DebugIsActive() !== 0;

[neis, 2016/08/24 09:48:51]

I think we don't need the !== 0 comparison.  (But why doesn't DebugIsActive
return a boolean in the first place?)

[Dan Ehrenberg, 2016/08/24 18:03:22]

Right, I was considering this but left it out as the existing Promise code did
the comparison. But I don't see the point either, so the new patch removes the
comparison.

+  // try {
+  //   if (.debug_is_active) %DebugPushPromise(.promise);

[neis, 2016/08/24 09:48:51]

Can't we move this out of the try-block?  Seems cleaner and avoids creating
another block.

[Dan Ehrenberg, 2016/08/24 18:03:22]

Done.

+  //   <block>
+  // } catch (.catch) {
+  //   %RejectPromise(.promise, .catch);
+  //   return .promise;
+  // } finally {
+  //   if (.debug_is_active) %DebugPopPromise();
+  // }
+  Block* block = factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);

[jgruber, 2016/08/24 09:16:36]

We could set the initial size to 3 to avoid having to grow the ZoneList.

[neis, 2016/08/24 09:48:51]

s/block/result/ for clarity (and to avoid conflict)

[Dan Ehrenberg, 2016/08/24 18:03:22]

Done, and fixed the initial size to 4.

+
+  // var .promise = %CreatePromise();
+  Statement* set_promise;
+  {
+    DeclareVariable(ast_value_factory()->dot_promise_string(), VAR,

[jgruber, 2016/08/24 09:16:36]

Since I'm not familiar with the parser, a question: Whenever I see dot_* names
used for variables in the parser, NewTemporary() is used instead of
DeclareVariable(). Is that not applicable here?

[Dan Ehrenberg, 2016/08/24 18:03:22]

I could do that, but then I'd have to store the temporary in FunctionState like
.generator_object is. I don't really see the point in going through that
bookkeeping in this case--scope analysis does just fine here.

+                    kNoSourcePosition, CHECK_OK);
+    Expression* create_promise = factory()->NewCallRuntime(
+        Context::PROMISE_CREATE_INDEX,
+        new (zone()) ZoneList<Expression*>(0, zone()), kNoSourcePosition);
+    Assignment* assign_promise = factory()->NewAssignment(
+        Token::INIT, BuildDotPromise(), create_promise, kNoSourcePosition);
+    set_promise =
+        factory()->NewExpressionStatement(assign_promise, kNoSourcePosition);
+  }
+  block->statements()->Add(set_promise, zone());
+
+  // var .debug_is_active = %_DebugIsActive() != 0;

[jgruber, 2016/08/24 09:16:36]

Nit: This differs from !== above.

[Dan Ehrenberg, 2016/08/24 18:03:22]

Removed the comparison, as it is unnecessary.

+  Statement* set_debug_is_active;
+  {
+    DeclareVariable(ast_value_factory()->dot_debug_is_active_string(), VAR,
+                    kNoSourcePosition, CHECK_OK);
+    Expression* debug_is_active_num = factory()->NewCallRuntime(
+        Runtime::kInlineDebugIsActive,
+        new (zone()) ZoneList<Expression*>(0, zone()), kNoSourcePosition);
+    Expression* debug_is_active = factory()->NewCompareOperation(
+        Token::EQ_STRICT, debug_is_active_num,
+        factory()->NewSmiLiteral(0, kNoSourcePosition), kNoSourcePosition);
+    debug_is_active = factory()->NewUnaryOperation(Token::NOT, debug_is_active,
+                                                   kNoSourcePosition);
+    Assignment* assign_debug_is_active = factory()->NewAssignment(
+        Token::INIT, BuildDebugIsActive(), debug_is_active, kNoSourcePosition);
+    set_debug_is_active = factory()->NewExpressionStatement(
+        assign_debug_is_active, kNoSourcePosition);
+  }
+  block->statements()->Add(set_debug_is_active, zone());
+
+  Block* try_block = factory()->NewBlock(nullptr, 2, true, kNoSourcePosition);
+  //   if (.debug_is_active) %DebugPushPromise(.promise);
+  Statement* conditionally_debug_push_promise;
+  {
+    ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
+    args->Add(BuildDotPromise(), zone());
+    Expression* call_push_promise = factory()->NewCallRuntime(
+        Runtime::kDebugPushPromise, args, kNoSourcePosition);
+    Statement* debug_push_promise =
+        factory()->NewExpressionStatement(call_push_promise, kNoSourcePosition);
+    conditionally_debug_push_promise = factory()->NewIfStatement(
+        BuildDebugIsActive(), debug_push_promise,
+        factory()->NewEmptyStatement(kNoSourcePosition), kNoSourcePosition);
+  }
+  try_block->statements()->Add(conditionally_debug_push_promise, zone());
+  try_block->statements()->Add(inner_block, zone());
+
+  // catch (.catch) { return %RejectPromise(.promise, .catch), .promise }
   Scope* catch_scope = NewScope(CATCH_SCOPE);
   catch_scope->set_is_hidden();
   Variable* catch_variable =
       catch_scope->DeclareLocal(ast_value_factory()->dot_catch_string(), VAR,
                                 kCreatedInitialized, Variable::NORMAL);
   Block* catch_block = factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
 
-  Expression* promise_reject = BuildPromiseReject(
+  Expression* promise_reject = BuildRejectPromise(
       factory()->NewVariableProxy(catch_variable), kNoSourcePosition);
-
   ReturnStatement* return_promise_reject =
       factory()->NewReturnStatement(promise_reject, kNoSourcePosition);
   catch_block->statements()->Add(return_promise_reject, zone());
+
   TryStatement* try_catch_statement = factory()->NewTryCatchStatement(
       try_block, catch_scope, catch_variable, catch_block, kNoSourcePosition);
 
-  block = factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
-  block->statements()->Add(try_catch_statement, zone());
+  // There is no TryCatchFinally node, so wrap it in an outer try/finally
+  Block* outer_try_block =
+      factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
+  outer_try_block->statements()->Add(try_catch_statement, zone());
+
+  // finally { if (.debug_is_active) %DebugPopPromise(); }
+  Block* finally_block =
+      factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
+  {
+    ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(0, zone());
+    Expression* call_pop_promise = factory()->NewCallRuntime(
+        Runtime::kDebugPopPromise, args, kNoSourcePosition);
+    Statement* debug_pop_promise =
+        factory()->NewExpressionStatement(call_pop_promise, kNoSourcePosition);
+    Statement* conditionally_debug_pop_promise = factory()->NewIfStatement(
+        BuildDebugIsActive(), debug_pop_promise,
+        factory()->NewEmptyStatement(kNoSourcePosition), kNoSourcePosition);
+    finally_block->statements()->Add(conditionally_debug_pop_promise, zone());
+  }
+
+  Statement* try_finally_statement = factory()->NewTryFinallyStatement(
+      outer_try_block, finally_block, kNoSourcePosition);
+
+  block->statements()->Add(try_finally_statement, zone());
   return block;
 }
 
 Expression* Parser::BuildCreateJSGeneratorObject(int pos, FunctionKind kind) {
   DCHECK_NOT_NULL(function_state_->generator_object_variable());
   ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
   args->Add(factory()->NewThisFunction(pos), zone());
   args->Add(IsArrowFunction(kind) ? GetLiteralUndefined(pos)
                                   : ThisExpression(kNoSourcePosition),
             zone());
   return factory()->NewCallRuntime(Runtime::kCreateJSGeneratorObject, args,
                                    pos);
 }
 
-Expression* Parser::BuildPromiseResolve(Expression* value, int pos) {
-  ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
+Expression* Parser::BuildResolvePromise(Expression* value, int pos) {
+  // %ResolvePromise(.promise, value), .promise
+  ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
+  args->Add(BuildDotPromise(), zone());
   args->Add(value, zone());
-  return factory()->NewCallRuntime(Context::PROMISE_CREATE_RESOLVED_INDEX, args,
-                                   pos);
+  Expression* call_runtime =
+      factory()->NewCallRuntime(Context::PROMISE_RESOLVE_INDEX, args, pos);
+  return factory()->NewBinaryOperation(Token::COMMA, call_runtime,
+                                       BuildDotPromise(), pos);
 }
 
-Expression* Parser::BuildPromiseReject(Expression* value, int pos) {
-  ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
+Expression* Parser::BuildRejectPromise(Expression* value, int pos) {
+  // %RejectPromise(.promise, value, true), .promise
+  // The 'true' flag disables the additional debug event for the rejection
+  // since a debug event already happened for the exception that got us here.

[neis, 2016/08/24 09:48:51]

I think you mean 'false' here, not 'true', but since it doesn't appear in the
AST anymore you don't need to mention the argument.  I'm wondering, though, if
it's really worth having RejectPromiseNoDebugEvent as a seperate definition.

[Dan Ehrenberg, 2016/08/24 18:03:22]

I made it a separate function because RejectPromise is exported to a couple
other places and I didn't want to mess with it, especially once the calling
convention switched to using 'true' to trigger an event (which pre-existing
callers did). Fixed the comment.

+  ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(3, zone());

[jgruber, 2016/08/24 09:16:36]

Should this be 2? There's only two args added.

[Dan Ehrenberg, 2016/08/24 18:03:22]

Fixed

+  args->Add(BuildDotPromise(), zone());
   args->Add(value, zone());
-  return factory()->NewCallRuntime(Context::PROMISE_CREATE_REJECTED_INDEX, args,
-                                   pos);
+  Expression* call_runtime = factory()->NewCallRuntime(
+      Context::PROMISE_REJECT_NO_DEBUG_EVENT_INDEX, args, pos);
+  return factory()->NewBinaryOperation(Token::COMMA, call_runtime,
+                                       BuildDotPromise(), pos);
+}
+
+VariableProxy* Parser::BuildDotPromise() {
+  return NewUnresolved(ast_value_factory()->dot_promise_string(), VAR);
+}
+
+VariableProxy* Parser::BuildDebugIsActive() {

[jgruber, 2016/08/24 09:16:36]

This should be BuildDotDebugIsActive for consistency.

[Dan Ehrenberg, 2016/08/24 18:03:22]

Done

+  return NewUnresolved(ast_value_factory()->dot_debug_is_active_string(), VAR);
 }
 
 ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     const AstRawString* function_name, int pos,
     const ParserFormalParameters& parameters, FunctionKind kind,
     FunctionLiteral::FunctionType function_type, bool* ok) {
   // Everything inside an eagerly parsed function will be parsed eagerly
   // (see comment above).
   ParsingModeScope parsing_mode(this, PARSE_EAGERLY);
   ZoneList<Statement*>* result = new(zone()) ZoneList<Statement*>(8, zone());
@@ skipping 105 matching lines @@
     DCHECK_EQ(function_scope, inner_scope->outer_scope());
     DCHECK_EQ(body, inner_block->statements());
     SetLanguageMode(function_scope, inner_scope->language_mode());
     Block* init_block = BuildParameterInitializationBlock(parameters, CHECK_OK);
 
     if (is_sloppy(inner_scope->language_mode())) {
       InsertSloppyBlockFunctionVarBindings(inner_scope, function_scope,
                                            CHECK_OK);
     }
 
+    // TODO(littledan): Merge the two rejection blocks into one
     if (IsAsyncFunction(kind)) {
-      init_block = BuildRejectPromiseOnException(init_block);
+      init_block = BuildRejectPromiseOnException(init_block, CHECK_OK);
     }
 
     DCHECK_NOT_NULL(init_block);
 
     inner_scope->set_end_position(scanner()->location().end_pos);
     if (inner_scope->FinalizeBlockScope() != nullptr) {
       CheckConflictingVarDeclarations(inner_scope, CHECK_OK);
       InsertShadowingVarBindingInitializers(inner_block);
     }
     inner_scope = nullptr;
@@ skipping 2092 matching lines @@
   node->Print(Isolate::Current());
 }
 #endif  // DEBUG
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8