From 61f9b4d73a4d7a543f5a5ae099eda9260444c46b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez=20de=20Dios?= Date: Tue, 8 Apr 2014 02:44:39 +0200 Subject: [PATCH] Change semantics of finish This is a backwards incompatible change that changes the meaning of the `finish` command. `finish n` means now literally "finish _n_ frames from current position and then stop", which makes more sense than the old "run program until frame number _n_". The `Context#stop_return` method has been removed from the API because it was just added to support `byebug` statements at the end of blocks and now `Context#step_out` can perfectly handle this. Now the API now can be easily extended in the future to support breakpoints at the end of method, which could be useful to inspect the return value, for example. pry-byebug will need to be upgraded to support this. --- ext/byebug/byebug.c | 17 +++++++----- ext/byebug/byebug.h | 10 +++---- ext/byebug/context.c | 52 ++++++++++++++--------------------- lib/byebug.rb | 11 +++----- lib/byebug/commands/finish.rb | 25 +++++++---------- lib/byebug/commands/trace.rb | 2 +- test/finish_test.rb | 12 ++++---- 7 files changed, 57 insertions(+), 72 deletions(-) diff --git a/ext/byebug/byebug.c b/ext/byebug/byebug.c index 1910c14be..c88f97c56 100644 --- a/ext/byebug/byebug.c +++ b/ext/byebug/byebug.c @@ -160,7 +160,7 @@ call_at_catchpoint(VALUE context_obj, debug_context_t *dc, VALUE exp) static VALUE call_at_return(VALUE context_obj, debug_context_t *dc, VALUE file, VALUE line) { - dc->stop_reason = CTX_STOP_BREAKPOINT; + CTX_FL_UNSET(dc, CTX_FL_STOP_ON_RET); return call_at(context_obj, dc, rb_intern("at_return"), 2, file, line); } @@ -241,6 +241,9 @@ call_event(VALUE trace_point, void *data) dc->calced_stack_size++; + if (CTX_FL_TEST(dc, CTX_FL_STOP_ON_RET)) + dc->steps_out = dc->steps_out <= 0 ? -1 : dc->steps_out + 1; + EVENT_COMMON breakpoint = Qnil; @@ -270,7 +273,11 @@ return_event(VALUE trace_point, void *data) EVENT_COMMON - if (dc->calced_stack_size + 1 == dc->before_frame) + if (dc->steps_out == 1) + { + dc->steps = 1; + } + else if ((dc->steps_out == 0) && (CTX_FL_TEST(dc, CTX_FL_STOP_ON_RET))) { VALUE file, line; @@ -280,11 +287,7 @@ return_event(VALUE trace_point, void *data) call_at_return(context, dc, file, line); } - if (dc->calced_stack_size + 1 == dc->after_frame) - { - reset_stepping_stop_points(dc); - dc->steps = 1; - } + dc->steps_out = dc->steps_out <= 0 ? -1 : dc->steps_out - 1; cleanup(dc); } diff --git a/ext/byebug/byebug.h b/ext/byebug/byebug.h index 96e040ec4..39e06628c 100644 --- a/ext/byebug/byebug.h +++ b/ext/byebug/byebug.h @@ -12,6 +12,7 @@ #define CTX_FL_SUSPEND (1<<5) /* thread currently suspended */ #define CTX_FL_TRACING (1<<6) /* call at_tracing method */ #define CTX_FL_WAS_RUNNING (1<<7) /* thread was previously running */ +#define CTX_FL_STOP_ON_RET (1<<8) /* can stop on method 'end' */ /* macro functions */ #define CTX_FL_TEST(c,f) ((c)->flags & (f)) @@ -34,11 +35,10 @@ typedef struct { VALUE thread; int thnum; - int dest_frame; - int lines; /* # of lines in dest_frame before stopping */ - int steps; /* # of steps before stopping */ - int after_frame; /* stop right after returning from this frame */ - int before_frame; /* stop right before returning from this frame */ + int dest_frame; /* next stop's frame if stopped by next */ + int lines; /* # of lines in dest_frame before stopping */ + int steps; /* # of steps before stopping */ + int steps_out; /* # of returns before stopping */ VALUE last_file; VALUE last_line; diff --git a/ext/byebug/context.c b/ext/byebug/context.c index 9076d3b29..5d861b6c2 100644 --- a/ext/byebug/context.c +++ b/ext/byebug/context.c @@ -12,8 +12,7 @@ reset_stepping_stop_points(debug_context_t *context) context->dest_frame = -1; context->lines = -1; context->steps = -1; - context->after_frame = -1; - context->before_frame = -1; + context->steps_out = -1; } /* @@ -421,22 +420,35 @@ Context_step_into(int argc, VALUE *argv, VALUE self) * call-seq: * context.step_out(frame) * - * Stops after frame number +frame+ is activated. Implements +finish+ and - * +next+ commands. + * Stops after +n_frames+ frames are finished. Implements +finish+ and + * +next+ commands. +force+ parameter (if true) ensures that the cursor will + * stop in the specified frame even when there's no more instructions to run. + * In that case, it will stop when the return event for that frame is + * triggered. */ static VALUE -Context_step_out(VALUE self, VALUE frame) +Context_step_out(int argc, VALUE *argv, VALUE self) { + int n_args, n_frames; + VALUE v_frames, v_force; debug_context_t *context; + n_args = rb_scan_args(argc, argv, "02", &v_frames, &v_force); + n_frames = n_args == 0 ? 1 : FIX2INT(v_frames); + v_force = (n_args < 2) ? Qfalse : v_force; + Data_Get_Struct(self, debug_context_t, context); - if (FIX2INT(frame) < 0 || FIX2INT(frame) >= context->calced_stack_size) + if (n_frames < 0 || n_frames >= context->calced_stack_size) rb_raise(rb_eRuntimeError, "Stop frame is out of range."); - context->after_frame = context->calced_stack_size - FIX2INT(frame); + context->steps_out = n_frames; + if (RTEST(v_force)) + CTX_FL_SET(context, CTX_FL_STOP_ON_RET); + else + CTX_FL_UNSET(context, CTX_FL_STOP_ON_RET); - return frame; + return Qnil; } /* @@ -474,27 +486,6 @@ Context_step_over(int argc, VALUE *argv, VALUE self) return Qnil; } -/* - * call-seq: - * context.stop_return(frame) - * - * Stops before frame number +frame+ is activated. Useful when you enter the - * debugger after the last statement in a method. - */ -static VALUE -Context_stop_return(VALUE self, VALUE frame) -{ - debug_context_t *context; - - Data_Get_Struct(self, debug_context_t, context); - if (FIX2INT(frame) < 0 || FIX2INT(frame) >= context->calced_stack_size) - rb_raise(rb_eRuntimeError, "Stop frame is out of range."); - - context->before_frame = context->calced_stack_size - FIX2INT(frame); - - return frame; -} - static void context_suspend_0(debug_context_t *context) { @@ -639,9 +630,8 @@ Init_context(VALUE mByebug) rb_define_method(cContext, "resume" , Context_resume , 0); rb_define_method(cContext, "calced_stack_size", Context_calced_stack_size, 0); rb_define_method(cContext, "step_into" , Context_step_into , -1); - rb_define_method(cContext, "step_out" , Context_step_out , 1); + rb_define_method(cContext, "step_out" , Context_step_out , -1); rb_define_method(cContext, "step_over" , Context_step_over , -1); - rb_define_method(cContext, "stop_return" , Context_stop_return , 1); rb_define_method(cContext, "stop_reason" , Context_stop_reason , 0); rb_define_method(cContext, "suspend" , Context_suspend , 0); rb_define_method(cContext, "suspended?" , Context_is_suspended , 0); diff --git a/lib/byebug.rb b/lib/byebug.rb index c85382a6e..e367023c8 100644 --- a/lib/byebug.rb +++ b/lib/byebug.rb @@ -181,16 +181,13 @@ class Exception module Kernel # - # Enters byebug after _steps_into_ line events and _steps_out_ return events - # occur. Before entering byebug startup, the init script is read. + # Enters byebug right before (or right after if _before_ is false) return + # events occur. Before entering byebug the init script is read. # - def byebug(steps_into = 1, steps_out = 2) + def byebug(steps_out = 1, before = true) Byebug.start Byebug.run_init_script(StringIO.new) - if Byebug.current_context.calced_stack_size > 2 - Byebug.current_context.stop_return steps_out if steps_out >= 1 - end - Byebug.current_context.step_into steps_into if steps_into >= 0 + Byebug.current_context.step_out(steps_out, before) end alias_method :debugger, :byebug diff --git a/lib/byebug/commands/finish.rb b/lib/byebug/commands/finish.rb index ba9aff385..975acec00 100644 --- a/lib/byebug/commands/finish.rb +++ b/lib/byebug/commands/finish.rb @@ -9,14 +9,12 @@ def regexp end def execute - if not @match[1] - frame_pos = @state.frame_pos - else - max_frame = Context.stack_size - @state.frame_pos - frame_pos = get_int(@match[1], "finish", 0, max_frame-1, 0) - return nil unless frame_pos - end - @state.context.step_out frame_pos + max_frames = Context.stack_size - @state.frame_pos + n_frames = get_int(@match[1], "finish", 0, max_frames - 1, 1) + return nil unless n_frames + + force = n_frames == 0 ? true : false + @state.context.step_out(@state.frame_pos + n_frames, force) @state.frame_pos = 0 @state.proceed end @@ -27,14 +25,11 @@ def names end def description - %{fin[ish][ frame-number]\tExecute until selected stack frame returns. - - If no frame number is given, we run until the currently selected frame - returns. The currently selected frame starts out the most-recent frame - or 0 if no frame positioning (e.g "up", "down" or "frame") has been - performed. + %{fin[ish][ n_frames]\tExecute until frame returns. - If a frame number is given we run until that frame returns.} + If no number is given, we run until the current frame returns. If a + number of frames `n_frames` is given, then we run until `n_frames` + return from the current position.} end end end diff --git a/lib/byebug/commands/trace.rb b/lib/byebug/commands/trace.rb index 547b9bee3..8cb747c12 100644 --- a/lib/byebug/commands/trace.rb +++ b/lib/byebug/commands/trace.rb @@ -19,7 +19,7 @@ def execute if @match[3] && @match[3] !~ /(:?no)?stop/ errmsg "expecting \"stop\" or \"nostop\"; got \"#{@match[3]}\"\n" else - dbg_cmd = (@match[3] && @match[3] !~ /nostop/) ? 'byebug(1,0)' : '' + dbg_cmd = (@match[3] && @match[3] !~ /nostop/) ? 'byebug(1, false)' : '' end eval("trace_var(:\"\$#{varname}\") do |val| print \"traced global variable '#{varname}' has value '\#{val}'\"\n diff --git a/test/finish_test.rb b/test/finish_test.rb index b598362d5..f7fc31a23 100644 --- a/test/finish_test.rb +++ b/test/finish_test.rb @@ -18,22 +18,22 @@ def d class TestFinish < TestDsl::TestCase before { enter "break #{__FILE__}:14", 'cont' } - it 'must stop at the next frame by default' do + it 'must stop after current frame is finished when without arguments' do enter 'finish' debug_file('finish') { state.line.must_equal 11 } end - it 'must stop at the #0 frame by default' do + it 'must stop before current frame finishes if 0 specified as argument' do enter 'finish 0' - debug_file('finish') { state.line.must_equal 11 } + debug_file('finish') { state.line.must_equal 15 } end - it 'must stop at the specified frame' do + it 'must stop after current frame is finished if 1 specified as argument' do enter 'finish 1' - debug_file('finish') { state.line.must_equal 7 } + debug_file('finish') { state.line.must_equal 11 } end - it 'must stop at the next frame if the current frame was changed' do + it 'must behave consistenly even if current frame has been changed' do enter 'up', 'finish' debug_file('finish') { state.line.must_equal 7 } end