Re: [patch] [python] Implement stop_p for gdb.Breakpoint

[Phil Muldoon <pmuldoon@redhat.com>]

Doug Evans <dje@google.com> writes:

>> I believe I have implemented all of the requests from the previous patch
>> discussion.  I eventually renamed eval to stop_p.  Also, recently, Tom
>> wrote a log_printf Python command that uses this feature. I've included
>> it here, along with a few alterations Tom had to make to Breakpoint
>> initialization to make that work.
>>
>> Here it is. What do you think?
>
> Hi.
> Some nits and comments:
>
> - "consistency is good", so if we go with _p for stop_p we need to go
> with _p for all predicates
>   - are we prepared for that?
>   - are there any existing predicates that don't have _p?
>   - does python have an existing convention?
>   [I used stop_p at the time for clarity's sake.  But I think these
> questions need to be asked.]

There are two instances that I can think of where we allow the user
to implement methods that we supply the interface for.  One is the
pretty-printer string, children and hint methods.  The other is the
patch I sent last week for the redesign of parameters.  None of those
use the _p for predicate style.  As far as I can tell (with the express
disclaimer I am don't hack on actual Python code that much), there does
not seem to be a convention.  I'll defer to real Python hackers
here. For my part, I don't have much of an opinion what we call it, or
if we should have a convention; I'll rely on the maintainers being
directive here ;)

> - I didn't see any tests for log-printf
>
> - log.py feels misnamed but since the name isn't exported to the user
> it's not important enough to find a better name
>
> - can printf get an error (e.g. bad memory access) and would one
> necessarily want execution to halt when that happens?
>   - I can imagine wanting to just see an error message and have
> execution continue
>   - OTOH while testing a log-printf I would want execution to stop if
> I misspelled something
>   - we don't have to add the functionality now, but IWBN to at least
> think about if/how we'd provide it
>
> - we probably should document log-printf in the manual

When I edited the original message, I guess I edited out a paragraph I
wanted to leave in! Anyway, I'm not sure we should include log-printf
in the original FSF inclusion, I added it here for a "real world"
case. Just illustrative. I think if we want to include it that is fine,
or Tom can submit it as a separate patch.  I should have probably asked
Tom first ;)  Anyway apologies for the confusion!

> - is the logic for deciding whether to stop correct?
>   E.g. if stop_p says "don't stop" and a condition says "stop" will
> execution continue?  It looks like it, but maybe I'm misunderstanding
> something.

The case of the user having an old-style GDB condition, and an
implementation of a "stop_p" is an odd one. I was inclined to disallow
it, but eventually decided against it.  There will be conflict if stop_p
and condition disagree.  My first thoughts are "stop" should always
trump "don't stop". What do you think?

Cheers

Phil