[RFC] Python Finish Breakpoints

[Kevin Pouget]

On Tue, Oct 25, 2011 at 1:36 PM, Kevin Pouget <kevin.pouget@gmail.com> wrote:
> On Tue, Oct 25, 2011 at 1:05 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>>
>> Kevin Pouget <kevin.pouget@gmail.com> writes:
>>
>> > On Mon, Oct 24, 2011 at 6:06 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>> >> Kevin Pouget <kevin.pouget@gmail.com> writes:
>> >>
>> >> I have some comments regarding the Python bits.
>> >
>> > Thanks for that, I replied inline
>>
>> Thanks.
>>
>> >>> @@ -5700,6 +5700,7 @@ init_raw_breakpoint_without_location (struct breakpoint *b,
>> >>>    b->frame_id = null_frame_id;
>> >>>    b->condition_not_parsed = 0;
>> >>>    b->py_bp_object = NULL;
>> >>> +  b->is_py_finish_bp = 0;
>>
>>
>> >Phil>> Is there any reason why this needs to be in the breakpoint struct?
>>
>> >Kevin> just to put it back in context (in was back in May ...), here is the
>> >Kevin> rational behind the flag:
>>
>>
>> > On Thu, May 19, 2011 at 6:20 PM, Tom Tromey <tromey@redhat.com> wrote:
>> >> gdbpy_is_stopped_at_finish_bp is not safe to call here -- it assumes the
>> >> GIL has been acquired, which it has not.  I would rather it not be changed
>> >> to acquire the GIL, however.  I think one of two other approaches would
>> >> be preferable.
>> >>
>> >> One way you could handle this is to add a new constant to enum bptype.
>> >> This is likely to be pretty invasive.
>> >>
>> >> Another way would be to add a flag to the struct breakpoint itself.
>> >>
>> >> Yet another way would be a new breakpoint_ops method.
>>
>> >Kevin> I've refactored the code according to your comment anyway, it make
>> >Kevin> sense, so now there are two version:
>>
>> First let me apologies for not picking up on these previous comments.
>>
>> My own personal opinion is to abstract the details to the GDB Python
>> code, instead of adding another flag to 'struct breakpoint'. That was
>> the original ethos of adding a pointer inside the breakpoint struct to
>> the Python breakpoint-object - so we can have access to the whole of the
>> breakpoint object without breaking out pieces of it here and there.
>
> yes, I totally agree with this opinion, and that's why I changed the
> code arguing,
> "what's for Python stays in Python" !
>
>> That being said, I don't want to delay this patch any further (and I'm
>> not sure why you cannot acquire the GIL in the accessor function?  There
>> is a performance hit involved in acquiring the GIL, maybe that). Tom
>> gave three options that make sense, so whatever works for you and Tom
>> will be fine.  Thanks for taking the time to refactor it.  Tom, what do
>> you think?
>
>
> I've got time for my patches, provided that they're not "forgotten",
> and that the
> discussions are constructive, I'll be happy to fix and refactor accordingly :)
>
> I've got no idea about the cost of Python functions in general, let
> see if that's
> what Tom had in mind.
>
>
> Cordially,
>
> Kevin


Hello,

I've updated this patch against the last GIT tree and fixed some typos
in the code, if you want to give it another look Tom...
I followed your suggestion (delete BP once the frame in gone) as well
as Phil's (move is_finish_py from 'struct breakpoint' to the Py
object, looks with encapsulation)


Thanks,

Kevin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Introduce-gdb.FinishBreakpoints.patch
Type: text/x-patch
Size: 54292 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20111028/e0af459d/attachment.bin>