Re: [RFC] A new command 'explore'

[Siva Chandra <sivachandra at google dot com>]

Thanks for taking a look Joel. My comments inline.

Siva > +import gdb

Joel > GDB already pre-imports module `gdb', and it's documented in the GDB
Joel > manual. So I think you can get rid of this import. Though, I wonder
Joel > if that's the recommended style or not. Tom?

This is the convention(?) in other files in gdb/python/lib/gdb/command.

Joel > It seems that the design is centered around these various *Explorer
Joel > classes implementing a defined interface. ?If that's the case,
Joel > I think it would make it clearer if they inherited from an abstract
Joel > class where you can then clearly describe what the interface is,
Joel > and what each element is expected to do. Put an assertion in each
Joel > abstract method to raise an exception if the method is not implemented
Joel > by the deriving class.
Joel >
Joel > Also, almost everything is declared @staticmethod. Is that typical
Joel > Python? I don't have enough experience with expert Python to really
Joel > say anything, but it almost seems like you're taking the problem
Joel > backwards. Let's imagine that you had one abstract Explorer class
Joel > which did all the generic stuff, and then a collection of *Explorer
Joel > classes which derived from it, you'd then only need a factory that
Joel > the commands would invoke, and then the rest would be standard python.
Joel > It would avoid all the @staticmethod, as well as the explicit class
Joel > names in the method invocations.
Joel >
Joel > This is not a request to change, but just some suggestions based
Joel > on my limited understanding of the code and of Python in the general.
Joel > This code is self contained, so it does not matter as much how it
Joel > is designed. Please do what you think is best.

I did have inheritance in my first implementation. But soon realized
that I am essentially implementing stateless functions. Hence, I
removed the inheritance, kept the classes, and made the functions
static (to imply that they are stateless). In fact, we need not have
the classes as well. Like you have noted in your penultimate sentence
above, this is really not an API for others to use. So, isn't an
abstract base class and inheritance an overkill for a Python
implementation here?

Siva > + ? ?def get_type_from_str(type_str):
Siva > + ? ? ? ?try:
Siva > + ? ? ? ? ? ?# Assume the current language to be C/C++ and make a try.
Siva > + ? ? ? ? ? ?return gdb.parse_and_eval("(%s *)0" %
type_str).type.target()
Siva > + ? ? ? ?except RuntimeError:
Siva > + ? ? ? ? ? ?# If assumption of current language to be C/C++
was wrong, then
Siva > + ? ? ? ? ? ?# lookup the type using the API.
Siva > + ? ? ? ? ? ?try:
Siva > + ? ? ? ? ? ? ? ?return gdb.lookup_type(type_str)
Siva > + ? ? ? ? ? ?except RuntimeError:
Siva> + ? ? ? ? ? ? ? ?return None

Joel > We really need to think about giving access to parameter values
Joel > from python.... That would make the code above a little easier,
Joel > as you'd be able to temporarily force the language to C...

Like I have said in my response to Tom, I am OK to take a look at this
once this patch is cleared.

Thanks,
Siva Chandra