Re: [patch][python] Add symbol, symbol table and frame block support to GDB API

[Phil Muldoon <pmuldoon at redhat dot com>]

> 
>> +* Frames In Python::            Accessing inferior stack frames from Python.
>> +* Blocks In Python::            Accessing frame blocks from Python.
>> +* Symbols In Python::           Python representation of Symbols.
>> +* Symbol Tables In Python::     Symbol table representation in Python.
>>  * Lazy Strings In Python::      Python representation of lazy strings.
> I would prefer a consistent phrasing here: "Python representation of FOO".


OK.

>> +@defmethod Frame block
>> +Returns the frame's code block.  (@pxref{Blocks In Python}).
> 
> Please use "@xref{Blocks In Python}." here (without the parens).
> @pxref is for references in the middle or end of a sentence.
> Stand-alone cross-references should use @xref.

OK.

 
>> +@defmethod Frame select
>> +Set this frame to be the user's selected frame.
> 
> I got confused by "user's selected frame" in this description.  If you
> meant the "selected frame" in the sense we use that in the node
> "Stack", then I suggest to drop the "user's" part and add a
> cross-reference to the node "Stack" where this term is explained.

OK, will drop user in this context.

>> +All of the name-scope contours of a program are represented as 'block'
> 
> What are "contours"?  Is this a widespread enough terminology to be
> understood without defining it first?  If not, I suggest to define it.


I really agonized over the whole block description, as there is no (as
far as I know) analogue to the GDB CLI for blocks.  It's revealing an
API that GDB uses to manipulate blocks in a frame.  I could use some
help making this more palatable enough to the consumer of the API,
over trying to teach the user compiler/runtime semantics (which is not
the purpose of the GDB manual).  What do you think?


 
> Also, we don't use single quotes in Texinfo, as in 'block'.  If you
> meant to make it stand out, I suggest @dfn{block objects} instead.  If
> you meant to quote it, please use double quotes, as in ``block''.


I'll use the dfn comment in this case, thanks.


>> +                                       Each @code{gdb.Block} object
>> +has to be sourced, and parented from a @code{gdb.Frame} object.
> 
> I don't understand what it means ``sourced and parented from''.


I'll delete this as this is obvious (a block has to belong to a
frame).  The sourced and parented from is confusing and
redundant. Thanks.

 
>> +                              This can be summarized as: each block
>> +represents one name scope; each lexical context has its own block.
> 
> Are you sure people without solid background in compilers will
> understand this?


See above on the block general comment. ;)


> 
>> +@findex gdb.block_for_pc pc
> 
> findex entries should not include arguments, just the name of the
> function.

OK.

> 
>> +@defun block_for_pc pc
>> +Return the @code{gdb.Block} containing the given @code{pc} value.  If the
>                                                     ^^^^^^^^^
> Arguments should have a @var markup, not @code.

OK.

> 
>> +@defivar Block superblock
>> +This attribute holds the superblock containing this block represented as a
> 
> Isn't it better to say
> 
>   This attribute holds the block containing this block
> 
> ?  After all, the value of the superblock attribute is just another
> block, right?

OK.

> 
>> +@value{GDBN} represents the name of every variable, function and type
>> +as a symbol (@pxref{Symbols, ,Examining the Symbol Table}).
> 
> I think, from the GDB user POV, a name of a variable, function, and
> type is just a string, not a symbol.

But in the context of the Python API, they are represented as symbols?  How should I word this?

>> +@findex gdb.lookup_symbol name [block] [domain]
> 
> Arguments in @findex again.

OK, and for all others too!

> 
>> +@defivar Symbol print_name
>> +The name of the symbol in a form suitable for output.  This is either
>> +name or linkage_name, depending on whether the user asked @value{GDBN}
> 
> Please give "name" and "linkage_name" the @code markup here, since
> they are names of attributes.

OK.
 
>> +@defivar Symbol is_argument
>> +Returns @code{True} if the symbol is an argument of a function.
> 
> "Returns"? maybe you meant "holds"?  This is an attribute, not a
> method, right?

OK, for all other examples of definitions too.

> 
>> +@item SYMBOL_VAR_DOMAIN
>> +This domain contains variables, function names, typedef names and enum
>> +type values.
> 
> Isn't this description too C-centric?  What about other languages?

This is straight from the comments that represent this enum.  I think
from a GDB point of view, this is correct. I'd have to ask other
language hackers if this holds true? Joel, what about ADA?

 
>> +@item SYMBOL_STRUCT_DOMAIN
>> +This domain holds struct, union and enum type names.
>> +@findex SYMBOL_LABEL_DOMAIN
>> +@findex gdb.SYMBOL_LABEL_DOMAIN
>> +@item SYMBOL_LABEL_DOMAIN
>> +This domain contains names of labels (for gotos).
> 
> Same here.

See above comment

>> +@item SYMBOL_LOC_REF_ARG
>> +Value address is an offset in arglist.
> 
> I couldn't understand what this means.


Yeah, this is a concept expressed in the comments of the code.  I'm
not sure how to word it any differently.


> 
>> +               Access to the @code{gdb.Symtab_and_line} object for
>> +each frame is returned from the @code{find_sal} method in
>> +@code{gdb.Frame} object (@pxref{Frames In Python}).
> 
> Something is wrong in this sentence ("Access to ... object ... is
> returned from ..." -- how can access be returned from somewhere?).

I'll reword it.


>> +For more information on @value{GDBN}'s symbol table management
>> +@xref{Symbols, ,Examining the Symbol Table}.
>    ^^^^^
> You want "see @ref" here, since "@xref" produces a capitalized "See",
> which will look wrong in the middle of a sentence.

OK.

>> +
>> +A @code{gdb.Symtab_and_line} object has the following attributes:
>> +
>> +@table @code
>> +@defivar Symtab_and_line symtab
>> +The symbol table object (@code{gdb.Symtab}) for this frame.
>> +This attribute is not writable.
>> +@end defivar
>> +
>> +@defivar Symtab_and_line pc
>> +Indicates the current program counter address.  This attribute is not
>> +writable.
>> +@end defivar
>> +
>> +@defivar Symtab_and_line line
>> +Indicates the current line number information for this object.
> 
> What is the ``current line information''?  Do you mean line number?
> If not, what kind of object is it?

Just line number in this context will be ok. Thanks!

>> +@defivar Symtab objfile
>> +The symbol table's backing object file. @xref{Objfiles In Python}.
>                                          ^^
> Two spaces, please.

OK.

>> +@defmethod Symtab fullname
>> +Return the symbol table's full source filename.
> 
> "Full" as in "absolute file name" or something else?
> 
> Do we need a NEWS entry for this?

Not sure. From my point of view I am merging code from Archer to FSF
GDB. I'll happily write one up if you think we need one? What do you
think?

Cheers,

Phil