dart2dart: Implementation of simple try/catch.

pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart

Index: pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart
diff --git a/pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart b/pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart
index 3bb2206b450480ff3ed276cb30c924b90948d524..4908b55e0b7fb7ee5882673a4db3b729aa44540d 100644
--- a/pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart
+++ b/pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.dart
@@ -100,7 +100,13 @@ class StatementRewriter extends Visitor<Statement, Expression> with PassMixin {
   /// for a break to L' if L maps to L'.
   Map<Label, Jump> labelRedirects = <Label, Jump>{};
 
-  /// Returns the redirect target of [label] or [label] itself if it should not
+  // The successor statements for labeled statements that have only one break

[karlklose, 2015/02/16 10:15:48]

Use '///'.

+  // from them are normally rewritten inline at the site of the break.  This is
+  // not safe if the code would be move inside the scope of an exception

[karlklose, 2015/02/16 10:15:48]

'move' -> 'moved'.

+  // handler (i.e., if the code would be moved into a try from outside it.

[karlklose, 2015/02/16 10:15:48]

Missing ')'.

[Kevin Millikin (Google), 2015/02/25 11:06:36]

That's embarassing for a Lisper.

+  List<Label> safeForHandlers = <Label>[];
+
+  /// Returns the redirect target of [jump] or [jump] itself if it should not
   /// be redirected.
   Jump redirect(Jump jump) {
     Jump newJump = labelRedirects[jump.target];
@@ -265,7 +271,9 @@ class StatementRewriter extends Visitor<Statement, Expression> with PassMixin {
     // Note that useCount was accounted for at visitLabeledStatement.
     // Note redirect may return either a Break or Continue statement.
     Jump jump = redirect(node);
-    if (jump is Break && jump.target.useCount == 1) {
+    if (jump is Break &&
+        jump.target.useCount == 1 &&
+        safeForHandlers.contains(jump.target)) {

[asgerf, 2015/02/20 10:10:07]

Isn't this a bit expensive? It looks like a quadratic behavior will kick in very
easily.

Wouldn't it be better to use a Set, or at least only add singly-used labels to
the list to make it shorter (so it becomes "safeForInlining").

[Kevin Millikin (Google), 2015/02/25 11:06:36]

I don't think it's best to add only singly-used labels, since it's difficult to
reason about the way that we adjust the use counts of labels on the fly.

I'll change it to a Set, though that might have a higher cost than List for
'pushing' and 'popping'.  I suppose it's not necessary to ever remove labels
from the set --- do you think it's a good idea not to remove them?

I'll still change the name to safeForInlining, because that's better (having
multiple uses doesn't make a label 'unsafe' anyway).

[asgerf, 2015/02/25 11:55:11]

Ah, I forgot the use-counts on labels change during this phase.

I like to remove garbage from maps and sets when it's easy to do so. It usually
pays off.

       --jump.target.useCount;
       return visitStatement(jump.target.binding.next);
     }
@@ -292,7 +300,9 @@ class StatementRewriter extends Visitor<Statement, Expression> with PassMixin {
       return result;
     }
 
+    safeForHandlers.add(node.label);
     node.body = visitStatement(node.body);
+    safeForHandlers.removeLast();
 
     if (node.label.useCount == 0) {
       // Eliminate the label if next was inlined at a break
@@ -358,6 +368,15 @@ class StatementRewriter extends Visitor<Statement, Expression> with PassMixin {
     throw "Unexpected WhileCondition in StatementRewriter";
   }
 
+  Statement visitTryStatement(TryStatement node) {
+    List<Label> saved = safeForHandlers;
+    safeForHandlers = <Label>[];
+    node.tryBody = visitStatement(node.tryBody);
+    safeForHandlers = saved;
+    node.catchBody = visitStatement(node.catchBody);
+    return node;
+  }
+
   Expression visitConstant(Constant node) {
     return node;
   }