Re: [patch] [python] Expose some breakpoint operations to Python

Phil Muldoon
2011-06-23T15:57:14Z

Pedro Alves <pedro@codesourcery.com> writes:

> On Thursday 23 June 2011 15:46:14, Phil Muldoon wrote:

> No plan yet. I just started out by adding a single breakpoint_ops instance
> that handles all breakpoint types that currently aren't using breakpoint_ops,
> and moving all default actions to the corresponding callback. This would
> be straightforward if the API is a good fit. But take a look at
> the fallback code in print_one_breakpoint_location, and print_one_breakpoint.
> Breakpoints with multiple locations are handled a bit differently than
> just a fallbach, with multiple calls to print_one_breakpoint. Maybe
> we could move all that !print_one code inside the new print_one
> callback?

Yes the multiple locations thing is a bit weird. But from (my Python)
point of view, that is an internal abstraction that the Python glue code
will absorb. I don't think this will affect Python API users too much.

> Not sure. Also, maybe we should split printing the "what" from
> the "address"?

This may be an external API change. But we can always abstract that from
the user by using keywords in the Python side instead of
actual positional arguments.

> Not sure either, but it looks bizarre to me that
> current print_one implementations need to know to call annotate_field.
> Then there are implementations that print free form text
> surrounding those values, e.g., see print_one_catch_vfork. These
> issues certainly inpact the python api.

The annotate_field thing was somewhat interesting. I take care of this
for the Python user though so I am not too worried about that. Where the
field annotation happens is not a big deal for "me", we can move it from
the py-breakpoint code back to some new refactored breakpoint.c code.
The user does not need to know about this.

> (You'll also note that the ->insert_location and
> ->remove_location call sites reveal that something isn't fully
> abstracted, since failing to insert different kinds of breakpoints
> leads to different reactions in the code).
>
> Note, the doesn't even compile.

I'm not sure about this. The Python breakpoints are just regular
breakpoints - we don't even support catchpoints at this point, they are
just too flaky to wrap right now. In fact breakpoints created in Python
- even the internal ones - are just regular old breakpoints which we
masquerade to the user.

> Something else that's obviously missing with exposing breakpoint_ops
> to python as is (yes I know you haven't exported all of it), is that most
> certainly users will want to be able to call the "super" method. That is,
> say, e.g, override, the ->check_status method, and still call the default
> method, whatever it was.

I actually have no (solid) plans to expose anymore of the breakpoint
operations than are already exposed in this patch. The other operations
require substantial knowledge of GDB's inner-workings and access to
non-exported data-structures to be feasible at this youngling Python
stage. It is not something that won't ever happen, but it won't happen
for awhile. So in this context, refactorings should not affect the Python code.

> Well, what does grep tell us?
>
> breakpoint.c: PRINT_SRC_ONLY: Means we printed something, and we do *not* desire
> breakpoint.c: PRINT_SRC_ONLY: Means we printed something, but there is no need
> breakpoint.c: if (val == PRINT_SRC_ONLY
> breakpoint.h: PRINT_SRC_ONLY,
> infrun.c: case PRINT_SRC_ONLY:
>
> Looks like it's never used? Maybe gdbtk uses it, haven't checked.
> Certainly a good example of that cleaning up the house a little before
> exposing the API is a good idea.

Yeah I did the grep thing too. I ended up exporting it because it was
there. We can just remove it if it is too flaky. While we have a
strict philosophy on avoiding API changes to the Python API, we can
always add content later.

But I take your point. I'm trying to position this patch so that users
can access a limited subset of printing breakpoint operations. The
time-line is important here too. If this refactoring internally is just
going to take a few weeks, hey, no big deal, I can just wait and
adjust. But if it is a long term thing, I think we could expose the
limited functionality we expose now.

Cheers

Phil

--
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham (USA), Brendan Lane (Ireland), Matt Parson
(USA), Charlie Peters (USA)