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

[Phil Muldoon <pmuldoon at redhat dot com>]

Tom Tromey <tromey@redhat.com> writes:

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

> Phil> +@defun Block.is_global ()
> Phil> +@defun Block.is_static ()
>
> Why are these methods rather than attributes?

Because is_valid is a method in py-block.c, and I decided to continue
the is_* APIs to be methods.  It confuses my *why* is_valid is a method
over an attribute, too.  I guess we missed this when we ported it over
from archer (several other classes, too).  I thought about changing
is_valid, but this would be an API break.


> 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.

Gratuitous on my part, apologies.

> Phil> +      Py_INCREF (Py_None);
> Phil> +      return Py_None;
>
> I think it is more normal to use Py_RETURN_NONE here.

Thanks.

> 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 ",".

Thanks.  I've modified the patch accordingly, but as these are trivial
changes I won't resubmit it until we reach a conclusion on the is_*
question (and we need a doc review, anyway).

Cheers,

Phil