dart2dart: Implementation of simple try/catch.

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

 // 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.
 
 part of dart2js.cps_ir.optimizers;
 
 /// Eliminate redundant phis from the given [FunctionDefinition].
 ///
 /// Phis in this case are [Continuations] together with corresponding
 /// [InvokeContinuation]s. A [Continuation] parameter at position i is redundant
@@ skipping 60 matching lines @@
         } else if (value == null) {
           value = def; // Set initial comparison value.
         } else if (value != def) {
           return null; // Differing invocation arguments.
         }
       }
 
       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
     // to index `dst`.
     int dst = 0;
     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;
       }
 
       Definition oldDefinition = cont.parameters[src];
 
       // 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;
           if (thatCont != cont) {
             workSet.add(thatCont);
           }
         }
       }
 
       // Replace individual parameters:
       // * In the continuation body, replace occurrence of param with value,
       // * and implicitly remove param from continuation signature and
       //   invocations by not incrementing `dst`. References of removed
       //   arguments are unlinked to keep definition usages up to date.
       uniqueDefinition.substituteFor(oldDefinition);
       for (InvokeContinuation invoke in invokes) {
         invoke.arguments[src].unlink();
       }
 
       // Finally, if the substituted definition is not in scope of the affected
       // continuation, move the continuation binding. This is safe to do since
       // the continuation is referenced only as the target in continuation
       // 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);
     }
 
     // Remove trailing items from parameter and argument lists.
     cont.parameters.length = dst;
     for (InvokeContinuation invoke in invokes) {
       invoke.arguments.length = dst;
     }
   }
@@ skipping 29 matching lines @@
 
   // Insert it just below the binding of definition.
   InteriorNode binding = definition.parent;
 
   letCont.body = binding.body;
   binding.body.parent = letCont;
 
   binding.body = letCont;
   letCont.parent = binding;
 }