Re: [RFC - Python Scripting] New method gdb.Symtab.blocks_iterator - docs included

[Tom Tromey <tromey@redhat.com>]

>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:

Siva> While the work on Objfile.symtabs (or Objfile.iterator) is pending, I
Siva> want to complete the API for the exploratory path Objfile => Symtabs
Siva> => Blocks => Symbols.  This patch adds the missing link Symtabs =>
Siva> Blocks.  The method Symtab.blocks_iterator returns an iterator to
Siva> iterate over the scope blocks of a symtab.  The patch is attached and
Siva> the ChangeLog is as follows:

Thanks.

Siva> +   ** A new method 'blocks_iterator' on gdb.Symtab objects which returns
Siva> +      an iterator to iterate over the scope blocks (gdb.Block objects) of
Siva> +      the symbol table (gdb.Symtab object).

It seems to me that the Symtab itself could provide the iterator so you
could just write:

    for block in symtab:
      ...

The precedent here is that gdb.Block is also an iterator over symbols.

Alternatively, 'Symtab.blocks'.
I find 'blocks_iterator' a bit too wordy somehow.

What do you think of those?

Siva> +   /* The iterator holds a reference to the symtab_object.  */
Siva> +   Py_INCREF (self);
Siva> + 
Siva> +   iter = PyObject_New (symtab_blocks_iterator_object,
Siva> +                        &symtab_blocks_iterator_object_type);
Siva> +   if (iter)
Siva> +     {
Siva> +       iter->symtab_obj = symtab_obj;
Siva> +       iter->iter_index = 0;
Siva> +     }
Siva> + 
Siva> +   return (PyObject *) iter;

This leaks a reference to 'self' if PyObject_New fails.
Moving the incref into the 'if' would fix it.

Siva> + static void
Siva> + symtab_blocks_iterator_dealloc (PyObject *self)
Siva> + {
Siva> +   symtab_blocks_iterator_object *iter;
Siva> + 
Siva> +   /* Decrement the reference to the symtab_object.  */
Siva> +   if (self)

Can you ever get here with self==NULL?
I expect not, but I am not 100% sure.
If not, remove the if.

Siva> +     {
Siva> +       iter = (symtab_blocks_iterator_object *) self;
Siva> +       Py_DECREF ((PyObject *) iter->symtab_obj);

It might be a little cleaner if symtab_obj is just a plain PyObject*
and you downcast it in the one place that uses it.

Siva> +   block_object = block_to_block_object (block, symtab->objfile);
Siva> +   if (! block_object)
Siva> +     {
Siva> +       PyErr_SetString (PyExc_RuntimeError,
Siva> +                        _("Unable to get the next gdb.Block object."));

If block_to_block_object fails, then the error will already be set.
I think it is generally better to propagate the original exception in
cases like this.  Otherwise, the new exception may obscure some more
fundamental error.

Siva> +   (iter->iter_index)++;

Also I'm curious if an error should invalidate the iterator in some way.

Siva> ! # Proc to setup and get the symbol table in the Python environment.
Siva> ! proc setup_python_env { line_no } {
Siva> !     gdb_breakpoint $line_no
Siva> !     gdb_continue_to_breakpoint "Block break here."
Siva> !     gdb_py_test_silent_cmd "python def func_symbol(block): return block.function" "Define a func to get the function symbols" "0"

If you run the same set of tests twice from a .exp you should use
with_test_prefix to ensure the tests have unique names.

Siva> + # Compile the source file as a C++ file and test again.
Siva> + if { [prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}] } {

We were recently discussing that it is preferable to give each
executable its own name, so that if the test fails it is simpler to
reproduce the problem from outside dejagnu.

So, please choose a new name for the executable here.

Tom