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

src/interpreter/bytecode-generator.cc

 // Copyright 2015 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/interpreter/bytecode-generator.h"
 
 #include "src/ast/compile-time-value.h"
 #include "src/ast/scopes.h"
 #include "src/builtins/builtins-constructor.h"
 #include "src/code-stubs.h"
@@ skipping 688 matching lines @@
   // since we skip some obviously dead code. Hence the generated bytecode may
   // contain jumps to unbound labels (resume points that will never be used).
   // We bind these now.
   for (auto& label : generator_resume_points_) {
     if (!label.is_bound()) builder()->Bind(&label);
   }
 
   // Emit an implicit return instruction in case control flow can fall off the
   // end of the function without an explicit return being present on all paths.
   if (builder()->RequiresImplicitReturn()) {
+    DCHECK(execution_control() == &control);

[rmcilroy, 2017/02/08 12:21:56]

nit - please add a comment here that we can do a hard return since we know that
the execution_control is the top level and doesn't need to defer the return.

[caitp, 2017/02/08 13:42:58]

Done.

     builder()->LoadUndefined();
-    BuildReturn();
+    BuildTraceExit();
+    builder()->Return();
   }
   DCHECK(!builder()->RequiresImplicitReturn());
 }
 
 void BytecodeGenerator::GenerateBytecodeBody() {
   // Build the arguments object if it is used.
   VisitArgumentsObject(scope()->arguments());
 
   // Build rest arguments array if it is used.
   Variable* rest_parameter = scope()->rest_parameter();
@@ skipping 330 matching lines @@
 }
 
 void BytecodeGenerator::VisitBreakStatement(BreakStatement* stmt) {
   builder()->SetStatementPosition(stmt);
   execution_control()->Break(stmt->target());
 }
 
 void BytecodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
   builder()->SetStatementPosition(stmt);
   VisitForAccumulatorValue(stmt->expression());
+
+  if (stmt->is_hard_return()) {
+    // TODO(caitp): remove hard_return handling and ReturnStatement AST node
+    // flags once BytecodeGenerator performs all implicit control flow for
+    // async functions.
+    builder()->Return();

[rmcilroy, 2017/02/08 12:21:56]

BuildTraceExit here too?  How about Creating a BuildHardReturn() function which
does the trace exit and builder()->Return and call it here, in the implicit
return and the BuildReturn call (in which case, you probably wouldn't need the
extra BuildTraceExit function).

[caitp, 2017/02/08 13:42:58]

That looks cleaner, nice. Done

+    return;
+  }
+
   execution_control()->ReturnAccumulator();
 }
 
 void BytecodeGenerator::VisitWithStatement(WithStatement* stmt) {
   builder()->SetStatementPosition(stmt);
   VisitForAccumulatorValue(stmt->expression());
   BuildNewLocalWithContext(stmt->scope());
   VisitInScope(stmt->statement(), stmt->scope());
 }
 
@@ skipping 913 matching lines @@
 }
 
 void BytecodeGenerator::BuildVariableLoadForAccumulatorValue(
     Variable* variable, FeedbackSlot slot, HoleCheckMode hole_check_mode,
     TypeofMode typeof_mode) {
   ValueResultScope accumulator_result(this);
   BuildVariableLoad(variable, slot, hole_check_mode, typeof_mode);
 }
 
 void BytecodeGenerator::BuildReturn() {
+  if (IsAsyncFunction(info()->literal()->kind())) {
+    RegisterAllocationScope register_scope(this);
+    RegisterList args = register_allocator()->NewRegisterList(3);
+    Register receiver = args[0];
+    Register promise = args[1];
+    Register return_value = args[2];
+    builder()->StoreAccumulatorInRegister(return_value);
+
+    Variable* var_promise = scope()->promise_var();
+    DCHECK_NOT_NULL(var_promise);
+    BuildVariableLoad(var_promise, FeedbackSlot::Invalid(),
+                      HoleCheckMode::kElided);
+    builder()
+        ->StoreAccumulatorInRegister(promise)
+        .LoadUndefined()
+        .StoreAccumulatorInRegister(receiver)
+        .CallJSRuntime(Context::PROMISE_RESOLVE_INDEX, args)
+        .LoadAccumulatorWithRegister(promise);
+  }
+
+  BuildTraceExit();
+  builder()->Return();
+}
+
+void BytecodeGenerator::BuildTraceExit() {
   if (FLAG_trace) {
     RegisterAllocationScope register_scope(this);
     Register result = register_allocator()->NewRegister();
     // Runtime returns {result} value, preserving accumulator.
     builder()->StoreAccumulatorInRegister(result).CallRuntime(
         Runtime::kTraceExit, result);
   }
-  builder()->Return();
 }
 
 void BytecodeGenerator::BuildReThrow() { builder()->ReThrow(); }
 
 void BytecodeGenerator::BuildAbort(BailoutReason bailout_reason) {
   RegisterAllocationScope register_scope(this);
   Register reason = register_allocator()->NewRegister();
   builder()
       ->LoadLiteral(Smi::FromInt(static_cast<int>(bailout_reason)))
       .StoreAccumulatorInRegister(reason)
@@ skipping 1369 matching lines @@
 }
 
 Runtime::FunctionId BytecodeGenerator::StoreKeyedToSuperRuntimeId() {
   return is_strict(language_mode()) ? Runtime::kStoreKeyedToSuper_Strict
                                     : Runtime::kStoreKeyedToSuper_Sloppy;
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8