Re: [RFC v5 14/18] gdb/python: add add_symbol () method to gdb.Block

[Andrew Burgess <aburgess@redhat.com>]

Andrew Burgess <aburgess@redhat.com> writes:

> Jan Vrany <jan.vrany@labware.com> writes:
>
>> This commit adds new method add_symbol () to gdb.Block objects.
>> A typical use of it is to add previously instantiated gdb.Symbol object
>> to block when interfacing with JIT compiler.
>>
>> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>> ---
>>  gdb/doc/python.texi                   |  5 +++
>>  gdb/python/py-block.c                 | 52 +++++++++++++++++++++++----
>>  gdb/testsuite/gdb.python/py-block.exp | 20 +++++++++++
>>  3 files changed, 71 insertions(+), 6 deletions(-)
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index 709ef7c9d1a..15f851e5fd0 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -6180,6 +6180,11 @@ The new block's @var{start}--@var{end} range must be within superblock's
>>  range and must not overlap with any block already contained in superblock.
>>  @end defun
>>  
>> +@defun Block.add_symbol (symbol)
>> +Add @var{symbol} to this block.  Both the block and the @var{symbol} must
>> +belong to the same compunit (@pxref{Compunits In Python}).
>> +@end defun
>> +
>>  @defun Block.is_valid ()
>>  Returns @code{True} if the @code{gdb.Block} object is valid,
>>  @code{False} if not.  A block object can become invalid if the block it
>> diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
>> index 43b15b6013e..7926808d4db 100644
>> --- a/gdb/python/py-block.c
>> +++ b/gdb/python/py-block.c
>> @@ -263,6 +263,43 @@ blpy_is_static (PyObject *self, void *closure)
>>    Py_RETURN_FALSE;
>>  }
>>  
>> +/* Implementation of gdb.Block.add_symbol (self, symbol).
>> +   Adds SYMBOL to this block.  */
>> +
>> +static PyObject *
>> +blpy_add_symbol (PyObject *self, PyObject *symbol_obj)
>> +{
>> +  const struct block *block;
>> +
>> +  BLPY_REQUIRE_VALID (self, block);
>> +
>> +  struct symbol *symbol = symbol_object_to_symbol (symbol_obj);
>> +  if (symbol == nullptr)
>> +    {
>> +      return PyErr_Format (PyExc_TypeError,
>> +			   _("The symbol argument is not valid gdb.Symbol"));
>> +    }
>> +
>> +  if (symbol->symtab ()->compunit() != block->global_block ()->compunit ())
>> +    {
>> +      return PyErr_Format (PyExc_TypeError,
>> +			   _("The symbol argument belongs to different "
>> +			     "compunit than block"));
>> +    }
>
> In both these `if` blocks, the '{' and '}' are not needed.  In the
> second one, is TypeError the best choice?  Would ValueError?  Or
> RuntimeError? be better?
>

I just realised I managed to send both these emails to the v5 thread,
rather than the v6 thread.  Sorry about that, I was jumping back and
forward looking at different past review comments, and got mixed up at
some point.

I think everything still applies to the v6 patches, if not then just
ignore me :)

The patches I actually downloaded are for sure the v6 ones though!

Sorry about that.

Thanks,
Andrew