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] Debug Methods in GDB Python


On Wed, Jan 1, 2014 at 4:23 AM, Siva Chandra <sivachandra@google.com> wrote:
> On Mon, Dec 16, 2013 at 9:56 AM, Doug Evans <xdje42@gmail.com> wrote:
>> I wonder, though, about the match method returning True/False vs the
>> pretty-printer way of the lookup object returning an object that then
>> does the printing.
>> [In the debug method case the lookup method might want to return a
>> list if the method is overloaded.]
>> Unless there's a compelling reason to be different, I like
>> "Consistency Is Good", so that's what I'm shooting for.
>> [But if there is a compelling reason to be different, I'm not opposed
>> to being different here.]
>
> Though I had responded to this part before, I had made a note to
> revisit it. Some of the points I make are really subjective based on
> my personal feel.
>
> In the pretty printers world, a pretty printer is a callable which
> returns an object (which has a specific API) which can do the
> printing. So, a pretty printer is really not doing the printing, it
> returns a printer. For me, this is really odd: an object which is an
> instance of PrettyPrinter does not do the printing, but returns a
> printer in turn.

The concept feels sufficiently useful that I cope with the naming.

> I did not want debug methods to be like pretty printers. As in, a
> debug method should be _the_ method and not return a method. If a
> debug method were to be a match-er which returns a method, then we
> would need to enforce two APIs, one for the match-er and another for
> the method. Also, for debug methods, the method part will not be
> purely a method as it still needs to do argument matching.

That's a fair point, but we need to not conflate how things are named
with how they are used / what they look like.

I like having the object gdb calls to do the lookup return another
object that gdb calls to implement the functionality.

1) It supports more flexibility in how the user can organize things
(e.g., one "lookup" object can handle different "worker" objects).

2) The worker object can obtain state at runtime (it is constructed at
lookup time).

[I'm not sure I phrased that sufficiently well.  I'm trying to
enumerate what the pretty-printer way of doing things brings to the
table, so to speak.]

> As it stands in the latest patch, I feel the debug methods API is a
> bit verbose. The debug method base class has a "match" method and a
> "get_argtypes" method which need to be overridden by a specific debug
> method implementation. To cater to templates, both these methods
> currently take the "class_type" and "method_name" arguments. This
> makes the debug method objects almost stateless (almost because they
> have "name" and "enabled" fields). A simple way to fix this is to
> rename the "match" method to "init". The "init" method will be exactly
> the same as the match method. It will take exactly the same arguments
> as the "match" method and return True/False based on a match/mismatch.
> However, since it is named "init", it will be expected to save enough
> state so that the "get_argtypes" method need not take any arguments.

Note that this kind of simplification can fall out of the two step
lookup+worker approach: construction of the worker could save the
needed state so that it didn't have to be passed back to subsequent
methods.  [Blech.  "method" is getting too overloaded here. :-)]

["init" is a bit too close to __init__, but that could be fixed if one
went that route.]

> If we can agree on the basic API, I will send out a patch which
> addresses the formatting issues which Doug pointed out sometime back.
>
> Thanks,
> Siva Chandra
> [I have just noticed that it is a week shy of an year that I had sent
> my v1 patch for this feature. Hope this feature lands in 2014.]


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