Re: [PATCH v3] Add more methods to gdb.Progspace

[Simon Marchi <simark@simark.ca>]

On 2018-09-16 08:48, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> LGTM, I just noted some minor comments.
> Simon> I wish there was more consistency in how the methods are
> documented (in the
> Simon> progspace_object_methods array), but that should be another 
> patch.
> 
> Send more details and let's agree on some kind of standard here.
> I think these doc strings are mostly an afterthought at the moment, but
> there's no reason they should be.

In these:

+  { "solib_name", pspy_solib_name, METH_VARARGS,
+    "solib_name (Long) -> String.\n\
+Return the name of the shared library holding a given address, or 
None." },
+  { "block_for_pc", pspy_block_for_pc, METH_VARARGS,
+    "Return the block containing the given pc value, or None." },
+  { "find_pc_line", pspy_find_pc_line, METH_VARARGS,
+    "find_pc_line (pc) -> Symtab_and_line.\n\
+Return the gdb.Symtab_and_line object corresponding to the pc value." 
},
+  { "is_valid", pspy_is_valid, METH_NOARGS,
+    "is_valid () -> Boolean.\n\
+Return true if this program space is valid, false if not." },

Two things stand out:

- Some of them have this "Prototype -> Return type" form, some don't.
- solib_name uses Long to describe the parameter while find_pc_line uses 
pc.  They should be the same, since they both take a pc.

I like the -> notation, and that's what cpython uses, for example if you 
do "help(str.find)".  cpython seems to describe parameter using their 
names, so from the above, "pc" would be the right one.  I think that 
makes sense, since

1. long doesn't exist in Python 3 (and it should be long, not Long)
2. we could accept different types for pc, either an int or a gdb.Value 
for example
3. It's not always clear just by the type what the parameter means.

Simon