This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Doug Evans <dje@google.com> writes:

>> 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 ;)
>>
>
> The only thing I would insist on is consistency (and documentation of it).

I don't particularly like the stop_p name, or any predicate suffix.
Originally the function was named "eval", but that seemed to stir up
objections.  FWIW, I think given that existing functions don't use the
predicate notation, we should not either, here.  Now, what do we call
the function? ;) I really want to move past this point;  it has been a
productive review in exploring the growing API.  But we still don't have
a convention that everyone agrees on.  I'll defer to the directive
prerogative of the maintainers here


>> - 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?
>
>
> For things like this I like to start slow: "It's easier to relax
> restrictions than it is to impose them after the fact."  If it turns out
> there's a reasonable use for having both and the first release doesn't
> support that, we can easily add the support later (I think!).  But if it
> turns out that supporting both causes a lot of
> trouble/confusion/wasted-time/whatever, and there's no real need for the
> feature anyway, then removing it later will be a lot harder (if not
> impossible).
>
> So my vote would be to not support both in the first pass.

Okay, I will make the condition mutually exclusive.  If one exists in
the CLI then we will not allow the implementation (or rather, bar the
execution) of a function.  I'm not sure on the CLI condition side.  That
would require us to do some grokking of the Python object carried around
in the breakpoint struct.  That would implement some Python specific
code in breakpoint.c. If that is fine, then I'll implement this today.

Cheers,

Phil


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]