dart2dart: Implementation of simple try/catch.

pkg/compiler/lib/src/cps_ir/redundant_phi.dart

Index: pkg/compiler/lib/src/cps_ir/redundant_phi.dart
diff --git a/pkg/compiler/lib/src/cps_ir/redundant_phi.dart b/pkg/compiler/lib/src/cps_ir/redundant_phi.dart
index c4964c0e4d02d887c9b36b66b922f6eda6bbde6a..6a437bf4971a7e2299bfa4ce9802d5f30c5f0014 100644
--- a/pkg/compiler/lib/src/cps_ir/redundant_phi.dart
+++ b/pkg/compiler/lib/src/cps_ir/redundant_phi.dart
@@ -78,6 +78,24 @@ class RedundantPhiEliminator extends RecursiveVisitor with PassMixin {
       return value;
     }
 
+    // If uniqueDefinition is in the body of the LetCont binding the
+    // continuation, then we will drop the continuation binding to just inside
+    // the binding of uniqueDefiniton.  This is not safe if we drop the
+    // continuation binding inside a LetHandler exception handler binding.
+    LetCont letCont = cont.parent;
+    bool safeForHandlers(Definition uniqueDefinition) {
+      bool seenHandler = false;
+      Node current = uniqueDefinition.parent;
+      while (current != null) {
+        if (current == letCont) return !seenHandler;

[floitsch, 2015/02/16 14:54:07]

while (current != letCont) {
  if (current is LetHandler) return false;
  current = current.parent;
}
return true;

[Kevin Millikin (Google), 2015/02/24 11:59:25]

That's not quite the same.  It can be the case that uniqueDefinition is bound
outside letCont, in that case we will not move uniqueDefinition and it is safe.

So we need to check: is letCont on the path to the root and do we see a handler
before seeing letCont.

+        seenHandler = seenHandler || current is LetHandler;
+        current = current.parent;
+      }
+      // When uniqueDefinition is not in the body of the LetCont binding the
+      // continuation, we will not move any code, so that is safe.
+      return true;
+    }
+
     // Check if individual parameters are always called with a unique
     // definition, and remove them if that is the case. During each iteration,
     // we read the current parameter/argument from index `src` and copy it
@@ -86,13 +104,14 @@ class RedundantPhiEliminator extends RecursiveVisitor with PassMixin {
     for (int src = 0; src < cont.parameters.length; src++) {
       // Is the current phi redundant?
       Definition uniqueDefinition = uniqueDefinitionOf(src);
-      if (uniqueDefinition == null) {
+      if (uniqueDefinition == null || !safeForHandlers(uniqueDefinition)) {
         // Reorganize parameters and arguments in case of deletions.
-        cont.parameters[dst] = cont.parameters[src];
-        for (InvokeContinuation invoke in invokes) {
+        if (src != dst) {
+          cont.parameters[dst] = cont.parameters[src];
+          for (InvokeContinuation invoke in invokes) {
             invoke.arguments[dst] = invoke.arguments[src];
+          }
         }
-
         dst++;
         continue;
       }
@@ -101,8 +120,9 @@ class RedundantPhiEliminator extends RecursiveVisitor with PassMixin {
 
       // Add continuations of about-to-be modified invokes to worklist since
       // we might introduce new optimization opportunities.
-      for (Reference ref = oldDefinition.firstRef; ref != null;
-           ref = ref.next) {
+      for (Reference ref = oldDefinition.firstRef;
+          ref != null;

[floitsch, 2015/02/16 14:54:07]

align with "Reference".

[Kevin Millikin (Google), 2015/02/24 11:59:25]

Done.

+          ref = ref.next) {
         Node parent = ref.parent;
         if (parent is InvokeContinuation) {
           Continuation thatCont = parent.continuation.definition;
@@ -128,7 +148,6 @@ class RedundantPhiEliminator extends RecursiveVisitor with PassMixin {
       // invokes, and all such invokes must be within the scope of
       // [uniqueDefinition]. Note that this is linear in the depth of
       // the binding of [uniqueDefinition].
-      LetCont letCont = cont.parent;
       assert(letCont != null);
       _moveIntoScopeOf(letCont, uniqueDefinition);
     }