Fix async machinery (issue 22445 and possibly others to be triaged later).

runtime/vm/ast_transformer.cc

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/ast_transformer.h"
 
 #include "vm/object_store.h"
 #include "vm/parser.h"
 #include "vm/thread.h"
 
@@ skipping 27 matching lines @@
 
 #define DEFINE_UNREACHABLE(BaseName)                                           \
 void AwaitTransformer::Visit##BaseName##Node(BaseName##Node* node) {           \
   UNREACHABLE();                                                               \
 }
 
 FOR_EACH_UNREACHABLE_NODE(DEFINE_UNREACHABLE)
 #undef DEFINE_UNREACHABLE
 
 AwaitTransformer::AwaitTransformer(SequenceNode* preamble,
-                                   const ParsedFunction& parsed_function,
                                    LocalScope* function_top)
     : preamble_(preamble),
       temp_cnt_(0),
-      parsed_function_(parsed_function),
       function_top_(function_top),
       thread_(Thread::Current()) {
   ASSERT(function_top_ != NULL);
 }
 
 
 AstNode* AwaitTransformer::Transform(AstNode* expr) {
   expr->Visit(this);
   return result_;
 }
@@ skipping 39 matching lines @@
 void AwaitTransformer::VisitLiteralNode(LiteralNode* node) {
   result_ = node;
 }
 
 
 void AwaitTransformer::VisitTypeNode(TypeNode* node) {
   result_ = new(Z) TypeNode(node->token_pos(), node->type());
 }
 
 
+// Restore the currently relevant :saved_try_context_var on the stack
+// from the captured :async_saved_try_ctx_var_<try_index>.
+// * Try blocks: Set the context variable for this try block.
+// * Catch blocks: Set the context variable for this try block and for any outer
+//                 try block (if existent).
+// * Finally blocks: Set the context variable for any outer try block (if
+//                   existent).

[Ivan Posva, 2015/02/27 10:23:43]

This second half of the comment makes it sound as if the distinction between
try, catch or finally blocks is being done here, but this is just a helper
function.

[regis, 2015/02/27 21:41:53]

I've moved the second half of the comment to Parser::CheckAwaitInTryBlock (to be
renamed).

+AstNode* AwaitTransformer::RestoreSavedTryContext(Zone* zone,
+                                                  LocalScope* scope,
+                                                  int16_t try_index) {
+  LocalVariable* saved_try_ctx =
+      scope->LookupVariable(Symbols::SavedTryContextVar(), false);
+  ASSERT((saved_try_ctx != NULL) && !saved_try_ctx->is_captured());
+  const String& async_saved_try_ctx_name = String::ZoneHandle(zone,
+      Symbols::New(String::Handle(zone,
+          String::NewFormatted("%s%d",
+                               Symbols::AsyncSavedTryCtxVarPrefix().ToCString(),
+                               try_index))));
+  LocalVariable* async_saved_try_ctx =
+      scope->LookupVariable(async_saved_try_ctx_name, false);
+  ASSERT(async_saved_try_ctx != NULL);
+  ASSERT(async_saved_try_ctx->is_captured());
+  return new (zone) StoreLocalNode(
+      Scanner::kNoSourcePos,
+      saved_try_ctx,
+      new (zone) LoadLocalNode(Scanner::kNoSourcePos, async_saved_try_ctx));
+}
+
+
 void AwaitTransformer::VisitAwaitNode(AwaitNode* node) {
   // Await transformation:
   //
   //   :await_temp_var_X = <expr>;
   //   :result_param = :await_temp_var_X;
   //   if (:result_param is !Future) {
   //     :result_param = Future.value(:result_param);
   //   }
   //   AwaitMarker(kNewContinuationState);
   //   :result_param = :result_param.then(:async_op);
@@ skipping 83 matching lines @@
       Scanner::kNoSourcePos, async_op));
   preamble_->Add(new (Z) StaticCallNode(
       Scanner::kNoSourcePos,
       async_catch_helper,
       catch_helper_args));
   ReturnNode* continuation_return = new(Z) ReturnNode(Scanner::kNoSourcePos);
   continuation_return->set_return_type(ReturnNode::kContinuationTarget);
   preamble_->Add(continuation_return);
 
   // If this expression is part of a try block, also append the code for
-  // restoring the saved try context that lives on the stack.
-  const String& async_saved_try_ctx_name =
-      String::Handle(Z, parsed_function_.async_saved_try_ctx_name());
-  if (!async_saved_try_ctx_name.IsNull()) {
-    LocalVariable* async_saved_try_ctx =
-        GetVariableInScope(preamble_->scope(), async_saved_try_ctx_name);
-    preamble_->Add(new (Z) StoreLocalNode(
-        Scanner::kNoSourcePos,
-        parsed_function_.saved_try_ctx(),
-        new (Z) LoadLocalNode(Scanner::kNoSourcePos, async_saved_try_ctx)));
+  // restoring the saved try context that lives on the stack and possibly the
+  // saved try context of the outer try block.
+  if (node->try_scope() != NULL) {
+    preamble_->Add(RestoreSavedTryContext(Z,
+                                          node->try_scope(),

[Florian Schneider, 2015/02/27 11:52:53]

How about passing the correct LocalVariable objects instead the scope and
try_index? Maybe this would simplify the code not to have to repeat the lookup
and constructing of the correct variable name here since it is already done in
the parser.

[regis, 2015/02/27 21:41:53]

I am not sure that all variables are already setup correctly when we construct
the AwaitNode.
But I've added a TODO in Parser::CheckAwaitInTryBlock to check if it would be
possible.

+                                          node->try_index()));
+    if (node->outer_try_scope() != NULL) {
+      preamble_->Add(RestoreSavedTryContext(Z,
+                                            node->outer_try_scope(),
+                                            node->outer_try_index()));
+    }
+  } else {
+    ASSERT(node->outer_try_scope() == NULL);
   }
 
   LoadLocalNode* load_error_param = new (Z) LoadLocalNode(
       Scanner::kNoSourcePos, error_param);
   LoadLocalNode* load_stack_trace_param = new (Z) LoadLocalNode(
       Scanner::kNoSourcePos, stack_trace_param);
   SequenceNode* error_ne_null_branch = new (Z) SequenceNode(
       Scanner::kNoSourcePos, ChainNewScope(preamble_->scope()));
   error_ne_null_branch->Add(new (Z) ThrowNode(
       Scanner::kNoSourcePos,
@@ skipping 364 matching lines @@
 void AwaitTransformer::VisitThrowNode(ThrowNode* node) {
   // TODO(mlippautz): Check if relevant.
   AstNode* new_exception = Transform(node->exception());
   AstNode* new_stacktrace = Transform(node->stacktrace());
   result_ = new(Z) ThrowNode(node->token_pos(),
                              new_exception,
                              new_stacktrace);
 }
 
 }  // namespace dart