jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
This looks good. I have one worry. If the plan is stepping through a range of
line 10, and that code branches to:
0x120 - line 0
0x128 - line 11
0x130 - line 12
You will see that we are starting at line 0, so you will choose 11 as the NEW
line number to match, extend the range to 0x130 and then step through that. So
we would have effectively stepped through lines 10 and 11. There are actually
times the stepping will do this - for instance if you jump into the middle of
another line we step out of the line for you, since being mid-source line is
disconcerting.
It is important not to stop the stepping at line 0, since being "nowhere" is
even more disconcerting. So consuming the line 0 range is important. But
somewhere you need to add the notion of "expected completion line" so that you
don't extend past an initial line 0 if the following line is not the expected
one.
================
Comment at: include/lldb/Symbol/LineEntry.h:143-154
@@ -142,1 +142,14 @@
+ //------------------------------------------------------------------
+ /// Give the range for this LineEntry + any additional LineEntries for
+ /// this same source line that are contiguous.
+ ///
+ /// A compiler may emit multiple line entries for a single source line,
+ /// e.g. to indicate subexpressions at different columns. This method
+ /// will get the AddressRange for all of the LineEntries for this source
+ /// line that are contiguous.
+ ///
+ /// @return
+ /// The contiguous AddressRange for this source line.
+ //------------------------------------------------------------------
+ AddressRange
----------------
Do you want to mention the treatment of line number 0 here? You also glom
those into the extended line range, which isn't obvious from the name or
comment.
================
Comment at: source/Target/Thread.cpp:1546-1547
@@ +1545,4 @@
+{
+ return QueueThreadPlanForStepOverRange (abort_other_plans,
line_entry.GetSameLineContiguousAddressRange(), addr_context,
stop_other_threads, step_out_avoids_code_withoug_debug_info);
+}
+
----------------
This should get broken up across more lines.
================
Comment at: source/Target/Thread.cpp:1583-1584
@@ +1582,4 @@
+{
+ return QueueThreadPlanForStepInRange (abort_other_plans,
line_entry.GetSameLineContiguousAddressRange(), addr_context, step_in_target,
stop_other_threads, step_in_avoids_code_without_debug_info,
step_out_avoids_code_without_debug_info);
+}
+
----------------
This line is too long.
Repository:
rL LLVM
http://reviews.llvm.org/D15407
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
