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

[Siva Chandra <sivachandra@google.com>]

[-- Attachment #1: Type: text/plain, Size: 3342 bytes --]

Thanks Eli and Tom for taking a look.

Tom, I have addressed all your comments.  For some of them, I have my
own comments below.  The updated patch is attached.

Code -
2012-04-10  Siva Chandra Reddy  <sivachandra@google.com>

        Add a new method gdb.Symtab.blocks to iterate
        over the scope blocks of a symbol table.
        * NEWS (Python scripting): Add entry about the new method.
        * python/py-symtab.c (symtab_blocks_iterator_object): New
        iterator type to iterate over the scope blocks of a symtab.
        (stpy_blocks): New function which implements the new method.
        (symtab_blocks_iterator_dealloc): New function which serves
        as the tp_dealloc function for symtab_blocks_iterator_object.
        (symtab_blocks_iterator_iter): New function which serves as
        the tp_iter function for symtab_blocks_iterator_object.
        (symtab_blocks_iterator_iternext): New function which serves as
        the tp_iternext function for symtab_blocks_iterator_object.
        (gdbpy_initialize_symtabs): Add initializations for the new
        iterator type.

Docs -
2012-04-10  Siva Chandra Reddy  <sivachandra@google.com>

        * gdb.texinfo (Symbol Tables In Python): Add description about
        the new method gdb.Symtab.blocks.

Tests -
2012-04-10  Siva Chandra Reddy  <sivachandra@google.com>

        * gdb.python/py-symtab.exp: Add tests to test the new method
        gdb.Symtab.blocks.

Tom > It seems to me that the Symtab itself could provide the iterator so you
Tom > could just write:
Tom >
Tom >    for block in symtab:
Tom >      ...
Tom >
Tom > The precedent here is that gdb.Block is also an iterator over symbols.
Tom >
Tom > Alternatively, 'Symtab.blocks'.
Tom > I find 'blocks_iterator' a bit too wordy somehow.
Tom >
Tom > What do you think of those?

I have picked Symtab.blocks.  I personally don't like making Symtab
iterable as blocks are just one of them.

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."));

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

block_to_block_object just returns NULL on failure.  Am I missing something?

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

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

Since the iterator exists only in Python environment, my opinion is
that if the iterator gets invalidated, the execution should never
reach this place.   Do you see something else?

Tom> We were recently discussing that it is preferable to give each
Tom> executable its own name, so that if the test fails it is simpler to
Tom> reproduce the problem from outside dejagnu.
Tom>
Tom> So, please choose a new name for the executable here.

The source file py-symbol.c is used by py-symtab.exp as well as by
py-symbol.exp.  Hence, I have changed in py-symtab.exp to use
executables py-symbol-symtab and py-symbol-symtab-cc.

Thanks,
Siva Chandra

[-- Attachment #2: blocks_iterator_patch_v2.txt --]
[-- Type: text/plain, Size: 14286 bytes --]

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.506
diff -c -p -r1.506 NEWS
*** NEWS	2 Apr 2012 07:32:31 -0000	1.506
--- NEWS	10 Apr 2012 07:35:13 -0000
***************
*** 32,37 ****
--- 32,41 ----
    ** A new method 'referenced_value' on gdb.Value objects which can
       dereference pointer as well as C++ reference values.
  
+   ** A new method 'blocks' on gdb.Symtab objects which returns an
+      iterator to iterate over the scope blocks (gdb.Block objects) of
+      the symbol table (gdb.Symtab object).
+ 
  * GDBserver now supports stdio connections.
    E.g. (gdb) target remote | ssh myhost gdbserver - hello
  
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.939
diff -c -p -r1.939 gdb.texinfo
*** doc/gdb.texinfo	28 Mar 2012 21:31:46 -0000	1.939
--- doc/gdb.texinfo	10 Apr 2012 07:35:21 -0000
*************** if it is invalid at the time the method 
*** 24392,24397 ****
--- 24392,24403 ----
  @defun Symtab.fullname ()
  Return the symbol table's source absolute file name.
  @end defun
+ 
+ @defun Symtab.blocks ()
+ Return an iterator with which the user can iterate over the symbol
+ scope blocks (@code{gdb.Block} objects) of the symbol table
+ (@code{gdb.Symtab} object).  @xref{Blocks In Python}.
+ @end defun
  @end table
  
  @node Breakpoints In Python
Index: python/py-symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-symtab.c,v
retrieving revision 1.8
diff -c -p -r1.8 py-symtab.c
*** python/py-symtab.c	4 Jan 2012 08:17:25 -0000	1.8
--- python/py-symtab.c	10 Apr 2012 07:35:21 -0000
***************
*** 20,25 ****
--- 20,26 ----
  #include "defs.h"
  #include "charset.h"
  #include "symtab.h"
+ #include "block.h"
  #include "source.h"
  #include "python-internal.h"
  #include "objfiles.h"
*************** typedef struct stpy_symtab_object {
*** 36,42 ****
--- 37,63 ----
    struct stpy_symtab_object *next;
  } symtab_object;
  
+ /* Iterator object type whose instance serves as an iterator over the
+    scope blocks in the underlying symtab of a symtab_object.  */
+ 
+ typedef struct stpy_blocks_iterator_object
+ {
+   PyObject_HEAD
+ 
+   /* A reference to the symtab_object from which this iterator was
+      created.  */
+ 
+   PyObject *symtab_obj;
+ 
+   /* Index of the block object currently pointed to by the
+      iterator.  */
+ 
+   int iter_index;
+ } symtab_blocks_iterator_object;
+ 
  static PyTypeObject symtab_object_type;
+ static PyTypeObject symtab_blocks_iterator_object_type;
+ 
  static const struct objfile_data *stpy_objfile_data_key;
  
  /* Require a valid symbol table.  All access to symtab_object->symtab
*************** stpy_is_valid (PyObject *self, PyObject 
*** 153,158 ****
--- 174,205 ----
    Py_RETURN_TRUE;
  }
  
+ /* Implementation of gdb.Symtab.blocks (self) -> iterator.
+    Returns an iterator with which a user can iterator over the scope
+    blocks (gdb.Block objects) of the underlying symtab.  */
+ 
+ static PyObject *
+ stpy_blocks (PyObject *self, PyObject *args)
+ {
+   struct symtab *symtab = NULL;
+   symtab_blocks_iterator_object *iter = NULL;
+   symtab_object *symtab_obj = (symtab_object *) self;
+ 
+   STPY_REQUIRE_VALID (self, symtab);
+ 
+   iter = PyObject_New (symtab_blocks_iterator_object,
+                        &symtab_blocks_iterator_object_type);
+   if (iter)
+     {
+       iter->symtab_obj = (PyObject *) symtab_obj;
+       iter->iter_index = 0;
+       /* The iterator holds a reference to the symtab_object.  */
+       Py_INCREF (self);
+     }
+ 
+   return (PyObject *) iter;
+ }
+ 
  static PyObject *
  salpy_str (PyObject *self)
  {
*************** stpy_dealloc (PyObject *obj)
*** 193,199 ****
    symtab->symtab = NULL;
  }
  
- 
  static PyObject *
  salpy_get_pc (PyObject *self, void *closure)
  {
--- 240,245 ----
*************** del_objfile_sal (struct objfile *objfile
*** 431,436 ****
--- 477,535 ----
      }
  }
  
+ /* The tp_dealloc function of symtab_blocks_iterator_object_type.  */
+ 
+ static void
+ symtab_blocks_iterator_dealloc (PyObject *self)
+ {
+   symtab_blocks_iterator_object *iter;
+ 
+   iter = (symtab_blocks_iterator_object *) self;
+   /* Decrement the reference to the symtab_object.  */
+   Py_DECREF (iter->symtab_obj);
+ }
+ 
+ /* The tp_iter function of symtab_blocks_iterator_object_type.  */
+ 
+ static PyObject *
+ symtab_blocks_iterator_iter (PyObject *self)
+ {
+   Py_INCREF (self);
+   return self;
+ }
+ 
+ /* The tp_iternext function of symtab_blocks_iterator_object_type.  */
+ 
+ static PyObject *
+ symtab_blocks_iterator_iternext (PyObject *self)
+ {
+   PyObject *block_object;
+   symtab_blocks_iterator_object *iter;
+   symtab_object *symtab_obj;
+   struct symtab *symtab;
+   struct block *block;
+ 
+   iter = (symtab_blocks_iterator_object *) self;
+ 
+   STPY_REQUIRE_VALID (iter->symtab_obj, symtab); 
+   symtab_obj = (symtab_object *) iter->symtab_obj;
+ 
+   if (iter->iter_index >= symtab->blockvector->nblocks)
+     return NULL;
+ 
+   block = symtab->blockvector->block[iter->iter_index];
+   block_object = block_to_block_object (block, symtab->objfile);
+   if (! block_object)
+     {
+       PyErr_SetString (PyExc_RuntimeError,
+                        _("Unable to get the next gdb.Block object."));
+       return NULL;
+     }
+ 
+   (iter->iter_index)++;
+   return block_object;
+ }
+ 
  void
  gdbpy_initialize_symtabs (void)
  {
*************** gdbpy_initialize_symtabs (void)
*** 442,447 ****
--- 541,550 ----
    if (PyType_Ready (&sal_object_type) < 0)
      return;
  
+   symtab_blocks_iterator_object_type.tp_new = PyType_GenericNew;
+   if (PyType_Ready (&symtab_blocks_iterator_object_type) < 0)
+     return;
+ 
    /* Register an objfile "free" callback so we can properly
       invalidate symbol tables, and symbol table and line data
       structures when an object file that is about to be
*************** gdbpy_initialize_symtabs (void)
*** 458,463 ****
--- 561,570 ----
    Py_INCREF (&sal_object_type);
    PyModule_AddObject (gdb_module, "Symtab_and_line",
  		      (PyObject *) &sal_object_type);
+ 
+   Py_INCREF (&symtab_blocks_iterator_object_type);
+   PyModule_AddObject (gdb_module, "BlockIterator",
+ 		      (PyObject *) &symtab_blocks_iterator_object_type);
  }
  
  \f
*************** Return true if this symbol table is vali
*** 477,482 ****
--- 584,593 ----
    { "fullname", stpy_fullname, METH_NOARGS,
      "fullname () -> String.\n\
  Return the symtab's full source filename." },
+   { "blocks", stpy_blocks, METH_NOARGS,
+     "blocks () -> iterator.\n\
+ Return an iterator over the scope blocks (gdb.Block objects) of the\n\
+ underlying Symtab." },
    {NULL}  /* Sentinel */
  };
  
*************** static PyTypeObject sal_object_type = {
*** 562,564 ****
--- 673,706 ----
    0,				  /*tp_members */
    sal_object_getset		  /*tp_getset */
  };
+ 
+ static PyTypeObject symtab_blocks_iterator_object_type = {
+   PyObject_HEAD_INIT (NULL)
+   0,                              /*ob_size*/
+   "gdb.BlockIterator",            /*tp_name*/
+   sizeof (symtab_blocks_iterator_object),  /*tp_basicsize*/
+   0,                              /*tp_itemsize*/
+   symtab_blocks_iterator_dealloc, /*tp_dealloc*/
+   0,                              /*tp_print*/
+   0,                              /*tp_getattr*/
+   0,                              /*tp_setattr*/
+   0,                              /*tp_compare*/
+   0,                              /*tp_repr*/
+   0,                              /*tp_as_number*/
+   0,                              /*tp_as_sequence*/
+   0,                              /*tp_as_mapping*/
+   0,                              /*tp_hash */
+   0,                              /*tp_call*/
+   0,                              /*tp_str*/
+   0,                              /*tp_getattro*/
+   0,                              /*tp_setattro*/
+   0,                              /*tp_as_buffer*/
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_ITER,  /*tp_flags*/
+   "Iterator over GDB blocks",     /*tp_doc */
+   0,                              /*tp_traverse */
+   0,                              /*tp_clear */
+   0,                              /*tp_richcompare */
+   0,                              /*tp_weaklistoffset */
+   symtab_blocks_iterator_iter,    /*tp_iter */
+   symtab_blocks_iterator_iternext,  /*tp_iternext */
+ };
Index: testsuite/gdb.python/py-symtab.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.python/py-symtab.exp,v
retrieving revision 1.8
diff -c -p -r1.8 py-symtab.exp
*** testsuite/gdb.python/py-symtab.exp	7 Feb 2012 19:42:27 -0000	1.8
--- testsuite/gdb.python/py-symtab.exp	10 Apr 2012 07:35:21 -0000
*************** load_lib gdb-python.exp
*** 20,37 ****
  
  set testfile "py-symbol"
  set srcfile ${testfile}.c
! set binfile ${objdir}/${subdir}/${testfile}
! if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
!     untested "Couldn't compile ${srcfile}"
      return -1
  }
  
- # Start with a fresh gdb.
- gdb_exit
- gdb_start
- gdb_reinitialize_dir $srcdir/$subdir
- gdb_load ${binfile}
- 
  # Skip all tests if Python scripting is not enabled.
  if { [skip_python_tests] } { continue }
  
--- 20,30 ----
  
  set testfile "py-symbol"
  set srcfile ${testfile}.c
! 
! if { [prepare_for_testing ${testfile}.exp ${testfile}-symtab ${srcfile}] } {
      return -1
  }
  
  # Skip all tests if Python scripting is not enabled.
  if { [skip_python_tests] } { continue }
  
*************** if ![runto_main] then {
*** 41,69 ****
  }
  
  global hex decimal
- 
- # Setup and get the symbol table.
  set line_no [gdb_get_line_number "Block break here."]
! gdb_breakpoint $line_no
! gdb_continue_to_breakpoint "Block break here."
! gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
! gdb_py_test_silent_cmd "python sal = frame.find_sal()" "Get block" 0
! gdb_py_test_silent_cmd "python symtab = sal.symtab" "Get block" 0
! 
! # Test sal.
! gdb_test "python print sal.symtab" ".*gdb.python/py-symbol.c.*" "Test symtab"
! gdb_test "python print sal.pc" "${decimal}" "Test sal.pc"
! gdb_test "python print sal.line" "$line_no" "Test sal.line"
! gdb_test "python print sal.is_valid()" "True" "Test sal.is_valid"
! 
! # Test symbol table.
! gdb_test "python print symtab.filename" ".*gdb.python/py-symbol.c.*" "Test symtab.filename"
! gdb_test "python print symtab.objfile" "<gdb.Objfile object at ${hex}>" "Test symtab.objfile"
! gdb_test "python print symtab.fullname()" "testsuite/gdb.python/py-symbol.c.*" "Test symtab.fullname"
! gdb_test "python print symtab.is_valid()" "True" "Test symtab.is_valid()"
! 
! # Test is_valid when the objfile is unloaded.  This must be the last
! # test as it unloads the object file in GDB.
! gdb_unload
! gdb_test "python print sal.is_valid()" "False" "Test sal.is_valid"
! gdb_test "python print symtab.is_valid()" "False" "Test symtab.is_valid()"
--- 34,93 ----
  }
  
  global hex decimal
  set line_no [gdb_get_line_number "Block break here."]
! 
! # Proc to setup and get the symbol table in the Python environment.
! proc setup_python_env { line_no } {
!     gdb_breakpoint $line_no
!     gdb_continue_to_breakpoint "Block break here."
!     gdb_py_test_silent_cmd "python def func_symbol(block): return block.function" "Define a func to get the function symbols" "0"
!     gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
!     gdb_py_test_silent_cmd "python sal = frame.find_sal()" "Get block" 0
!     gdb_py_test_silent_cmd "python symtab = sal.symtab" "Get block" 0
!     gdb_py_test_silent_cmd "python block_list = list(symtab.blocks())" "Create a list of blocks" 0
!     gdb_py_test_silent_cmd "python func_symbols = map(func_symbol, block_list)" "Create a list of function symbols" 0
!     gdb_py_test_silent_cmd "python func_names = map(str, func_symbols)" "Create a list of function names" 0
! }
! 
! setup_python_env $line_no
! 
! with_test_prefix "C Test" {
!     # Test sal.
!     gdb_test "python print sal.symtab" ".*gdb.python/py-symbol.c.*" "Test symtab"
!     gdb_test "python print sal.pc" "${decimal}" "Test sal.pc"
!     gdb_test "python print sal.line" "$line_no" "Test sal.line"
!     gdb_test "python print sal.is_valid()" "True" "Test sal.is_valid"
!     
!     # Test symbol table.
!     gdb_test "python print symtab.filename" ".*gdb.python/py-symbol.c.*" "Test symtab.filename"
!     gdb_test "python print symtab.objfile" "<gdb.Objfile object at ${hex}>" "Test symtab.objfile"
!     gdb_test "python print symtab.fullname()" "testsuite/gdb.python/py-symbol.c.*" "Test symtab.fullname"
!     gdb_test "python print symtab.is_valid()" "True" "Test symtab.is_valid()"
!     gdb_test "python print 'func' in func_names" "True" "Test that there is a symbol for the function func"
!     gdb_test "python print 'main' in func_names" "True" "Test that there is a symbol for the function main"
!       
!     
!     # Test is_valid when the objfile is unloaded.  This must be the last
!     # test as it unloads the object file in GDB.
!     gdb_unload
!     gdb_test "python print sal.is_valid()" "False" "Test sal.is_valid"
!     gdb_test "python print symtab.is_valid()" "False" "Test symtab.is_valid()"
! }
! 
! # Compile the source file as a C++ file and test again.
! if { [prepare_for_testing $testfile.exp ${testfile}-symtab-cc $srcfile {debug c++}] } {
!     return -1
! }
! 
! if ![runto_main] {
!    return -1
! }
! 
! setup_python_env $line_no
! 
! with_test_prefix "C++ Test" {
!     gdb_test "python print 'func(int)' in func_names" "True" "Test that there is a symbol for the function func"
!     gdb_test "python print 'main(int, char**)' in func_names" "True" "Test that there is a symbol for the function main"
!     gdb_test "python print 'SimpleClass::seti(int)' in func_names" "True" "Test that the function SimpleClass::seti(int) is present"
!     gdb_test "python print 'SimpleClass::valueofi()' in func_names" "True" "Test that the function SimpleClass::valueofi() is present"
! }