Re: [python] [patch] PR python/12656 (API for special blocks)

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> This patch addresses PR python/12656.  It allows the user to determine if
Phil> a given gdb.Block is the static block, or the global block.
Phil> Additionally I added two new APIs to actually get the global/static
Phil> blocks on behalf of the user.

Thanks.

Phil> I also, because the global/static GDB fetch methods use const struct
Phil> *block, changed the Python code to use consts.  This required some minor
Phil> refactoring both inside and outside the Python API code.

Nice.  Looks pretty good.

Phil> +@defun Block.is_global ()
Phil> +@defun Block.is_static ()

Why are these methods rather than attributes?

Phil> diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
Phil> index 08d4455..5ef7d2e 100644
Phil> --- a/gdb/python/py-block.c
Phil> +++ b/gdb/python/py-block.c
Phil> @@ -28,7 +28,7 @@
Phil>  typedef struct blpy_block_object {
Phil>    PyObject_HEAD
Phil>    /* The GDB block structure that represents a frame's code block.  */
Phil> -  struct block *block;
Phil> +  const struct block *block;
Phil>    /* The backing object file.  There is no direct relationship in GDB
Phil>       between a block and an object file.  When a block is created also
Phil>       store a pointer to the object file for later use.  */
Phil> @@ -50,7 +50,7 @@ typedef struct {
Phil>    /* Pointer back to the original source block object.  Needed to
Phil>       check if the block is still valid, and has not been invalidated
Phil>       when an object file has been freed.  */
Phil> -  struct blpy_block_object *source;
Phil> +  const struct blpy_block_object *source;

Why make this one const?
It seems wrong given that we have to decref it when the iterator is
destroyed.

Phil> +      Py_INCREF (Py_None);
Phil> +      return Py_None;

I think it is more normal to use Py_RETURN_NONE here.

Phil>  gdb_test "python print block_iter.is_valid()" "False" \
Phil>           "Check block validity"
Phil> +

Gratuitous newline addition.

Phil> -extern struct value *value_of_variable (struct symbol *var, struct block *b);
Phil> +extern struct value *value_of_variable (struct symbol *var, const struct block *b);

This should probably wrap after the ",".

Tom