VM: Zero-cost try-finally with an empty finally-block.

runtime/vm/flow_graph_builder.cc

Index: runtime/vm/flow_graph_builder.cc
diff --git a/runtime/vm/flow_graph_builder.cc b/runtime/vm/flow_graph_builder.cc
index 6ac47b80e3f930d349e5ef254c03bef06c94de6d..a74f02afc83ce926bd152ecab62b2ee77e1bbd5a 100644
--- a/runtime/vm/flow_graph_builder.cc
+++ b/runtime/vm/flow_graph_builder.cc
@@ -4225,6 +4225,22 @@ void EffectGraphVisitor::VisitCatchClauseNode(CatchClauseNode* node) {
 
 void EffectGraphVisitor::VisitTryCatchNode(TryCatchNode* node) {
   InlineBailout("EffectGraphVisitor::VisitTryCatchNode (exception)");
+  CatchClauseNode* catch_block = node->catch_block();
+  SequenceNode* finally_block = node->finally_block();
+  if ((finally_block != NULL) && (finally_block->length() == 0)) {

[rmacnak, 2016/09/08 22:02:37]

finally_block == NULL || finally_block->length() == 0 ?

try {
  ...
} catch (e) {
  rethrow;
}

should be okay too.

[Florian Schneider, 2016/09/09 00:51:48]

Yes, the only issue is that when 'catch' is parsed, it translated into:

-Catch
 -Seqence
  -Sequence
   -StoreLocal(e)  // if exception var present
   -StoreLocal(s)  // if stacktrace var present
   -Sequence
    -Rethrow

With an 'on' handler there is also an is-test.
This all makes the pattern matching more complicated and brittle depending on
many details in the parser implementation. I'll stick to the simpler synthetic
catch clauses for now.

+    SequenceNode* catch_sequence = catch_block->sequence();
+    if (catch_sequence->length() == 1) {
+      ThrowNode* throw_node = catch_sequence-> NodeAt(0)->AsThrowNode();

[rmacnak, 2016/09/08 22:02:37]

->NodeAt

[Florian Schneider, 2016/09/09 00:51:48]

Done.

+      if ((throw_node != NULL) && (throw_node->exception() != NULL)) {

[Florian Schneider, 2016/09/09 00:51:48]

Actually, I have to check (throw_node->stack_trace() != NULL) to detect the
rethrow correctly. It worked because catch-clauses parsed from Dart code are
parsed differently.

+        // Empty finally-block in a try-finally can be optimized away.
+        EffectGraphVisitor for_try(owner());
+        node->try_block()->Visit(&for_try);
+        Append(for_try);
+        return;
+      }
+    }
+  }
+
   const intptr_t original_handler_index = owner()->try_index();
   const intptr_t try_handler_index = node->try_index();
   ASSERT(try_handler_index != original_handler_index);
@@ -4254,9 +4270,6 @@ void EffectGraphVisitor::VisitTryCatchNode(TryCatchNode* node) {
   // We are done generating code for the try block.
   owner()->set_try_index(original_handler_index);
 
-  CatchClauseNode* catch_block = node->catch_block();
-  SequenceNode* finally_block = node->finally_block();
-
   // If there is a finally block, it is the handler for code in the catch
   // block.
   const intptr_t catch_handler_index = (finally_block == NULL)