Debug: support breakpoints set in the middle of statement

src/debug.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 217 matching lines @@
     Next();
   }
 
   // Move to the break point found.
   Reset();
   Next(closest_break_point);
 }
 
 
 // Find the break point closest to the supplied source position.
-void BreakLocationIterator::FindBreakLocationFromPosition(int position) {
+void BreakLocationIterator::FindBreakLocationFromPosition(int position,
+    bool statement_alighned) {

[Yang, 2013/06/25 11:32:00]

Change variable name to "statement_aligned".

Suggestion: I'd prefer introducing an enum for better redability. Something like

enum PositionAlignment {
  STATEMENT_ALIGNED = 0,
  BREAK_POSITION_ALIGNED = 1
};

The advantage is that the use sites would read like

it.FindBreakLocationFromPosition(
  *source_position, STATEMENT_ALIGNED);

Of course you would have to do that in javascript as well. You could add those
enum values as constants to macros.py.

[Peter.Rybin, 2013/06/27 14:07:29]

I'm fine with this idea. I did JavaScript part as a regular object, like it is
done in debug-debugger.js

Done

   // Run through all break points to locate the one closest to the source
   // position.
   int closest_break_point = 0;
   int distance = kMaxInt;
+
   while (!Done()) {
+    int next_position;
+    if (statement_alighned) {

[Yang, 2013/06/25 11:32:00]

You could use a ternary operator to simplify this
int next_position = statement_aligned ? this->statement_position()
                                      : this->position();

[Peter.Rybin, 2013/06/27 14:07:29]

I guess with enum I should do switch? Or is it too long?

+      next_position = this->statement_position();
+    } else {
+      next_position = this->position();
+    }
     // Check if this break point is closer that what was previously found.
-    if (position <= statement_position() &&
-        statement_position() - position < distance) {
+    if (position <= next_position && next_position - position < distance) {
       closest_break_point = break_point();
-      distance = statement_position() - position;
+      distance = next_position - position;
       // Check whether we can't get any closer.
       if (distance == 0) break;
     }
     Next();
   }
 
   // Move to the break point found.
   Reset();
   Next(closest_break_point);
 }
@@ skipping 910 matching lines @@
     // Return if retrieving debug info failed.
     return;
   }
 
   Handle<DebugInfo> debug_info = GetDebugInfo(shared);
   // Source positions starts with zero.
   ASSERT(*source_position >= 0);
 
   // Find the break point and change it.
   BreakLocationIterator it(debug_info, SOURCE_BREAK_LOCATIONS);
-  it.FindBreakLocationFromPosition(*source_position);
+  it.FindBreakLocationFromPosition(*source_position, true);
   it.SetBreakPoint(break_point_object);
 
   *source_position = it.position();
 
   // At least one active break point now.
   ASSERT(debug_info->GetBreakPointCount() > 0);
 }
 
 
 bool Debug::SetBreakPointForScript(Handle<Script> script,
                                    Handle<Object> break_point_object,
-                                   int* source_position) {
+                                   int* source_position,
+                                   bool statement_alighned) {

[Yang, 2013/06/25 11:32:00]

Ditto.

[Peter.Rybin, 2013/06/27 14:07:29]

Done.

   HandleScope scope(isolate_);
 
   PrepareForBreakPoints();
 
   // Obtain shared function info for the function.
   Object* result = FindSharedFunctionInfoInScript(script, *source_position);
   if (result->IsUndefined()) return false;
 
   // Make sure the function has set up the debug info.
   Handle<SharedFunctionInfo> shared(SharedFunctionInfo::cast(result));
@@ skipping 10 matching lines @@
   } else {
     position = *source_position - shared->start_position();
   }
 
   Handle<DebugInfo> debug_info = GetDebugInfo(shared);
   // Source positions starts with zero.
   ASSERT(position >= 0);
 
   // Find the break point and change it.
   BreakLocationIterator it(debug_info, SOURCE_BREAK_LOCATIONS);
-  it.FindBreakLocationFromPosition(position);
+  it.FindBreakLocationFromPosition(position, statement_alighned);
   it.SetBreakPoint(break_point_object);
 
   *source_position = it.position() + shared->start_position();
 
   // At least one active break point now.
   ASSERT(debug_info->GetBreakPointCount() > 0);
   return true;
 }
 
 
@@ skipping 2566 matching lines @@
     {
       Locker locker(reinterpret_cast<v8::Isolate*>(isolate_));
       isolate_->debugger()->CallMessageDispatchHandler();
     }
   }
 }
 
 #endif  // ENABLE_DEBUGGER_SUPPORT
 
 } }  // namespace v8::internal