Re: [RFC] Python Finish Breakpoints

[Kevin Pouget <kevin.pouget@gmail.com>]

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

> >> I think these comments should wrap? They wrap for me in emacs.
> >
> > I'm not sure about the exact meaning of 'wrap' here, but I assume it's
> > about the new line between computable and NULL; I've reformatted it.
>
> Yeah I meant, split the comments up.
>
> Cheers,
>
> Phil