[vm, aot] Ensure fixups applied after the world has been compiled are applied to closures in finall…

runtime/vm/precompiler.cc

Index: runtime/vm/precompiler.cc
diff --git a/runtime/vm/precompiler.cc b/runtime/vm/precompiler.cc
index 787b4b28b454340fe8a56bcb5bad1bb9f9fbf9d2..4d8279d1d204f93121524380b087165c661579ed 100644
--- a/runtime/vm/precompiler.cc
+++ b/runtime/vm/precompiler.cc
@@ -2186,6 +2186,16 @@ void Precompiler::BindStaticCalls() {
   };
 
   BindStaticCallsVisitor visitor(Z);
+
+  // This visit misses functions compiled outside of the treeshaker's
+  // queue.
+  FunctionSet::Iterator it(enqueued_functions_.GetIterator());
+  for (const Function** current = it.Next(); current != NULL;
+       current = it.Next()) {
+    visitor.Visit(**current);
+  }
+
+  // This visit misses closures from duplicated finally clauses.

[siva, 2017/08/23 20:11:37]

Maybe the comment should be more clear , stating that two iterations are needed
to ensure we visit all functions as the ProgramVisitor's VisitFunctions does not
not visit closures from duplicated finally clauses and an iterations over the
treeshaker's queue does not visit functions compiled outside the queue.

What do you think?

[rmacnak, 2017/08/23 23:25:58]

Yes, I like that better.

   ProgramVisitor::VisitFunctions(&visitor);
 }
 
@@ -2274,6 +2284,16 @@ void Precompiler::SwitchICCalls() {
 
   ASSERT(!I->compilation_allowed());
   SwitchICCallsVisitor visitor(Z);
+
+  // This visit misses functions compiled outside of the treeshaker's
+  // queue.
+  FunctionSet::Iterator it(enqueued_functions_.GetIterator());
+  for (const Function** current = it.Next(); current != NULL;
+       current = it.Next()) {
+    visitor.Visit(**current);
+  }
+
+  // This visit misses closures from duplicated finally clauses.

[siva, 2017/08/23 20:11:37]

Ditto comment here.

   ProgramVisitor::VisitFunctions(&visitor);
 #endif
 }