[async-await] (simpler) fix for Return in try/finally in async functions

src/ast/ast.h

 // 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.
 
 #ifndef V8_AST_AST_H_
 #define V8_AST_AST_H_
 
 #include "src/ast/ast-types.h"
 #include "src/ast/ast-value-factory.h"
 #include "src/ast/modules.h"
@@ skipping 878 matching lines @@
 
   BreakStatement(BreakableStatement* target, int pos)
       : JumpStatement(pos, kBreakStatement), target_(target) {}
 
   BreakableStatement* target_;
 };
 
 
 class ReturnStatement final : public JumpStatement {
  public:
+  enum Type { kNormal, kHardReturn };

[Dan Ehrenberg, 2017/02/08 21:12:56]

Bikeshedding suggestion: Instead, have normal returns and async returns.
RewriteReturn would turn return statements into async return statements when in
an async function. RewriteReturn only runs for syntactic return statements, not
for ones that come out of desugaring, so this should end up doing the same
thing, right?

[caitp, 2017/02/08 23:22:15]

As explained on IRC:

The key fix for the try/finally issue is to remove the Parser::RewriteReturn()
stuff for async functions.

In place of calling `ResolvePromise(<function_promise>, return_value);` in the
Parser/AST, it's done in BCG::BuildReturn().

But, because this version doesn't move the implicit control flow (top-level
try/catch blocks) into BCG, and the parser still generates the `return
RejectPromise(). <function_promise>;`, or effectively `return
<function_promise>;` at the end of those implicit try/catch blocks. As noted
above, BCG would turn this into `ResolvePromise(<function_promise>,
<function_promise>)`, which is a circular promise resolution.

This "hard return" bit tells the BCG not to transform the return value into a
ResolvePromise() call, so it can avoid the circular promise resolution.

Please let me know if I'm not explaining this clearly.

[Dan Ehrenberg, 2017/02/09 06:46:58]

I see how you need to differentiate between returns that need the promise
behavior and returns that don't in order to avoid introducing a bug, and the
logic seems valid here. Above, I was making a style suggestion about phrasing
that logic--I though the logic I described would end up generating the same
bytecode.

I am still missing something about how this fixes the issue. You end up
desugaring (in the BCG now) to the same thing in some cases. Which cases differ?
I can't find them. Is this all about the implicit 'return undefined' generated
at the end?

   Expression* expression() const { return expression_; }
 
   void set_expression(Expression* e) { expression_ = e; }
+  bool is_hard_return() const { return type_ == kHardReturn; }
 
  private:
   friend class AstNodeFactory;
 
-  ReturnStatement(Expression* expression, int pos)
-      : JumpStatement(pos, kReturnStatement), expression_(expression) {}
+  ReturnStatement(Expression* expression, Type type, int pos)
+      : JumpStatement(pos, kReturnStatement),
+        expression_(expression),
+        type_(type) {}
 
   Expression* expression_;
+  Type type_ = kNormal;
 };
 
 
 class WithStatement final : public Statement {
  public:
   Scope* scope() { return scope_; }
   Expression* expression() const { return expression_; }
   void set_expression(Expression* e) { expression_ = e; }
   Statement* statement() const { return statement_; }
   void set_statement(Statement* s) { statement_ = s; }
@@ skipping 2282 matching lines @@
 
   ContinueStatement* NewContinueStatement(IterationStatement* target, int pos) {
     return new (zone_) ContinueStatement(target, pos);
   }
 
   BreakStatement* NewBreakStatement(BreakableStatement* target, int pos) {
     return new (zone_) BreakStatement(target, pos);
   }
 
   ReturnStatement* NewReturnStatement(Expression* expression, int pos) {
-    return new (zone_) ReturnStatement(expression, pos);
+    return new (zone_)
+        ReturnStatement(expression, ReturnStatement::kNormal, pos);
+  }
+
+  ReturnStatement* NewHardReturnStatement(Expression* expression, int pos) {
+    return new (zone_)
+        ReturnStatement(expression, ReturnStatement::kHardReturn, pos);
   }
 
   WithStatement* NewWithStatement(Scope* scope,
                                   Expression* expression,
                                   Statement* statement,
                                   int pos) {
     return new (zone_) WithStatement(scope, expression, statement, pos);
   }
 
   IfStatement* NewIfStatement(Expression* condition,
@@ skipping 405 matching lines @@
                                     : NULL;                                   \
   }
 AST_NODE_LIST(DECLARE_NODE_FUNCTIONS)
 #undef DECLARE_NODE_FUNCTIONS
 
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_AST_AST_H_