Simplify the desugaring of catch clauses.

runtime/vm/parser.cc

 // Copyright (c) 2012, 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/parser.h"
 
 #include "lib/invocation_mirror.h"
-#include "vm/bigint_operations.h"
+#include "platform/utils.h"
 #include "vm/bootstrap.h"
 #include "vm/class_finalizer.h"
 #include "vm/compiler.h"
 #include "vm/compiler_stats.h"
 #include "vm/dart_api_impl.h"
 #include "vm/dart_entry.h"
 #include "vm/flags.h"
 #include "vm/growable_array.h"
+#include "vm/handles.h"
+#include "vm/heap.h"
+#include "vm/isolate.h"
 #include "vm/longjump.h"
+#include "vm/native_arguments.h"
 #include "vm/native_entry.h"
 #include "vm/object.h"
 #include "vm/object_store.h"
+#include "vm/os.h"
 #include "vm/resolver.h"
+#include "vm/scanner.h"
 #include "vm/scopes.h"
 #include "vm/stack_frame.h"
 #include "vm/symbols.h"
+#include "vm/timer.h"
+#include "vm/zone.h"
 
 namespace dart {
 
 DEFINE_FLAG(bool, enable_asserts, false, "Enable assert statements.");
 DEFINE_FLAG(bool, enable_type_checks, false, "Enable type checks.");
 DEFINE_FLAG(bool, trace_parser, false, "Trace parser operations.");
 DEFINE_FLAG(bool, warning_as_error, false, "Treat warnings as errors.");
 DEFINE_FLAG(bool, silent_warnings, false, "Silence warnings.");
 DEFINE_FLAG(bool, warn_mixin_typedef, true, "Warning on legacy mixin typedef");
 DECLARE_FLAG(bool, error_on_bad_type);
@@ skipping 152 matching lines @@
 
 // Class which describes an inlined finally block which is used to generate
 // inlined code for the finally blocks when there is an exit from a try
 // block using 'return', 'break' or 'continue'.
 class Parser::TryBlocks : public ZoneAllocated {
  public:
   TryBlocks(Block* try_block, TryBlocks* outer_try_block, intptr_t try_index)
       : try_block_(try_block),
         inlined_finally_nodes_(),
         outer_try_block_(outer_try_block),
-        try_index_(try_index) { }
+        try_index_(try_index),
+        inside_catch_(false) { }
 
   TryBlocks* outer_try_block() const { return outer_try_block_; }
   Block* try_block() const { return try_block_; }
   intptr_t try_index() const { return try_index_; }
+  bool inside_catch() const { return inside_catch_; }
+  void enter_catch() { inside_catch_ = true; }
 
   void AddNodeForFinallyInlining(AstNode* node);
   AstNode* GetNodeToInlineFinally(int index) {
     if (0 <= index && index < inlined_finally_nodes_.length()) {
       return inlined_finally_nodes_[index];
     }
     return NULL;
   }
 
  private:
   Block* try_block_;
   GrowableArray<AstNode*> inlined_finally_nodes_;
   TryBlocks* outer_try_block_;
   const intptr_t try_index_;
+  bool inside_catch_;
 
   DISALLOW_COPY_AND_ASSIGN(TryBlocks);
 };
 
 
 void Parser::TryBlocks::AddNodeForFinallyInlining(AstNode* node) {
   inlined_finally_nodes_.Add(node);
 }
 
 
@@ skipping 839 matching lines @@
   }
   LocalVariable* catch_excp_var =
       current_block_->scope->LocalLookupVariable(Symbols::ExceptionVar());
   if (catch_excp_var == NULL) {
     catch_excp_var = new LocalVariable(token_pos,
                                        Symbols::ExceptionVar(),
                                        Type::ZoneHandle(Type::DynamicType()));
     current_block_->scope->AddVariable(catch_excp_var);
   }
   LocalVariable* catch_trace_var =
-      current_block_->scope->LocalLookupVariable(Symbols::StacktraceVar());
+      current_block_->scope->LocalLookupVariable(Symbols::StackTraceVar());
   if (catch_trace_var == NULL) {
     catch_trace_var = new LocalVariable(token_pos,
-                                        Symbols::StacktraceVar(),
+                                        Symbols::StackTraceVar(),
                                         Type::ZoneHandle(Type::DynamicType()));
     current_block_->scope->AddVariable(catch_trace_var);
   }
 
   OpenBlock();  // Start try block.
   AstNode* expr = ParseExpr(kAllowConst, kConsumeCascades);
   const Field& field = Field::ZoneHandle(func.saved_static_field());
   ASSERT(!field.is_const());
   StoreStaticFieldNode* store = new StoreStaticFieldNode(field.token_pos(),
                                                          field,
                                                          expr);
   current_block_->statements->Add(store);
   SequenceNode* try_block = CloseBlock();  // End try block.
 
   OpenBlock();  // Start catch handler list.
-  SourceLabel* end_catch_label =
-      SourceLabel::New(token_pos, NULL, SourceLabel::kCatch);
-  current_block_->scope->AddLabel(end_catch_label);
-
   OpenBlock();  // Start catch clause.
   AstNode* compare_transition_sentinel = new ComparisonNode(
       token_pos,
       Token::kEQ_STRICT,
       new LoadStaticFieldNode(token_pos, field),
       new LiteralNode(field.token_pos(), Object::transition_sentinel()));
 
   SequenceNode* store_null = new SequenceNode(token_pos, NULL);
   store_null->Add(new StoreStaticFieldNode(
       field.token_pos(),
       field,
       new LiteralNode(token_pos, Instance::ZoneHandle())));
   AstNode* transition_sentinel_check =
       new IfNode(token_pos, compare_transition_sentinel, store_null, NULL);
   current_block_->statements->Add(transition_sentinel_check);
 
   current_block_->statements->Add(
       new ThrowNode(token_pos,
                     new LoadLocalNode(token_pos, catch_excp_var),
                     new LoadLocalNode(token_pos, catch_trace_var)));
-  current_block_->statements->Add(
-      new JumpNode(token_pos, Token::kCONTINUE, end_catch_label));
   SequenceNode* catch_clause = CloseBlock();  // End catch clause.
 
   current_block_->statements->Add(catch_clause);
   SequenceNode* catch_handler_list = CloseBlock();  // End catch handler list.
   CatchClauseNode* catch_block =
       new CatchClauseNode(token_pos,
                           catch_handler_list,
                           Array::ZoneHandle(Object::empty_array().raw()),
                           context_var,
                           catch_excp_var,
                           catch_trace_var,
                           CatchClauseNode::kInvalidTryIndex,
                           false);  // No stack trace needed.
 
   AstNode* try_catch_node = new TryCatchNode(token_pos,
                                              try_block,
-                                             end_catch_label,
                                              context_var,
                                              catch_block,
                                              NULL,  // No finally block.
                                              AllocateTryIndex());
   current_block_->statements->Add(try_catch_node);
   return CloseBlock();
 }
 
 
 // Create AstNodes for an implicit instance getter method:
@@ skipping 5491 matching lines @@
   condition = InsertClosureCallNodes(condition);
   condition = new UnaryOpNode(condition_pos, Token::kNOT, condition);
   AstNode* assert_throw = MakeAssertCall(condition_pos, condition_end);
   return new IfNode(condition_pos,
                     condition,
                     NodeAsSequenceNode(condition_pos, assert_throw, NULL),
                     NULL);
 }
 
 
-struct CatchParamDesc {
-  CatchParamDesc()
-      : token_pos(0), type(NULL), var(NULL) { }
+struct CatchParameter {

[hausner, 2013/11/08 21:52:30]

As I've aked in previous rounds of this review, please do not rename this.

[Kevin Millikin (Google), 2013/11/11 11:39:57]

CatchParamDesc is not a very good name.  First, what is a desc?  Description? 
Descriptor?  Probably not descendant.  The style guide is clear to 'do not use
abbreviations that are ambiguous...'.

But second, whatever desc is, it's a noise word.  It doesn't contribute anything
to the meaning of the identifier.  A distinction could be made between a
representation of a catch parameter and a representation of a
description/descriptor of a catch parameter but it's much too subtle to matter
here.  (And besides, how much more concrete would you expect this struct to be
for it to represent the parameter itself?)

+  CatchParameter()
+      : token_pos(0), type(NULL), name(NULL), var(NULL) { }
   intptr_t token_pos;
   const AbstractType* type;
-  const String* var;
+  const String* name;
+  LocalVariable* var;
 };
 
 
 // Populate local scope of the catch block with the catch parameters.
-void Parser::AddCatchParamsToScope(const CatchParamDesc& exception_param,
-                                   const CatchParamDesc& stack_trace_param,
-                                   LocalScope* scope) {
-  if (exception_param.var != NULL) {
-    LocalVariable* var = new LocalVariable(exception_param.token_pos,
-                                           *exception_param.var,
-                                           *exception_param.type);
+// Return false if the stack trace parameter shadows the exception
+// parameter.
+static bool AddCatchParamsToScope(CatchParameter* exception_param,

[hausner, 2013/11/08 21:52:30]

Please leave this as a method of the parser and keep the error handling where it
is now.

[Kevin Millikin (Google), 2013/11/11 11:39:57]

Why?  The reason I moved it is so we could see the error handling code when
reading the caller, instead of buried in the helper.

For a helper with exactly one call site, it otherwise isn't especially vital
whether the caller or callee does the error handling, is it?

+                                  CatchParameter* stack_trace_param,
+                                  LocalScope* scope) {
+  if (exception_param->name != NULL) {
+    LocalVariable* var = new LocalVariable(exception_param->token_pos,
+                                           *exception_param->name,
+                                           *exception_param->type);
     var->set_is_final();
     bool added_to_scope = scope->AddVariable(var);
     ASSERT(added_to_scope);
+    exception_param->var = var;
   }
-  if (stack_trace_param.var != NULL) {
-    LocalVariable* var = new LocalVariable(TokenPos(),
-                                           *stack_trace_param.var,
-                                           *stack_trace_param.type);
+  if (stack_trace_param->name != NULL) {
+    LocalVariable* var = new LocalVariable(stack_trace_param->token_pos,
+                                           *stack_trace_param->name,
+                                           *stack_trace_param->type);
     var->set_is_final();
     bool added_to_scope = scope->AddVariable(var);
-    if (!added_to_scope) {
-      ErrorMsg(stack_trace_param.token_pos,
-               "name '%s' already exists in scope",
-               stack_trace_param.var->ToCString());
-    }
+    stack_trace_param->var = var;
+    return added_to_scope;
   }
+  return true;
 }
 
 
 SequenceNode* Parser::ParseFinallyBlock() {
   TRACE_PARSER("ParseFinallyBlock");
   OpenBlock();
   ExpectToken(Token::kLBRACE);
   ParseStatementSequence();
   ExpectToken(Token::kRBRACE);
   SequenceNode* finally_block = CloseBlock();
@@ skipping 47 matching lines @@
                                    InlinedFinallyNode* finally_node) {
   if (node->IsReturnNode()) {
     node->AsReturnNode()->AddInlinedFinallyNode(finally_node);
   } else {
     ASSERT(node->IsJumpNode());
     node->AsJumpNode()->AddInlinedFinallyNode(finally_node);
   }
 }
 
 
-AstNode* Parser::ParseTryStatement(String* label_name) {
-  TRACE_PARSER("ParseTryStatement");
-
-  // We create three stack slots for exceptions here:
-  // ':saved_try_context_var' - Used to save the context before start of the try
-  //                            block. The context register is restored from
-  //                            this slot before processing the catch block
-  //                            handler.
-  // ':exception_var' - Used to save the current exception object that was
-  //                    thrown.
-  // ':stacktrace_var' - Used to save the current stack trace object into which
-  //                     the stack trace was copied into when an exception was
-  //                     thrown.
-  // :exception_var and :stacktrace_var get set with the exception object
-  // and the stacktrace object when an exception is thrown.
-  // These three implicit variables can never be captured variables.
-  LocalVariable* context_var =
-      current_block_->scope->LocalLookupVariable(Symbols::SavedTryContextVar());
-  if (context_var == NULL) {
-    context_var = new LocalVariable(TokenPos(),
-                                    Symbols::SavedTryContextVar(),
-                                    Type::ZoneHandle(Type::DynamicType()));
-    current_block_->scope->AddVariable(context_var);
-  }
-  LocalVariable* catch_excp_var =
-      current_block_->scope->LocalLookupVariable(Symbols::ExceptionVar());
-  if (catch_excp_var == NULL) {
-    catch_excp_var = new LocalVariable(TokenPos(),
-                                       Symbols::ExceptionVar(),
-                                       Type::ZoneHandle(Type::DynamicType()));
-    current_block_->scope->AddVariable(catch_excp_var);
-  }
-  LocalVariable* catch_trace_var =
-      current_block_->scope->LocalLookupVariable(Symbols::StacktraceVar());
-  if (catch_trace_var == NULL) {
-    catch_trace_var = new LocalVariable(TokenPos(),
-                                        Symbols::StacktraceVar(),
-                                        Type::ZoneHandle(Type::DynamicType()));
-    current_block_->scope->AddVariable(catch_trace_var);
-  }
-
-  const intptr_t try_pos = TokenPos();
-  ConsumeToken();  // Consume the 'try'.
-
-  SourceLabel* try_label = NULL;
-  if (label_name != NULL) {
-    try_label = SourceLabel::New(try_pos, label_name, SourceLabel::kStatement);
+SequenceNode* Parser::ParseCatchClauses(
+    intptr_t handler_pos,
+    LocalVariable* exception_var,
+    LocalVariable* stack_trace_var,
+    const GrowableObjectArray& handler_types,
+    bool* needs_stack_trace) {
+  // All catch blocks are merged into an if-then-else sequence of the
+  // different types specified using the 'is' operator.  While parsing
+  // record the type tests (or NULL for an generic catch) and the catch
+  // bodies in a pair of parallel lists.  Afterward, construct the nested
+  // if-then-else.
+  GrowableArray<ComparisonNode*> type_tests;
+  GrowableArray<SequenceNode*> catch_blocks;
+  while ((CurrentToken() == Token::kCATCH) || IsLiteral("on")) {
+    // Open a block that contains the if or an unconditional body.  It's
+    // closed in the loop that builds the if-then-else nest.
     OpenBlock();
-    current_block_->scope->AddLabel(try_label);
-  }
-
-  // Now parse the 'try' block.
-  OpenBlock();
-  PushTryBlock(current_block_);
-  ExpectToken(Token::kLBRACE);
-  ParseStatementSequence();
-  ExpectToken(Token::kRBRACE);
-  SequenceNode* try_block = CloseBlock();
-
-  if ((CurrentToken() != Token::kCATCH) && !IsLiteral("on") &&
-      (CurrentToken() != Token::kFINALLY)) {
-    ErrorMsg("catch or finally clause expected");
-  }
-
-  // Now create a label for the end of catch block processing so that we can
-  // jump over the catch block code after executing the try block.
-  SourceLabel* end_catch_label =
-      SourceLabel::New(TokenPos(), NULL, SourceLabel::kCatch);
-
-  // Now parse the 'catch' blocks if any and merge all of them into
-  // an if-then sequence of the different types specified using the 'is'
-  // operator.
-  bool generic_catch_seen = false;
-  const intptr_t handler_pos = TokenPos();
-  OpenBlock();  // Start the catch block sequence.
-  current_block_->scope->AddLabel(end_catch_label);
-  const GrowableObjectArray& handler_types =
-      GrowableObjectArray::Handle(GrowableObjectArray::New());
-  bool needs_stacktrace = false;
-  while ((CurrentToken() == Token::kCATCH) || IsLiteral("on")) {
     const intptr_t catch_pos = TokenPos();
-    CatchParamDesc exception_param;
-    CatchParamDesc stack_trace_param;
+    CatchParameter exception_param;
+    CatchParameter stack_trace_param;
     if (IsLiteral("on")) {
       ConsumeToken();
       exception_param.type = &AbstractType::ZoneHandle(
           ParseType(ClassFinalizer::kCanonicalize));
     } else {
       exception_param.type = &AbstractType::ZoneHandle(Type::DynamicType());
     }
     if (CurrentToken() == Token::kCATCH) {
       ConsumeToken();  // Consume the 'catch'.
       ExpectToken(Token::kLPAREN);
       exception_param.token_pos = TokenPos();
-      exception_param.var = ExpectIdentifier("identifier expected");
+      exception_param.name = ExpectIdentifier("identifier expected");
       if (CurrentToken() == Token::kCOMMA) {
         ConsumeToken();
         // TODO(hausner): Make implicit type be StackTrace, not dynamic.
         stack_trace_param.type =
             &AbstractType::ZoneHandle(Type::DynamicType());
         stack_trace_param.token_pos = TokenPos();
-        stack_trace_param.var = ExpectIdentifier("identifier expected");
+        stack_trace_param.name = ExpectIdentifier("identifier expected");
       }
       ExpectToken(Token::kRPAREN);
     }
 
-    // Create a block containing the catch clause parameters and
-    // the following code:
+    // Create a block containing the catch clause parameters and the
+    // following code:
     // 1) Store exception object and stack trace object into user-defined
     //    variables (as needed).
     // 2) Nested block with source code from catch clause block.
-    // 3) Unconditional JUMP to the end of the try block.
     OpenBlock();
-    AddCatchParamsToScope(exception_param,
-                          stack_trace_param,
-                          current_block_->scope);
+    if (!AddCatchParamsToScope(&exception_param, &stack_trace_param,
+                               current_block_->scope)) {
+      ErrorMsg(stack_trace_param.token_pos,
+               "name '%s' already exists in scope",
+               stack_trace_param.name->ToCString());
+    }
 
     if (exception_param.var != NULL) {
       // Generate code to load the exception object (:exception_var) into
       // the exception variable specified in this block.
-      LocalVariable* var = LookupLocalScope(*exception_param.var);
-      ASSERT(var != NULL);
-      ASSERT(catch_excp_var != NULL);
-      current_block_->statements->Add(
-          new StoreLocalNode(catch_pos, var,
-                             new LoadLocalNode(catch_pos, catch_excp_var)));
+      ASSERT(exception_var != NULL);
+      current_block_->statements->Add(
+          new StoreLocalNode(catch_pos, exception_param.var,
+                             new LoadLocalNode(catch_pos, exception_var)));
     }
     if (stack_trace_param.var != NULL) {
       // A stack trace variable is specified in this block, so generate code
-      // to load the stack trace object (:stacktrace_var) into the stack trace
-      // variable specified in this block.
-      needs_stacktrace = true;
+      // to load the stack trace object (:stack_trace_var) into the stack
+      // trace variable specified in this block.
+      *needs_stack_trace = true;
       ArgumentListNode* no_args = new ArgumentListNode(catch_pos);
-      LocalVariable* trace = LookupLocalScope(*stack_trace_param.var);
-      ASSERT(catch_trace_var != NULL);
-      current_block_->statements->Add(
-          new StoreLocalNode(catch_pos, trace,
-                             new LoadLocalNode(catch_pos, catch_trace_var)));
+      ASSERT(stack_trace_var != NULL);
+      current_block_->statements->Add(
+          new StoreLocalNode(catch_pos, stack_trace_param.var,
+                             new LoadLocalNode(catch_pos, stack_trace_var)));
       current_block_->statements->Add(
           new InstanceCallNode(
               catch_pos,
-              new LoadLocalNode(catch_pos, trace),
+              new LoadLocalNode(catch_pos, stack_trace_param.var),
               Library::PrivateCoreLibName(Symbols::_setupFullStackTrace()),
               no_args));
     }
 
-    // Add nested block with user-defined code.
+    // Add nested block with user-defined code.  This blocks allows
+    // declarations in the body to shadow the catch parameters.
     CheckToken(Token::kLBRACE);
     current_block_->statements->Add(ParseNestedStatement(false, NULL));
-
-    // Add unconditional jump to end of catch clause list.
-    current_block_->statements->Add(
-        new JumpNode(catch_pos, Token::kCONTINUE, end_catch_label));
-
-    SequenceNode* catch_clause = CloseBlock();
-
-    const bool is_bad_type = exception_param.type->IsMalformed() ||
-                             exception_param.type->IsMalbounded();
-    if (!is_bad_type && !exception_param.type->IsDynamicType()) {
-      // Has a type specification that is not malformed or malbounded.
-      // Now form an 'if type check' to guard the catch handler code.
+    catch_blocks.Add(CloseBlock());
+
+    const bool is_bad_type =
+        exception_param.type->IsMalformed() ||
+        exception_param.type->IsMalbounded();
+    if (exception_param.type->IsDynamicType() || is_bad_type) {
+      // There is no exception type or else it is malformed or malbounded.
+      // In the first cast, unconditionally execute the catch body.  In the
+      // second case, unconditionally throw.
+      type_tests.Add(NULL);
+      if (is_bad_type) {
+        // Replace the body with one that throws.
+        SequenceNode* block = new SequenceNode(catch_pos, NULL);
+        block->Add(ThrowTypeError(catch_pos, *exception_param.type));
+        catch_blocks.Last() = block;
+      }
+      // This catch clause will handle all exceptions. We can safely forget
+      // all previous catch clause types.
+      handler_types.SetLength(0);
+      handler_types.Add(*exception_param.type);
+    } else {
+      // Has a type specification that is not malformed or malbounded.  Now
+      // form an 'if type check' to guard the catch handler code.
       if (!exception_param.type->IsInstantiated() &&
           (current_block_->scope->function_level() > 0)) {
         // Make sure that the instantiator is captured.
         CaptureInstantiator();
       }
       TypeNode* exception_type = new TypeNode(catch_pos, *exception_param.type);
-      AstNode* exception_var = new LoadLocalNode(catch_pos, catch_excp_var);
+      AstNode* exception_value = new LoadLocalNode(catch_pos, exception_var);
       if (!exception_type->type().IsInstantiated()) {
         EnsureExpressionTemp();
       }
-      AstNode* type_cond_expr = new ComparisonNode(
-          catch_pos, Token::kIS, exception_var, exception_type);
-      current_block_->statements->Add(
-          new IfNode(catch_pos, type_cond_expr, catch_clause, NULL));
-
-      // Do not add uninstantiated types (e.g. type parameter T or
-      // generic type List<T>), since the debugger won't be able to
-      // instantiate it when walking the stack.
-      // This means that the debugger is not able to determine whether
-      // an exception is caught if the catch clause uses generic types.
-      // It will report the exception as uncaught when in fact it might
-      // be caught and handled when we unwind the stack.
+      type_tests.Add(new ComparisonNode(catch_pos, Token::kIS, exception_value,
+                                        exception_type));
+
+      // Do not add uninstantiated types (e.g. type parameter T or generic
+      // type List<T>), since the debugger won't be able to instantiate it
+      // when walking the stack.
+      //
+      // This means that the debugger is not able to determine whether an
+      // exception is caught if the catch clause uses generic types.  It
+      // will report the exception as uncaught when in fact it might be
+      // caught and handled when we unwind the stack.
       if (exception_param.type->IsInstantiated()) {
         handler_types.Add(*exception_param.type);
       }
+    }
+
+    ASSERT(type_tests.length() == catch_blocks.length());
+  }
+
+  // If the exception is not caught by any clause here, rethrow it.  This is
+  // the last (innermost) else block if it turns out to be reachable.
+  SequenceNode* current = new SequenceNode(handler_pos, NULL);
+  current->Add(new ThrowNode(handler_pos,
+                             new LoadLocalNode(handler_pos, exception_var),
+                             new LoadLocalNode(handler_pos, stack_trace_var)));
+  // Build the if/then/else nest from the inside out.
+  while (!type_tests.is_empty()) {
+    ComparisonNode* type_test = type_tests.RemoveLast();
+    SequenceNode* catch_block = catch_blocks.RemoveLast();
+    if (type_test == NULL) {
+      // Unconditional catch, forget the clauses seen so far.
+      current_block_->statements->Add(catch_block);
     } else {
-      if (is_bad_type) {
-        current_block_->statements->Add(ThrowTypeError(catch_pos,
-                                                       *exception_param.type));
-        // We still add the dead code below to satisfy the code generator.
-      }
-      // No exception type exists in the catch clause so execute the
-      // catch handler code unconditionally.
-      current_block_->statements->Add(catch_clause);
-      generic_catch_seen = true;
-      // This catch clause will handle all exceptions. We can safely forget
-      // all previous catch clause types.
-      handler_types.SetLength(0);
-      handler_types.Add(*exception_param.type);
-    }
-  }
-
-  SequenceNode* catch_handler_list = CloseBlock();
+      current_block_->statements->Add(
+          new IfNode(type_test->token_pos(), type_test, catch_block, current));
+    }
+    current = CloseBlock();

[hausner, 2013/11/08 21:52:30]

I don't think the way you nest the blocks is correct. These blocks need to be
siblings, not nested. Take this Dart snippet:

try {
...
} on T1 catch (e1) {
   S1;
} on T2 catch (e2) {
  S2;
}

The way you nest blocks, variable e1 would be visible in the block containing
statement S2.

I think you can eliminate the need for the catch_blocks and type_tests array.
Create the Ast nodes as you parse, remember the previous SequenceNode of the
else-branch, and add the next if statement in the following loop iteration (or
the throw node after the loop terminates.)


 

[Kevin Millikin (Google), 2013/11/11 11:39:57]

I don't think that's right.  The intent is to represent this as:

{
  if (:exception_var is T1) {
    var e1 = :exception_var;
    { S1; }
  } else if (:exception_var is T2) {
    var e2 = :exception-var;
    { S2; }
  }
}

I'll add a test.
Let me see if I understand.  You suggest that though the nesting is actually:

if (test0) {
  parameters0
  { body0 }
} else {
  if (test1) {
    parameters1
    { body1 }
  }
}

I could parse it as if the block containing { parameters1 { body1 }} were not
really inside the else block (as far as the parser's current_block_ stack is
concerned)?  Is the reason that's safe because we know that this block doesn't
contain any bindings and is not labeled, and we don't use the current_block_
stack for anything else?  I was assuming that it was important to keep the
parser's current_block_ stack consistent with the AST we were building, but
maybe we already violate that in other places so I can go ahead and do it here.

If you think it's safe and want me to make this change, I will.

Note that the code as written is just a literal translation of the recursive
implementation with the first loop doing the work before the recursive
(non-tail) call, and the second loop doing the work after the return from the
recursive call.

+  }
+  return current;
+}
+
+
+AstNode* Parser::ParseTryStatement(String* label_name) {
+  TRACE_PARSER("ParseTryStatement");
+
+  // We create three variables for exceptions here:
+  // ':saved_try_context_var' - Used to save the context before the start of
+  //                            the try block. The context register is
+  //                            restored from this variable before
+  //                            processing the catch block handler.
+  // ':exception_var' - Used to save the current exception object that was
+  //                    thrown.
+  // ':stack_trace_var' - Used to save the current stack trace object which
+  //                      the stack trace was copied into when an exception
+  //                      was thrown.
+  // :exception_var and :stack_trace_var get set with the exception object
+  // and the stack trace object when an exception is thrown.  These three
+  // implicit variables can never be captured.
+  LocalVariable* context_var =
+      current_block_->scope->LocalLookupVariable(Symbols::SavedTryContextVar());
+  if (context_var == NULL) {
+    context_var = new LocalVariable(TokenPos(),
+                                    Symbols::SavedTryContextVar(),
+                                    Type::ZoneHandle(Type::DynamicType()));
+    current_block_->scope->AddVariable(context_var);
+  }
+  LocalVariable* exception_var =
+      current_block_->scope->LocalLookupVariable(Symbols::ExceptionVar());
+  if (exception_var == NULL) {
+    exception_var = new LocalVariable(TokenPos(),
+                                      Symbols::ExceptionVar(),
+                                      Type::ZoneHandle(Type::DynamicType()));
+    current_block_->scope->AddVariable(exception_var);
+  }
+  LocalVariable* stack_trace_var =
+      current_block_->scope->LocalLookupVariable(Symbols::StackTraceVar());
+  if (stack_trace_var == NULL) {
+    stack_trace_var = new LocalVariable(TokenPos(),
+                                        Symbols::StackTraceVar(),
+                                        Type::ZoneHandle(Type::DynamicType()));
+    current_block_->scope->AddVariable(stack_trace_var);
+  }
+
+  const intptr_t try_pos = TokenPos();
+  ConsumeToken();  // Consume the 'try'.
+
+  SourceLabel* try_label = NULL;
+  if (label_name != NULL) {
+    try_label = SourceLabel::New(try_pos, label_name, SourceLabel::kStatement);
+    OpenBlock();
+    current_block_->scope->AddLabel(try_label);
+  }
+
+  // Now parse the 'try' block.
+  OpenBlock();
+  PushTryBlock(current_block_);
+  ExpectToken(Token::kLBRACE);
+  ParseStatementSequence();
+  ExpectToken(Token::kRBRACE);
+  SequenceNode* try_block = CloseBlock();
+
+  if ((CurrentToken() != Token::kCATCH) && !IsLiteral("on") &&
+      (CurrentToken() != Token::kFINALLY)) {
+    ErrorMsg("catch or finally clause expected");
+  }
+
+  // Now parse the 'catch' blocks if any.
+  try_blocks_list_->enter_catch();
+  const intptr_t handler_pos = TokenPos();
+  const GrowableObjectArray& handler_types =
+      GrowableObjectArray::Handle(GrowableObjectArray::New());
+  bool needs_stack_trace = false;
+  SequenceNode* catch_handler_list =
+      ParseCatchClauses(handler_pos, exception_var, stack_trace_var,
+                        handler_types, &needs_stack_trace);
+
   TryBlocks* inner_try_block = PopTryBlock();
   const intptr_t try_index = inner_try_block->try_index();
   TryBlocks* outer_try_block = try_blocks_list_;
   const intptr_t outer_try_index = (outer_try_block != NULL)
       ? outer_try_block->try_index()
       : CatchClauseNode::kInvalidTryIndex;
 
   // Finally parse the 'finally' block.
   SequenceNode* finally_block = NULL;
   if (CurrentToken() == Token::kFINALLY) {
@@ skipping 11 matching lines @@
                                                         context_var,
                                                         outer_try_index);
       AddFinallyBlockToNode(node_to_inline, node);
       node_index += 1;
       node_to_inline = inner_try_block->GetNodeToInlineFinally(node_index);
       tokens_iterator_.SetCurrentPosition(finally_pos);
     }
     finally_block = ParseFinallyBlock();
   }
 
-  if (!generic_catch_seen) {
-    // No generic catch handler exists so rethrow the exception so that
-    // the next catch handler can deal with it.
-    catch_handler_list->Add(
-        new ThrowNode(handler_pos,
-                      new LoadLocalNode(handler_pos, catch_excp_var),
-                      new LoadLocalNode(handler_pos, catch_trace_var)));
-  }
-  CatchClauseNode* catch_block =
+  CatchClauseNode* catch_clause =
       new CatchClauseNode(handler_pos,
                           catch_handler_list,
                           Array::ZoneHandle(Array::MakeArray(handler_types)),
                           context_var,
-                          catch_excp_var,
-                          catch_trace_var,
+                          exception_var,
+                          stack_trace_var,
                           (finally_block != NULL)
                               ? AllocateTryIndex()
                               : CatchClauseNode::kInvalidTryIndex,
-                          needs_stacktrace);
+                          needs_stack_trace);
 
   // Now create the try/catch ast node and return it. If there is a label
   // on the try/catch, close the block that's embedding the try statement
   // and attach the label to it.
   AstNode* try_catch_node =
-      new TryCatchNode(try_pos, try_block, end_catch_label,
-                       context_var, catch_block, finally_block, try_index);
+      new TryCatchNode(try_pos, try_block, context_var, catch_clause,
+                       finally_block, try_index);
 
   if (try_label != NULL) {
     current_block_->statements->Add(try_catch_node);
     SequenceNode* sequence = CloseBlock();
     sequence->set_label(try_label);
     try_catch_node = sequence;
   }
   return try_catch_node;
 }
 
@@ skipping 136 matching lines @@
     AddNodeForFinallyInlining(statement);
     ExpectSemicolon();
   } else if (CurrentToken() == Token::kSEMICOLON) {
     // Empty statement, nothing to do.
     ConsumeToken();
   } else if (CurrentToken() == Token::kRETHROW) {
     // Rethrow of current exception.
     ConsumeToken();
     ExpectSemicolon();
     // Check if it is ok to do a rethrow.
-    SourceLabel* label = current_block_->scope->LookupInnermostCatchLabel();
-    if (label == NULL ||
-        label->FunctionLevel() != current_block_->scope->function_level()) {
+    if ((try_blocks_list_ == NULL) || !try_blocks_list_->inside_catch()) {
       ErrorMsg(statement_pos, "rethrow of an exception is not valid here");
     }
-    ASSERT(label->owner() != NULL);
-    LocalScope* scope = label->owner()->parent();
+    // The exception and stack trace variables are bound in the block
+    // containing the try.
+    LocalScope* scope = try_blocks_list_->try_block()->scope->parent();
     ASSERT(scope != NULL);
     LocalVariable* excp_var =
         scope->LocalLookupVariable(Symbols::ExceptionVar());
     ASSERT(excp_var != NULL);
     LocalVariable* trace_var =
-        scope->LocalLookupVariable(Symbols::StacktraceVar());
+        scope->LocalLookupVariable(Symbols::StackTraceVar());
     ASSERT(trace_var != NULL);
     statement = new ThrowNode(statement_pos,
                               new LoadLocalNode(statement_pos, excp_var),
                               new LoadLocalNode(statement_pos, trace_var));
   } else {
     statement = ParseExpr(kAllowConst, kConsumeCascades);
     ExpectSemicolon();
   }
   return statement;
 }
@@ skipping 3544 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart