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

runtime/vm/precompiler.cc

 // Copyright (c) 2015, 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.
 
 #include "vm/precompiler.h"
 
 #include "vm/aot_optimizer.h"
 #include "vm/assembler.h"
 #include "vm/ast_printer.h"
 #include "vm/branch_optimizer.h"
@@ skipping 2168 matching lines @@
 
    private:
     Code& code_;
     Array& table_;
     Smi& pc_offset_;
     Function& target_;
     Code& target_code_;
   };
 
   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);
 }
 
 void Precompiler::SwitchICCalls() {
 #if !defined(TARGET_ARCH_DBC)
   // Now that all functions have been compiled, we can switch to an instance
   // call sequence that loads the Code object and entry point directly from
   // the ic data array instead indirectly through a Function in the ic data
   // array. Iterate all the object pools and rewrite the ic data from
   // (cid, target function, count) to (cid, target code, entry point), and
@@ skipping 68 matching lines @@
     ICData& ic_;
     String& target_name_;
     Array& args_descriptor_;
     UnlinkedCall& unlinked_;
     Code& target_code_;
     UnlinkedCallSet canonical_unlinked_calls_;
   };
 
   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
 }
 
 void Precompiler::FinalizeAllClasses() {
   Library& lib = Library::Handle(Z);
   Class& cls = Class::Handle(Z);
 
   for (intptr_t i = 0; i < libraries_.Length(); i++) {
     lib ^= libraries_.At(i);
@@ skipping 1035 matching lines @@
 
   ASSERT(FLAG_precompiled_mode);
   const bool optimized = function.IsOptimizable();  // False for natives.
   DartPrecompilationPipeline pipeline(zone, field_type_map);
   return PrecompileFunctionHelper(precompiler, &pipeline, function, optimized);
 }
 
 #endif  // DART_PRECOMPILER
 
 }  // namespace dart