Re: [patch] Add an evaluation function hook to Python breakpoints.

[Doug Evans <dje@google.com>]

On Mon, Dec 13, 2010 at 1:02 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Doug Evans <dje@google.com> writes:
>
>> On Mon, Dec 13, 2010 at 5:50 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>>> This patch allows Python breakpoints to be sub-classed and implements
>>> the "evaluate" function.  If the user defines an "evaluate" function in
>>> the Python breakpoint it will be called when GDB evaluates any
>>> conditions assigned to the breakpoint. This allows the user to write
>>> conditional breakpoints entirely in Python, and also allows the user to
>>> collect data at each breakpoint.  If the function returns True, GDB will
>>> stop the inferior at the breakpoint; if the function returns False
>>> the inferior will continue.
>>
>> Hi.
>> My $0.02 cents.
>>
>> Collecting data in the "evaluate" function feels too hacky for
>> something we explicitly tell users is the published way to do this
>> kind of thing.
>
> It's nothing more than a hook for the user to accomplish some truth
> finding mission at breakpoint evaluation time.  If the user wants to
> collect data, then I think they can do that -- that is we should not
> attempt to stop them.  I probably worded my email hastily; FWIW I think
> this a place where we can write conditional breakpoints that can be
> written entirely in Python.

I'm not suggesting we stop users from doing quick hacks. :-)
[Heck, can they already write a breakpoint condition in Python?  E.g.
with a python convenience function?]

But there is an existing need to collect data at a breakpoint in a
stable way (i.e. collecting data in the CLI commands section is
problematic).  I think such a facility is rather useful,  and we
should provide it.  But I'd rather make it explicit, instead of a
hacky use of the gdb breakpoint condition.

It's an orthogonal feature to allowing one to write the breakpoint
condition in Python via a subclass of gdb.Breakpoint, so it *could* be
a separate patch.
I just don't want the breakpoint condition being *the* way to collect data.

>> And the name "evaluate" doesn't feel right either.  What does it mean
>> to evaluate a breakpoint? [One might think it means to evaluate the
>> location of the breakpoint, e.g., since gdb tries to re-evaluate the
>> location when the binary changes.]
>> I realize the _p convention mightn't be sufficiently common to use in
>> the Python API, but for example a better name might be stop_p.
>> And then one would have another method (I don't have a good name for
>> it ATM) to use to collect data.
>> Setting aside name choices, I like that API better.
>
> I'm happy with any name.  Lets just call it what the majority are
> comfortable with.  I used "evaluate" purely as a descriptive term to what
> we are doing.  Whatever else fits will work with me.
>
>> OTOH, it seems like Python-based breakpoints have two conditions now
>> (or at least two kinds of conditions one has to think about).
>> One set with the "condition" command and one from the "evaluate" API function.
>> IWBN to have only one, at least conceptually.  Maybe you could rename
>> "evaluate" to "condition" (or some such, I realize there's already a
>> "condition"), and have the default be to use the CLI condition.
>
>
> This is something that is always a ponder-point for Python API hackers.
> There are lots of places where "things" have worked in CLI for many years,
> and we (well, I) do not want to deprecate existing CLI functionality
> while expanding other areas.  I'm not sure what the path is from here.
> Overloading the condition API seems like a pain for the script-writer to
> figure out if it is a Python scripted condition, or a CLI scripted
> condition.  That is why I ended up choosing a different name.

Yeah, a simple overloading of the condition API seems problematic.
Though conceptually having the value either be a string of text or a
method/closure doesn't seem entirely clumsy.
If it's a string it's a standard CLI condition, and if it's a
procedure(function/method/closure) it's Python.
[A simple test might be whether the condition has the __call__ attribute.]
From a u/i perspective is that better?  I don't know but it feels so.
[E.g., one doesn't have to deal with possible confusion over
gdb.Breakpoint having two conditions.]
There may be some implementation details to work out of course.