Re: [RFA] Reverse Debugging, 3/5

[Joel Brobecker <brobecker@adacore.com>]

I had some comments, too, but Pedro made most of them.  Here are mine
that don't overlap with his...

> +      /* OK, we're just gonna keep stepping here.  */

I would prefer if we kept slang out of the comments. Can we use
"going to" instead?

> +      if (stop_func_sal.pc == stop_pc)
> +	{
> +	  /* We're there already.  Just stop stepping now.  */
> +	  ecs->event_thread->stop_step = 1;
> +	  print_stop_reason (END_STEPPING_RANGE, 0);
> +	  stop_stepping (ecs);
> +	  return;
> +	}
> +      /* Else just reset the step range and keep going.
> +	 No step-resume breakpoint, they don't work for
> +	 epilogues, which can have multiple entry paths.  */
> +      ecs->event_thread->step_range_start = stop_func_sal.pc;
> +      ecs->event_thread->step_range_end   = stop_func_sal.end;
> +      keep_going (ecs);
> +      return;
> +    }

Regarding this hunk, it really took me a long time to understand
what we're supposed to do when execution is reverse.  I think it
was a combination of the function name together with the fact that
we're not trying to undo what the function does, but instead implement
whatever command the user has issued which caused us to call this
function. I am thinking, for the sake of clarity, to implement this
in a separate function and call the appropriate function from
handle_inferior_event.  The function name can then be used to explain
at a glance what it's supposed to do...

-- 
Joel