Cleanup how a best fit function is found when setting a breakpoint.

runtime/vm/debugger.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/debugger.h"
 
 #include "include/dart_api.h"
 
 #include "platform/address_sanitizer.h"
 
@@ skipping 2631 matching lines @@
     *best_fit = func->raw();
   } else {
     if ((func->token_pos() > best_fit->token_pos()) &&
         ((func->end_token_pos() <= best_fit->end_token_pos()))) {
       *best_fit = func->raw();
     }
   }
 }
 
 
+static bool IsTokenPosWithinFunction(const Function& func, TokenPosition pos) {
+  return (func.token_pos() <= pos && pos <= func.end_token_pos());
+}
+
+
 RawFunction* Debugger::FindBestFit(const Script& script,
                                    TokenPosition token_pos) {
   Zone* zone = Thread::Current()->zone();
   Class& cls = Class::Handle(zone);
   Array& functions = Array::Handle(zone);
   GrowableObjectArray& closures = GrowableObjectArray::Handle(zone);

[siva, 2017/05/25 01:26:25]

Maybe this can be

const GrowableObjectArray& closures = GrowableObjectArray::Handle(
  zone, isolate_->object_store()->closure_functions());

as we are not modifying closures in this function.

[sivachandra, 2017/05/25 04:48:18]

Done.

   Function& function = Function::Handle(zone);
   Function& best_fit = Function::Handle(zone);
   Error& error = Error::Handle(zone);
 
   closures = isolate_->object_store()->closure_functions();

[siva, 2017/05/25 01:26:25]

this line will not be needed if initialized in above fashion.

[sivachandra, 2017/05/25 04:48:18]

Done.

   const intptr_t num_closures = closures.Length();
   for (intptr_t i = 0; i < num_closures; i++) {
     function ^= closures.At(i);
-    if (FunctionContains(function, script, token_pos)) {
+    if (function.script() != script.raw()) {
+      continue;
+    }
+    if (IsTokenPosWithinFunction(function, token_pos)) {
+      // Select the inner most closure.
       SelectBestFit(&best_fit, &function);
     }
   }
+  if (!best_fit.IsNull()) {
+    // The inner most closure found will be the best fit. Going
+    // over class functions below will not help in any further
+    // narrowing.
+    return best_fit.raw();
+  }
 
   const ClassTable& class_table = *isolate_->class_table();
   const intptr_t num_classes = class_table.NumCids();
   for (intptr_t i = 1; i < num_classes; i++) {
     if (class_table.HasValidClassAt(i)) {
       cls = class_table.At(i);
-      // Note: if this class has been parsed and finalized already,
-      // we need to check the functions of this class even if
-      // it is defined in a different 'script'. There could
-      // be mixin functions from the given script in this class.
-      // However, if this class is not parsed yet (not finalized),
-      // we can ignore it and avoid the side effect of parsing it.
-      if ((cls.script() != script.raw()) && !cls.is_finalized()) {
+      ASSERT(!cls.IsNull());
+      if (cls.script() != script.raw()) {
         continue;
       }
       // Parse class definition if not done yet.
       error = cls.EnsureIsFinalized(Thread::Current());
       if (!error.IsNull()) {
         // Ignore functions in this class.
         // TODO(hausner): Should we propagate this error? How?
         // EnsureIsFinalized only returns an error object if there
         // is no longjump base on the stack.
         continue;
       }
       functions = cls.functions();
       if (!functions.IsNull()) {
         const intptr_t num_functions = functions.Length();
         for (intptr_t pos = 0; pos < num_functions; pos++) {
           function ^= functions.At(pos);
           ASSERT(!function.IsNull());
-          if (FunctionContains(function, script, token_pos)) {
-            SelectBestFit(&best_fit, &function);
+          if (IsTokenPosWithinFunction(function, token_pos)) {
+            // Closures and inner functions within a class method are not
+            // present in the functions of a class. Hence, we can return
+            // right away as looking through other functions of a class
+            // will not narrow down to any inner function/closure.
+            return function.raw();
           }
         }
       }
     }
   }
   return best_fit.raw();

[siva, 2017/05/25 01:26:25]

can this be return Function::null();

[sivachandra, 2017/05/25 04:48:18]

Done.

 }
 
 
 BreakpointLocation* Debugger::SetBreakpoint(const Script& script,
                                             TokenPosition token_pos,
                                             TokenPosition last_token_pos,
                                             intptr_t requested_line,
                                             intptr_t requested_column) {
   Function& func = Function::Handle();
   func = FindBestFit(script, token_pos);
@@ skipping 1710 matching lines @@
 
 void Debugger::RegisterCodeBreakpoint(CodeBreakpoint* bpt) {
   ASSERT(bpt->next() == NULL);
   bpt->set_next(code_breakpoints_);
   code_breakpoints_ = bpt;
 }
 
 #endif  // !PRODUCT
 
 }  // namespace dart