Collect closure functions in isolate

runtime/vm/flow_graph_builder.cc

 // Copyright (c) 2012, 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/flow_graph_builder.h"
 
 #include "lib/invocation_mirror.h"
 #include "vm/ast_printer.h"
 #include "vm/bit_vector.h"
 #include "vm/compiler.h"
@@ skipping 2465 matching lines @@
   ASSERT(is_implicit || function.IsNonImplicitClosureFunction());
   // The context scope may have already been set by the non-optimizing
   // compiler.  If it was not, set it here.
   if (function.context_scope() == ContextScope::null()) {
     ASSERT(!is_implicit);
     const ContextScope& context_scope = ContextScope::ZoneHandle(
         Z, node->scope()->PreserveOuterScope(owner()->context_level()));
     ASSERT(!function.HasCode());
     ASSERT(function.context_scope() == ContextScope::null());
     function.set_context_scope(context_scope);
-    const Class& cls = Class::Handle(Z, owner()->function().Owner());
+
     // The closure is now properly setup, add it to the lookup table.
     // It is possible that the compiler creates more than one function
     // object for the same closure, e.g. when inlining nodes from
     // finally clauses. If we already have a function object for the
-    // same closure, do not add a second one. We compare the origin
-    // class, token position, and parent function to detect duplicates.
+    // same closure, do not add a second one. We compare the script,
+    // token position, and parent function to detect duplicates.
     // Note that we can have two different closure object for the same
     // source text representation of the closure: one with a non-closurized
     // parent, and one with a closurized parent function.
 
-    const Function& found_func = Function::Handle(
-        Z, cls.LookupClosureFunction(function.token_pos()));
-
+    const Script& origin = Script::Handle(Z, function.script());
+    const Function& found_func = Function::Handle(Z,
+        isolate()->LookupClosureFunction(origin, function.token_pos()));
     if (found_func.IsNull() ||
         (found_func.token_pos() != function.token_pos()) ||
         (found_func.script() != function.script()) ||
         (found_func.parent_function() != function.parent_function())) {

[Ivan Posva, 2015/11/13 17:47:51]

Why would any of these tests for pos or script fail after we have looked up and
found the closure function?

Also it looks like we could be repeatedly entering a closure functions if the
one we find first does not have the same parent due to the parent being
closurized or not.

[hausner, 2015/11/13 20:13:29]

It seems that LookupClosureFunction was implemented to find the innermost
closure containing a certain token position, yet all call sites are interested
in the closure starting at a given token position. (The debugger searches for
innermost closures, but it has its own functions to do so.)

I changed LookupClosureFunction and adapted all call sites.

-      cls.AddClosureFunction(function);
+      isolate()->AddClosureFunction(function);
     }
   }
   ZoneGrowableArray<PushArgumentInstr*>* arguments =
       new(Z) ZoneGrowableArray<PushArgumentInstr*>(1);
   ASSERT(function.context_scope() != ContextScope::null());
 
   // The function type of a closure may have type arguments. In that case,
   // pass the type arguments of the instantiator.
   const Class& cls = Class::ZoneHandle(Z, function.signature_class());
   ASSERT(!cls.IsNull());
@@ skipping 2098 matching lines @@
   Report::MessageF(Report::kBailout,
                    Script::Handle(function.script()),
                    function.token_pos(),
                    "FlowGraphBuilder Bailout: %s %s",
                    String::Handle(function.name()).ToCString(),
                    reason);
   UNREACHABLE();
 }
 
 }  // namespace dart