[PATCH 1/7] [python] API for macros: abort or continuing macro iterators.

[PATCH 1/7] [python] API for macros: abort or continuing macro iterators.

[matt rice]

2011-08-23  Matt Rice  <ratmice@gmail.com>

	* macrocmd.c (print_macro_callback): Return a walk status
	and continue indefinitely.
	(print_one_macro): Ditto.
	* macrotab.c (foreach_macro): Return based on callback
	status.
	(foreach_macro_in_scope): Ditto.
	(macro_for_each): Return a macro walk completion result.
	(macro_for_each_in_scope): Ditto.
	* macrotab.h (macro_walk_status): New enum.
	(macro_walk_result): Ditto.
	(macro_callback_fn): Return a macro_walk_status.
	(macro_for_each, macro_for_each_in_scope): Return a macro_walk_result.
	* symtab.c (add_macro_name): Return a walk status and
	continue indefinitely.
---
 gdb/macrocmd.c |    7 +++++--
 gdb/macrotab.c |   33 ++++++++++++++++++++++-----------
 gdb/macrotab.h |   48 +++++++++++++++++++++++++++++++++++-------------
 gdb/symtab.c   |    3 ++-
 4 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
index d1ac7fa..e929fe0 100644
--- a/gdb/macrocmd.c
+++ b/gdb/macrocmd.c
@@ -220,13 +220,15 @@ info_macro_command (char *name, int from_tty)
    Otherwise USER_DATA is considered to be a string, printing
    only macros who's NAME matches USER_DATA.  Other arguments are
    routed to print_macro_definition.  */
-static void
+static enum macro_walk_status
 print_macro_callback (const char *name, const struct macro_definition *macro,
 		   struct macro_source_file *source, int line,
 		   void *user_data)
 {
   if (! user_data || strcmp (user_data, name) == 0)
     print_macro_definition (name, macro, source, line);
+
+  return macro_walk_continue;
 }
 
 /* Implementation of the "info definitions" command. */
@@ -435,7 +437,7 @@ macro_undef_command (char *exp, int from_tty)
 }
 
 
-static void
+static enum macro_walk_status
 print_one_macro (const char *name, const struct macro_definition *macro,
 		 struct macro_source_file *source, int line,
 		 void *ignore)
@@ -452,6 +454,7 @@ print_one_macro (const char *name, const struct macro_definition *macro,
       fprintf_filtered (gdb_stdout, ")");
     }
   fprintf_filtered (gdb_stdout, " %s\n", macro->replacement);
+  return macro_walk_continue;
 }
 
 
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index efcf835..e4007a5 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -912,14 +912,15 @@ foreach_macro (splay_tree_node node, void *arg)
   struct macro_for_each_data *datum = (struct macro_for_each_data *) arg;
   struct macro_key *key = (struct macro_key *) node->key;
   struct macro_definition *def = (struct macro_definition *) node->value;
+  enum macro_walk_status status;
 
-  (*datum->fn) (key->name, def, key->start_file, key->start_line,
-		datum->user_data);
-  return 0;
+  status = (*datum->fn) (key->name, def, key->start_file, key->start_line,
+			 datum->user_data);
+  return status == macro_walk_abort;
 }
 
 /* Call FN for every macro in TABLE.  */
-void
+enum macro_walk_result
 macro_for_each (struct macro_table *table, macro_callback_fn fn,
 		void *user_data)
 {
@@ -929,15 +930,21 @@ macro_for_each (struct macro_table *table, macro_callback_fn fn,
   datum.user_data = user_data;
   datum.file = NULL;
   datum.line = 0;
-  splay_tree_foreach (table->definitions, foreach_macro, &datum);
+
+  if (splay_tree_foreach (table->definitions, foreach_macro, &datum) != 0)
+    return macro_walk_aborted;
+  else
+    return macro_walk_completed;
 }
 
+/* Helper function for macro_for_each_in_scope.  */
 static int
 foreach_macro_in_scope (splay_tree_node node, void *info)
 {
   struct macro_for_each_data *datum = (struct macro_for_each_data *) info;
   struct macro_key *key = (struct macro_key *) node->key;
   struct macro_definition *def = (struct macro_definition *) node->value;
+  enum macro_walk_status status = macro_walk_continue;
 
   /* See if this macro is defined before the passed-in line, and
      extends past that line.  */
@@ -946,13 +953,14 @@ foreach_macro_in_scope (splay_tree_node node, void *info)
       && (!key->end_file
 	  || compare_locations (key->end_file, key->end_line,
 				datum->file, datum->line) >= 0))
-    (*datum->fn) (key->name, def, key->start_file, key->start_line,
-		  datum->user_data);
-  return 0;
+    status = (*datum->fn) (key->name, def, key->start_file, key->start_line,
+			   datum->user_data);
+
+  return status == macro_walk_abort;
 }
 
 /* Call FN for every macro is visible in SCOPE.  */
-void
+enum macro_walk_result
 macro_for_each_in_scope (struct macro_source_file *file, int line,
 			 macro_callback_fn fn, void *user_data)
 {
@@ -962,8 +970,11 @@ macro_for_each_in_scope (struct macro_source_file *file, int line,
   datum.user_data = user_data;
   datum.file = file;
   datum.line = line;
-  splay_tree_foreach (file->table->definitions,
-		      foreach_macro_in_scope, &datum);
+  if (splay_tree_foreach (file->table->definitions,
+			  foreach_macro_in_scope, &datum) == 0)
+    return macro_walk_completed;
+  else
+    return macro_walk_aborted;
 }
 
 
diff --git a/gdb/macrotab.h b/gdb/macrotab.h
index a10351a..4b1c686 100644
--- a/gdb/macrotab.h
+++ b/gdb/macrotab.h
@@ -305,28 +305,50 @@ struct macro_source_file *(macro_definition_location
                             const char *name,
                             int *definition_line));
 
+/* Allows you to abort or continue a walk of the macro table. */
+enum macro_walk_status {
+   macro_walk_abort,
+   macro_walk_continue
+};
+
 /* Callback function when walking a macro table.  NAME is the name of
    the macro, and DEFINITION is the definition.  SOURCE is the file at the
    start of the include path, and LINE is the line number of the SOURCE file
    where the macro was defined.  USER_DATA is an arbitrary pointer which is
-   passed by the caller to macro_for_each or macro_for_each_in_scope.  */
-typedef void (*macro_callback_fn) (const char *name,
-				   const struct macro_definition *definition,
-				   struct macro_source_file *source,
-				   int line,
-				   void *user_data);
+   passed by the caller to macro_for_each or macro_for_each_in_scope.
+   The callback must return a macro_walk_status.  A return value of
+   macro_walk_abort will stop walking the macro table, a return value of
+   macro_walk_continue will proceed to the next iteration.  */
+typedef enum macro_walk_status (*macro_callback_fn)
+				 (const char *name,
+				  const struct macro_definition *definition,
+				  struct macro_source_file *source,
+				  int line,
+				  void *user_data);
+
+/* Describes the results of a walk of the macro table */
+enum macro_walk_result {
+  macro_walk_aborted,
+  macro_walk_completed
+};
 
 /* Call the function FN for each macro in the macro table TABLE.
-   USER_DATA is passed, untranslated, to FN.  */
-void macro_for_each (struct macro_table *table, macro_callback_fn fn,
-		     void *user_data);
+   USER_DATA is passed, untranslated, to FN.  Returns macro_walk_aborted,
+   or macro_walk_complete depending on the value returned by the callback
+   function passed in.  */
+enum macro_walk_result macro_for_each (struct macro_table *table,
+				       macro_callback_fn fn,
+				       void *user_data);
 
 /* Call the function FN for each macro that is visible in a given
    scope.  The scope is represented by FILE and LINE.  USER_DATA is
-   passed, untranslated, to FN.  */
-void macro_for_each_in_scope (struct macro_source_file *file, int line,
-			      macro_callback_fn fn,
-			      void *user_data);
+   passed, untranslated, to FN.  Returns macro_walk_aborted, or
+   macro_walk_complete depending on value returned by the callback
+   function passed in.  */
+enum macro_walk_result macro_for_each_in_scope (struct macro_source_file *file,
+						int line,
+						macro_callback_fn fn,
+						void *user_data);
 
 
 #endif /* MACROTAB_H */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 9447bd9..6f22bce 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3769,7 +3769,7 @@ struct add_name_data
 
 /* A callback used with macro_for_each and macro_for_each_in_scope.
    This adds a macro's name to the current completion list.  */
-static void
+static enum macro_walk_status
 add_macro_name (const char *name, const struct macro_definition *ignore,
 		struct macro_source_file *ignore2, int ignore3,
 		void *user_data)
@@ -3779,6 +3779,7 @@ add_macro_name (const char *name, const struct macro_definition *ignore,
   completion_list_add_name ((char *) name,
 			    datum->sym_text, datum->sym_text_len,
 			    datum->text, datum->word);
+  return macro_walk_continue;
 }
 
 /* A callback for expand_partial_symbol_names.  */
-- 
1.7.4.4

[PATCH 2/7] [python] API for macros: memory management quirks.

[matt rice]

2011-08-23  Matt Rice  <ratmice@gmail.com

	* dwarf2read.c (macro_start_file): Update args in call to
	new_macro_table.
	* macroscope.c (_initialize_macroscope): Ditto.
	* macrotab.c (struct macro_table): Remove obstack and bcache,
	add objfile.
	(macro_alloc): Replace obstack and bcache usage with those from
	the objfile.
	(macro_free, macro_bcache_free, macro_allow_redefinitions): Ditto
	(macro_bcache, new_macro_table): Ditto, and add assertions.
	(macro_table_objfile): New function.
	* macrotab.h: Replace forward declarations of bcache and obstack
	with objfile.
	(enum macro_table_type, macro_table_objfile): Add.
---
 gdb/dwarf2read.c |    3 +--
 gdb/macroscope.c |    2 +-
 gdb/macrotab.c   |   53 ++++++++++++++++++++++++++++++++---------------------
 gdb/macrotab.h   |   44 ++++++++++++++++++++++++++------------------
 4 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index f66c1d9..c56f431 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14351,8 +14351,7 @@ macro_start_file (int file, int line,
   /* We don't create a macro table for this compilation unit
      at all until we actually get a filename.  */
   if (! pending_macros)
-    pending_macros = new_macro_table (&objfile->objfile_obstack,
-                                      objfile->macro_cache);
+    pending_macros = new_macro_table (objfile, macro_table_type_from_objfile);
 
   if (! current_file)
     /* If we have no current file, then this must be the start_file
diff --git a/gdb/macroscope.c b/gdb/macroscope.c
index b529e68..2447670 100644
--- a/gdb/macroscope.c
+++ b/gdb/macroscope.c
@@ -160,7 +160,7 @@ extern initialize_file_ftype _initialize_macroscope;
 void
 _initialize_macroscope (void)
 {
-  macro_user_macros = new_macro_table (0, 0);
+  macro_user_macros = new_macro_table (NULL, macro_table_type_user_defined);
   macro_set_main (macro_user_macros, "<user-defined>");
   macro_allow_redefinitions (macro_user_macros);
 }
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index efcf835..b2825d2 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -35,13 +35,10 @@
 
 struct macro_table
 {
-  /* The obstack this table's data should be allocated in, or zero if
-     we should use xmalloc.  */
-  struct obstack *obstack;
-
-  /* The bcache we should use to hold macro names, argument names, and
-     definitions, or zero if we should use xmalloc.  */
-  struct bcache *bcache;
+  /* The objfile who's obstack this table's data should be allocated in,
+     and bcache we should use to hold macro names, argument
+     names, and definitions, or zero if we should use xmalloc. */
+  struct objfile *objfile;
 
   /* The main source file for this compilation unit --- the one whose
      name was given to the compiler.  This is the root of the
@@ -83,8 +80,8 @@ struct macro_table
 static void *
 macro_alloc (int size, struct macro_table *t)
 {
-  if (t->obstack)
-    return obstack_alloc (t->obstack, size);
+  if (t->objfile)
+    return obstack_alloc (&t->objfile->objfile_obstack, size);
   else
     return xmalloc (size);
 }
@@ -93,7 +90,7 @@ macro_alloc (int size, struct macro_table *t)
 static void
 macro_free (void *object, struct macro_table *t)
 {
-  if (t->obstack)
+  if (t->objfile)
     /* There are cases where we need to remove entries from a macro
        table, even when reading debugging information.  This should be
        rare, and there's no easy way to free arbitrary data from an
@@ -110,8 +107,10 @@ macro_free (void *object, struct macro_table *t)
 static const void *
 macro_bcache (struct macro_table *t, const void *addr, int len)
 {
-  if (t->bcache)
-    return bcache (addr, len, t->bcache);
+  gdb_assert (t->objfile == NULL || (t->objfile && t->objfile->macro_cache));
+
+  if (t->objfile && t->objfile->macro_cache)
+    return bcache (addr, len, t->objfile->macro_cache);
   else
     {
       void *copy = xmalloc (len);
@@ -137,7 +136,7 @@ macro_bcache_str (struct macro_table *t, const char *s)
 static void
 macro_bcache_free (struct macro_table *t, void *obj)
 {
-  if (t->bcache)
+  if (t->objfile)
     /* There are cases where we need to remove entries from a macro
        table, even when reading debugging information.  This should be
        rare, and there's no easy way to free data from a bcache, so we
@@ -147,6 +146,12 @@ macro_bcache_free (struct macro_table *t, void *obj)
     xfree (obj);
 }
 
+/* Return the objfile of the macro table, NULL if it is user defined.  */
+struct objfile *
+macro_table_objfile (struct macro_table *table)
+{
+  return table->objfile;
+}
 
 \f
 /* Macro tree keys, w/their comparison, allocation, and freeing functions.  */
@@ -438,7 +443,7 @@ macro_main (struct macro_table *t)
 void
 macro_allow_redefinitions (struct macro_table *t)
 {
-  gdb_assert (! t->obstack);
+  gdb_assert (! t->objfile);
   t->redef_ok = 1;
 }
 
@@ -972,20 +977,26 @@ macro_for_each_in_scope (struct macro_source_file *file, int line,
 
 
 struct macro_table *
-new_macro_table (struct obstack *obstack,
-                 struct bcache *b)
+new_macro_table (struct objfile *objfile, enum macro_table_type table_type)
 {
   struct macro_table *t;
 
+  if (table_type == macro_table_type_user_defined)
+    gdb_assert (objfile == NULL);
+  else if (table_type == macro_table_type_from_objfile)
+    {
+      gdb_assert (objfile != NULL);
+      gdb_assert (objfile->macro_cache != NULL);
+    }
+
   /* First, get storage for the `struct macro_table' itself.  */
-  if (obstack)
-    t = obstack_alloc (obstack, sizeof (*t));
-  else
+  if (table_type == macro_table_type_from_objfile)
+    t = obstack_alloc (&objfile->objfile_obstack, sizeof (*t));
+  else if (table_type == macro_table_type_user_defined)
     t = xmalloc (sizeof (*t));
 
   memset (t, 0, sizeof (*t));
-  t->obstack = obstack;
-  t->bcache = b;
+  t->objfile = objfile;
   t->main_source = NULL;
   t->redef_ok = 0;
   t->definitions = (splay_tree_new_with_allocator
diff --git a/gdb/macrotab.h b/gdb/macrotab.h
index a10351a..a773c49 100644
--- a/gdb/macrotab.h
+++ b/gdb/macrotab.h
@@ -21,8 +21,7 @@
 #ifndef MACROTAB_H
 #define MACROTAB_H
 
-struct obstack;
-struct bcache;
+struct objfile;
 
 /* How do we represent a source location?  I mean, how should we
    represent them within GDB; the user wants to use all sorts of
@@ -149,23 +148,28 @@ struct macro_source_file
 };
 
 
-/* Create a new, empty macro table.  Allocate it in OBSTACK, or use
-   xmalloc if OBSTACK is zero.  Use BCACHE to store all macro names,
-   arguments, definitions, and anything else that might be the same
-   amongst compilation units in an executable file; if BCACHE is zero,
-   don't cache these things.
+enum macro_table_type {
+  macro_table_type_user_defined,
+  macro_table_type_from_objfile
+};
 
-   Note that, if either OBSTACK or BCACHE are non-zero, then removing
-   information from the table may leak memory.  Neither obstacks nor
-   bcaches really allow you to remove information, so although we can
-   update the data structure to record the change, we can't free the
-   old data.  At the moment, since we only provide obstacks and
-   bcaches for macro tables for symtabs, this isn't a problem; only
-   odd debugging information makes a definition and then deletes it at
-   the same source location (although 'gcc -DFOO -UFOO -DFOO=2' does
-   do that in GCC 4.1.2.).  */
-struct macro_table *new_macro_table (struct obstack *obstack,
-                                     struct bcache *bcache);
+/* Create a new, empty macro table.  Allocate it in OBJFILE's obstack,
+   or use xmalloc if OBJFILE is zero.  Use OBJFILE's bcache to store
+   all macro names, arguments, definitions, and anything else that
+   might be the same amongst compilation units in an executable file;
+   if OBJFILE is zero, don't cache these things.
+
+   Note that, if OBJFILE is non-zero, then removing information from the
+   table may leak memory.  Neither obstacks nor bcaches really allow
+   you to remove information, so although we can update the data
+   structure to record the change, we can't free the old data.
+   At the moment, since we only provide obstacks and bcaches for macro
+   tables for symtabs, this isn't a problem; only odd debugging
+   information makes a definition and then deletes it at the same
+   source location (although 'gcc -DFOO -UFOO -DFOO=2' does do that
+   in GCC 4.1.2.).  */
+struct macro_table *new_macro_table (struct objfile *objfile,
+				     enum macro_table_type table_type);
 
 
 /* Free TABLE, and any macro definitions, source file structures,
@@ -173,6 +177,10 @@ struct macro_table *new_macro_table (struct obstack *obstack,
    allocated on an obstack, or if it uses a bcache.  */
 void free_macro_table (struct macro_table *table);
 
+/* Returns the OBJFILE for the given table, if the table
+   is for user-defined macros this function will return NULL.  */
+struct objfile * macro_table_objfile (struct macro_table *table);
+
 
 /* Set FILENAME as the main source file of TABLE.  Return a source
    file structure describing that file; if we record the #definition
-- 
1.7.4.4

[PATCH 5/7] [python] API for macros: gdb.Objfile symtabs method.

[matt rice]

2011-08-23  Matt Rice  <ratmice@gmail.com>

	* python/py-symtab.h: New file.  Make symtab_to_symtab_object public.
	* python/py-objfile.c (objfpy_symtabs): New method.
	(objfile_object_methods): Ditto.
---
 gdb/python/py-objfile.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 gdb/python/py-symtab.h  |   26 ++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 0 deletions(-)
 create mode 100644 gdb/python/py-symtab.h

diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index f9821f5..1c121ae 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -22,6 +22,7 @@
 #include "charset.h"
 #include "objfiles.h"
 #include "language.h"
+#include "py-symtab.h"
 
 typedef struct
 {
@@ -118,6 +119,44 @@ objfpy_set_printers (PyObject *o, PyObject *value, void *ignore)
   return 0;
 }
 
+static PyObject *
+objfpy_symtabs (PyObject *self, PyObject *ignore)
+{
+  objfile_object *obj = (objfile_object *) self;
+  struct symtab *symtabs;
+  PyObject *list;
+  PyObject *py_symtab;
+
+  if (! obj->objfile)
+    return Py_None;
+
+  list = PyList_New (0);
+  if (!list)
+    return NULL;
+
+  symtabs = obj->objfile->symtabs;
+  while(symtabs)
+    {
+      py_symtab = symtab_to_symtab_object (symtabs);
+      if (! py_symtab)
+        goto fail;
+
+      if (PyList_Append (list, py_symtab) != 0)
+        goto fail;
+
+      Py_DECREF (py_symtab);
+
+      symtabs = symtabs->next;
+    }
+
+  return list;
+
+  fail:
+    Py_XDECREF (py_symtab);
+    Py_XDECREF (list);
+    return NULL;
+}
+
 /* Implementation of gdb.Objfile.is_valid (self) -> Boolean.
    Returns True if this object file still exists in GDB.  */
 
@@ -200,6 +239,9 @@ static PyMethodDef objfile_object_methods[] =
   { "is_valid", objfpy_is_valid, METH_NOARGS,
     "is_valid () -> Boolean.\n\
 Return true if this object file is valid, false if not." },
+  { "symtabs", objfpy_symtabs, METH_NOARGS,
+    "symtabs () -> List.\n\
+A List containing the object file's valid symtabs." },
 
   { NULL }
 };
diff --git a/gdb/python/py-symtab.h b/gdb/python/py-symtab.h
new file mode 100644
index 0000000..10c89cb
--- /dev/null
+++ b/gdb/python/py-symtab.h
@@ -0,0 +1,26 @@
+/* Python interface to Symtabs and Symtab_and_line's.
+
+   Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_PY_SYMTAB_H
+#define GDB_PY_SYMTAB_H
+
+PyObject *
+symtab_to_symtab_object (struct symtab *symtab);
+
+#endif /* GDB_PY_SYMTAB_H */
-- 
1.7.4.4

[PATCH 0/7] [python] API for macros

[matt rice]

take 2 on the python macro API...
In addition to the testsuite, I tested with the last release of
gcc following: http://gcc.gnu.org/wiki/DebuggingGCC

using variations of the do_stuff function in following script...
I didn't save exact timing #'s but it was something like
 2 mins to count all 600+ million macros
 5 mins to filter all by name with macro.name().startswith()
 15 mins to call the str() method (which calls all other methods).

I'd tried doing a deep copy in the gdb.Macro class,
to avoid all the objfile/obstack/bcache horse pucky evident in this series,
but i killed it before it completed when working with gcc...

it's not really timing equivalent to that last 15 minutes case since
we use lots more memory having a deep copy of all the macros in a symtab in a
list.  Where the 15 minute version does a deep copy, with only one macro's
contents in memory at a time.

thus, I think it is useful even for large projects (if used with care.)
this will fall over if someone has has way too many
macros in a single symtab.  should that happen we can add a macro_map()
that behaves sort of like the python map function.

I think a list is the most straight forward approach for general usage,
and has been shown to work even with projects that use macros extensively.

define read_all_psymtabs
set logging file /tmp/gdb.txt
set logging on
set logging redirect on
maint info psymtabs
# This relies on a side-effect maybe a better way.
shell for i in `grep "text addresses" /tmp/gdb.txt | awk '{print $3}' | sort | uniq`; do echo info line *$i; done >/tmp/foo.gdb
set logging off
python tmp = gdb.execute("source /tmp/foo.gdb", to_string = True);
python tmp = None;
end

python
def do_stuff():
  a_set = set();
  for i in gdb.objfiles():
    if i.is_valid():
      for s in i.symtabs:
        if s.is_valid():
          for a in s.macros():
            name = a.name()
            if name.startswith("TREE"):
              a_set.add(a.is_valid())
  return a_set
end

# hmm, these don't seem to execute if sourced, strange.
read_all_psymtabs
python print do_stuff()

With regards to the hash/compare methods, the implementation of those
is up for debate, I see at least 3 valid ways to compare them and have only one
comparison function.  right now I have it compare deeply e.g. including the whole include_trail
other options that seem valid, compare the name,args,defininition, 
and compare the name,args,definition and partial include_trail
(just definition location) since they pretty much are equal before expansion,
and expansion is an 'expression' thing.

I'd suppose though it's entirely possible for the user to just wrap the object
in another object and DIY comparison method... thats probably the best idea.
I'd defer to an actual user here and let that override `whatever i am using it for
in the testsuite because its convenient.'.  Should we form some kind of opinion i'll
do this in a follow up.

There are some implementation quirks described in the documentation,
some of these are so in the future we can add a gdb.UserMacro which does
a deep-copy on initialization, I wasn't going to add that unless someone
requests it.  Python doesn't seem to have any form of java's `interface',
or abstract base classing at least within our version range,
we could also hack up something ugly in the future to avoid this documentation,
(read union'ify the class implementation,
 or make gdb.Macro have a pointer to a gdb.ObjfileMacro
 and do the lambda macro: macro.method() inside gdb.Macro)
I'd personally just leave it there to give us future choice on the matter.
we can always remove that from the docs if we implement it in a way

matt rice (7):
  [python] API for macros: abort or continuing macro iterators.
  [python] API for macros: memory management quirks.
  [python] API for macros: Add gdb.Macro class.
  [python] API for macros: Add methods to get a gdb.Macro.
  [python] API for macros: gdb.Objfile symtabs method.
  [python] API for macros: Add docs.
  [python] API for macros: Add tests.

 gdb/Makefile.in                       |    6 +
 gdb/NEWS                              |    7 +
 gdb/doc/gdb.texinfo                   |   77 ++++
 gdb/dwarf2read.c                      |    3 +-
 gdb/macrocmd.c                        |    7 +-
 gdb/macroscope.c                      |    2 +-
 gdb/macrotab.c                        |   86 +++--
 gdb/macrotab.h                        |   92 +++--
 gdb/python/py-macro.c                 |  782 +++++++++++++++++++++++++++++++++
 gdb/python/py-macro.h                 |   62 +++
 gdb/python/py-objfile.c               |   42 ++
 gdb/python/py-symtab.c                |  112 +++++
 gdb/python/py-symtab.h                |   26 ++
 gdb/python/python-internal.h          |    1 +
 gdb/python/python.c                   |    1 +
 gdb/symtab.c                          |    3 +-
 gdb/testsuite/gdb.python/py-macro.c   |   87 ++++
 gdb/testsuite/gdb.python/py-macro.exp |  338 ++++++++++++++
 18 files changed, 1665 insertions(+), 69 deletions(-)
 create mode 100644 gdb/python/py-macro.c
 create mode 100644 gdb/python/py-macro.h
 create mode 100644 gdb/python/py-symtab.h
 create mode 100644 gdb/testsuite/gdb.python/py-macro.c
 create mode 100644 gdb/testsuite/gdb.python/py-macro.exp

-- 
1.7.4.4

[PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[matt rice]

2011-08-23  Matt Rice  <ratmice@gmail.com>

	PR python/10807
	* py-symtab.c (stpy_dealloc): Call super's tp_free.
	(salpy_macros, stpy_macros, stpy_macro_named): New methods.
	(sal_object_methods, stpy_object_methods): Add new methods to method list.
---
 gdb/python/py-symtab.c |  112 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 112 insertions(+), 0 deletions(-)

diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 107cdec..1bceeb6 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -22,6 +22,8 @@
 #include "symtab.h"
 #include "source.h"
 #include "python-internal.h"
+#include "py-macro.h"
+#include "macroscope.h"
 #include "objfiles.h"
 
 typedef struct stpy_symtab_object {
@@ -138,6 +140,35 @@ stpy_fullname (PyObject *self, PyObject *args)
   Py_RETURN_NONE;
 }
 
+static PyObject *
+stpy_macros (PyObject *self, PyObject *args)
+{
+  struct symtab *st = symtab_object_to_symtab (self);
+  PyObject *result;
+  enum macro_walk_result walk_result;
+  struct macropy_user_data mud;
+
+  STPY_REQUIRE_VALID (self, st);
+
+  result = PyList_New (0);
+  if (result == NULL)
+    return NULL;
+
+  mud.list = result;
+  mud.objfile = macro_table_objfile (st->macro_table);
+  walk_result = macro_for_each (st->macro_table,
+				pymacro_add_macro_to_list,
+				&mud);
+
+  if (walk_result == macro_walk_aborted)
+    {
+      Py_DECREF (result);
+      return NULL;
+    }
+
+  return result;
+}
+
 /* Implementation of gdb.Symtab.is_valid (self) -> Boolean.
    Returns True if this Symbol table still exists in GDB.  */
 
@@ -191,6 +222,7 @@ stpy_dealloc (PyObject *obj)
   if (symtab->next)
     symtab->next->prev = symtab->prev;
   symtab->symtab = NULL;
+  obj->ob_type->tp_free (obj);
 }
 
 
@@ -242,6 +274,75 @@ salpy_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
+static PyObject *
+salpy_macros (PyObject *self, PyObject *args)
+{
+  struct symtab_and_line *sal;
+  PyObject *result;
+  enum macro_walk_result walk_result;
+  struct macropy_user_data mud;
+  struct macro_scope *ms;
+
+  SALPY_REQUIRE_VALID (self, sal);
+
+  ms = sal_macro_scope (*sal);
+
+  result = PyList_New (0);
+  if (result == NULL)
+    return NULL;
+
+  mud.list = result;
+  mud.objfile = macro_table_objfile (ms->file->table);
+  walk_result = macro_for_each_in_scope (ms->file, ms->line,
+					 pymacro_add_macro_to_list, &mud);
+
+  if (walk_result == macro_walk_aborted)
+    {
+      Py_DECREF (result);
+      return NULL;
+    }
+
+  return result;
+}
+
+static PyObject *
+salpy_macro_named (PyObject *self, PyObject *args)
+{
+  struct symtab_and_line *sal;
+  struct macro_scope *ms;
+  const struct macro_definition *mdef;
+  char *macro_name;
+  struct cleanup *cleanup_chain;
+  struct macro_source_file *file;
+  struct objfile *objfile;
+  PyObject *result;
+  int line;
+
+  if (!PyArg_ParseTuple (args, "s", &macro_name))
+    return NULL;
+
+  SALPY_REQUIRE_VALID (self, sal);
+
+  ms = sal_macro_scope (*sal);
+  cleanup_chain = make_cleanup (free_current_contents, &ms);
+  if (ms == NULL)
+    goto none_exit;
+
+  mdef = macro_lookup_definition (ms->file, ms->line, macro_name);
+  if (mdef == NULL)
+    goto none_exit;
+
+  file = macro_definition_location(ms->file, ms->line, macro_name, &line);
+  objfile = macro_table_objfile(ms->file->table);
+  result = macropy_object_create (objfile, macro_name, mdef, file, line);
+  do_cleanups (cleanup_chain);
+  return result;
+
+  none_exit:
+    do_cleanups (cleanup_chain);
+    return Py_None;
+}
+
 static void
 salpy_dealloc (PyObject *self)
 {
@@ -477,6 +578,9 @@ Return true if this symbol table is valid, false if not." },
   { "fullname", stpy_fullname, METH_NOARGS,
     "fullname () -> String.\n\
 Return the symtab's full source filename." },
+  { "macros", stpy_macros, METH_NOARGS,
+    "macros () -> List.\n\
+Return a list of macros in the symtab." },
   {NULL}  /* Sentinel */
 };
 
@@ -526,6 +630,14 @@ static PyMethodDef sal_object_methods[] = {
   { "is_valid", salpy_is_valid, METH_NOARGS,
     "is_valid () -> Boolean.\n\
 Return true if this symbol table and line is valid, false if not." },
+  { "macro_named", salpy_macro_named, METH_VARARGS,
+    "macro_named (name) -> Macro.\n\
+Return the macro object for the given name, or None if the \
+macro cannot be found." },
+  { "macros", salpy_macros, METH_NOARGS,
+    "macros () -> List.\n\
+Return all the macros which are available at the symbol table and line \
+object's location." },
   {NULL}  /* Sentinel */
 };
 
-- 
1.7.4.4

[PATCH 3/7] [python] API for macros: Add gdb.Macro class.

[matt rice]

2011-08-23  Matt Rice  <ratmice@gmail.com>

	PR python/10807
	* Makefile.in:  Add python/py-macro.c.
	* python/py-macro.c: New file.
	* python/py-macro.h: Ditto.
	* python/python-internal.h: Declare gdbpy_initialize_macros.
	* python/python.c (_initialize_python): Call gdbpy_initialize_macros.
---
 gdb/Makefile.in              |    6 +
 gdb/python/py-macro.c        |  782 ++++++++++++++++++++++++++++++++++++++++++
 gdb/python/py-macro.h        |   62 ++++
 gdb/python/python-internal.h |    1 +
 gdb/python/python.c          |    1 +
 5 files changed, 852 insertions(+), 0 deletions(-)
 create mode 100644 gdb/python/py-macro.c
 create mode 100644 gdb/python/py-macro.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index bd00644..71389a6 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -289,6 +289,7 @@ SUBDIR_PYTHON_OBS = \
 	py-inferior.o \
 	py-infthread.o \
 	py-lazy-string.o \
+	py-macro.o \
 	py-objfile.o \
 	py-param.o \
 	py-prettyprint.o \
@@ -319,6 +320,7 @@ SUBDIR_PYTHON_SRCS = \
 	python/py-inferior.c \
 	python/py-infthread.c \
 	python/py-lazy-string.c \
+	python/py-macro.c \
 	python/py-objfile.c \
 	python/py-param.c \
 	python/py-prettyprint.c \
@@ -2110,6 +2112,10 @@ py-lazy-string.o: $(srcdir)/python/py-lazy-string.c
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-lazy-string.c
 	$(POSTCOMPILE)
 
+py-macro.o: $(srcdir)/python/py-macro.c
+	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-macro.c
+	$(POSTCOMPILE)
+
 py-objfile.o: $(srcdir)/python/py-objfile.c
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-objfile.c
 	$(POSTCOMPILE)
diff --git a/gdb/python/py-macro.c b/gdb/python/py-macro.c
new file mode 100644
index 0000000..184be02
--- /dev/null
+++ b/gdb/python/py-macro.c
@@ -0,0 +1,782 @@
+/* Python interface to macros.
+
+   Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "py-macro.h"
+#include "macrotab.h"
+#include "bcache.h"
+#include "objfiles.h"
+
+/* The macro_object_validator type has the ability to detect when an object
+   has been invalidated and nothing more.  The more featured validation
+   methods of pyst, pysal, provide no benefit for macros because once
+   we have been invalidated, we have no way to know what macro the object
+   once represented.  */
+typedef struct
+{
+  PyObject_HEAD;
+  int is_valid;
+  struct objfile *objfile;
+} macro_validator_object;
+
+typedef struct
+{
+  PyObject_HEAD;
+
+  /* A macro definition object representing this macro.  */
+  const struct macro_definition *def;
+  /* The name of the macro */
+  const char *name;
+  /* The file where the macro resides and its include trail.  */
+  struct macro_source_file *src;
+  /* the line location in the 'SRC' file.  */
+  int line;
+  /* This is used to tell us if the macro has been invalidated
+     The objfile who's obstack and bcache the mdef, and src, and name
+     fields reside in is stored here.  Those fields may be dangling
+     pointers if the objfile is NULL.
+     An invalid macro cannot become valid once again.  */
+  macro_validator_object *objfile_validator;
+} macro_object;
+
+static const struct objfile_data *macropy_objfile_data_key;
+
+static PyTypeObject macro_object_type;
+static PyTypeObject macro_validator_object_type;
+
+\f
+
+/* Internal methods for converting macros to python objects.  */
+
+static PyObject *
+macropy__definition_to_py (const struct macro_definition *macro)
+{
+  if (macro->replacement)
+    return PyString_FromString (macro->replacement);
+  else
+    Py_RETURN_NONE;
+}
+
+static PyObject *
+macropy__is_function_like_to_py (const struct macro_definition *macro)
+{
+  if (macro->kind == macro_function_like)
+    Py_RETURN_TRUE;
+  else
+    Py_RETURN_FALSE;
+}
+
+static PyObject *
+macropy__argv_to_py (const struct macro_definition *macro)
+{
+  PyObject *ret;
+
+  if (macro->kind == macro_function_like)
+    {
+      Py_ssize_t i;
+      ret = PyList_New (macro->argc);
+
+      if (ret == NULL)
+	goto err_ret;
+
+      for (i = 0; i < macro->argc; i++)
+        {
+	  PyObject *item = PyString_FromString (macro->argv[i]);
+
+	  if (!item)
+	    goto err_ret;
+
+	  if (PyList_SetItem (ret, i, item) != 0)
+            goto err_ret;
+        }
+
+      return ret;
+    }
+
+  Py_RETURN_NONE;
+
+  err_ret:
+    Py_XDECREF (ret);
+    return NULL;
+}
+
+
+static PyObject *
+macropy__include_trail_to_py (const struct macro_definition *macro,
+			      struct macro_source_file *file,
+			      int line)
+{
+  PyObject *tuple = NULL;
+  PyObject *result;
+  PyObject *tmp;
+
+  result = PyList_New (0);
+  if (!result)
+    goto err_ret;
+
+  tuple = PyTuple_New (2);
+  if (!tuple)
+    goto err_ret;
+
+  tmp = PyString_FromString (file->filename);
+  if (!tmp)
+    goto err_ret;
+  if (PyTuple_SetItem (tuple, 0, tmp) != 0)
+    goto err_ret;
+
+  tmp = PyInt_FromLong (line);
+  if (!tmp)
+    goto err_ret;
+  if (PyTuple_SetItem (tuple, 1, tmp) != 0)
+    goto err_ret;
+
+  if (PyList_Append (result, tuple) != 0)
+    goto err_ret;
+  Py_DECREF (tuple);
+
+  while (file->included_by)
+    {
+      tuple = PyTuple_New (2);
+
+      if (!tuple)
+        goto err_ret;
+
+      tmp = PyString_FromString (file->included_by->filename);
+      if (!tmp)
+        goto err_ret;
+      if (PyTuple_SetItem (tuple, 0, tmp) != 0)
+        goto err_ret;
+
+      tmp = PyInt_FromLong (file->included_at_line);
+      if (!tmp)
+        goto err_ret;
+      if (PyTuple_SetItem (tuple, 1, tmp) != 0)
+        goto err_ret;
+
+      if (PyList_Append (result, tuple) != 0)
+	goto err_ret;
+      Py_DECREF (tuple);
+
+      file = file->included_by;
+    }
+
+  return result;
+
+  err_ret:
+    Py_XDECREF (tuple);
+    Py_XDECREF (result);
+    return NULL;
+}
+
+static PyObject *
+macropy__name_to_py (const char *name)
+{
+  return PyString_FromString (name);
+}
+
+\f
+
+/* Publicly declared functions. */
+
+/* A macro_callback_fn for use with macro_foreach that adds
+   the macro to a python list.  */
+enum macro_walk_status
+pymacro_add_macro_to_list (const char *name,
+		   const struct macro_definition *definition,
+		   struct macro_source_file *source,
+		   int line,
+		   void *user_data)
+{
+  PyObject *tmp = NULL;
+  struct macropy_user_data *mud = user_data;
+  PyObject *list = (PyObject *) mud->list;
+
+  if (! PyObject_TypeCheck (list, &PyList_Type))
+    goto fail;
+
+  tmp = macropy_object_create (mud->objfile, name, definition, source, line);
+  if (!tmp)
+    goto fail;
+
+  if (PyList_Append (list, tmp) != 0)
+    goto fail;
+
+  Py_DECREF (tmp);
+  return macro_walk_continue;
+
+  fail:
+    Py_XDECREF (tmp);
+    return macro_walk_abort;
+}
+
+/* Create a new macro (gdb.Macro) object that encapsulates
+   the gdb macro representation.  */
+PyObject *
+macropy_object_create (struct objfile *objfile,
+		       const char *name,
+		       const struct macro_definition *def,
+		       struct macro_source_file *src,
+		       int line)
+{
+  macro_object *macro_obj;
+
+  macro_obj = PyObject_New (macro_object, &macro_object_type);
+
+  if (macro_obj)
+    {
+      macro_validator_object *validator;
+
+      macro_obj->objfile_validator = NULL;
+
+      /* check our arguments, Don't check line because 0 is a valid line.  */
+      if (!objfile)
+	{
+	  PyErr_SetString (PyExc_RuntimeError,
+			   _("Invalid objfile initializing gdb.Macro"));
+	  goto err_ret;
+	}
+
+      if (!name)
+	{
+	  PyErr_SetString (PyExc_RuntimeError,
+			   _("Invalid macro name initializing gdb.Macro"));
+	  goto err_ret;
+	}
+
+      if (!def)
+	{
+	  PyErr_SetString (PyExc_RuntimeError,
+			   _("Invalid macro definition initializing gdb.Macro"));
+	  goto err_ret;
+	}
+
+      if (!src)
+	{
+	  PyErr_SetString (PyExc_RuntimeError,
+			   _("Invalid macro source file initializing gdb.Macro"));
+	  goto err_ret;
+	}
+
+      validator = objfile_data (objfile, macropy_objfile_data_key);
+      if (!validator)
+        {
+          validator = PyObject_New (macro_validator_object,
+				    &macro_validator_object_type);
+	  if (!validator)
+	    goto err_ret;
+
+	  validator->is_valid = 1;
+	  validator->objfile = objfile;
+          set_objfile_data (objfile, macropy_objfile_data_key, validator);
+        }
+      else
+        Py_INCREF (validator);
+
+      macro_obj->name = bcache (name, strlen (name) + 1, objfile->macro_cache);
+      macro_obj->objfile_validator = validator;
+      macro_obj->def = def;
+      macro_obj->src = src;
+      macro_obj->line = line;
+
+      return (PyObject *) macro_obj;
+
+      err_ret:
+	Py_DECREF (macro_obj);
+	return NULL;
+    }
+  return NULL;
+}
+
+static void
+macropy_dealloc (PyObject *obj)
+{
+  macro_object *me = (macro_object *) obj;
+
+  Py_XDECREF (me->objfile_validator);
+
+  obj->ob_type->tp_free (obj);
+}
+
+static void
+macropy_validator_dealloc (PyObject *obj)
+{
+  macro_validator_object *me = (macro_validator_object *) obj;
+
+  if (me->objfile)
+    set_objfile_data (me->objfile, macropy_objfile_data_key, NULL);
+
+  obj->ob_type->tp_free (obj);
+}
+
+static void
+del_objfile_macro (struct objfile *objfile, void *datum)
+{
+  macro_validator_object *validator = datum;
+
+  if (validator)
+    {
+      validator->is_valid = 0;
+      validator->objfile = NULL;
+    }
+  else
+   PyErr_SetString (PyExc_RuntimeError,
+		    _("Internal inconsistency invalidating objfile macros."));
+}
+
+\f
+
+/* Helper function for macro validation */
+
+static inline macro_object *
+macropy__validate_self (PyObject *self)
+{
+  macro_object *me;
+
+  if (! PyObject_TypeCheck (self, &macro_object_type))
+    {
+      PyErr_SetString (PyExc_RuntimeError,
+			 _("Typecheck failure converting macro."));
+      return NULL;
+    }
+
+  me = (macro_object *) self;
+
+  if (me->objfile_validator->is_valid == 0)
+    {
+      PyErr_SetString (PyExc_RuntimeError, _("Macro is invalid."));
+      return NULL;
+    }
+
+  return me;
+}
+
+/* Macro object methods */
+
+static PyObject *
+macropy_name_method (PyObject *self, PyObject *args)
+{
+  macro_object *me = macropy__validate_self (self);
+
+  if (!me)
+    return NULL;
+
+  return macropy__name_to_py (me->name);
+}
+
+static PyObject *
+macropy_definition_method (PyObject *self, PyObject *args)
+{
+  macro_object *me = macropy__validate_self (self);
+
+  if (!me)
+    return NULL;
+
+  return macropy__definition_to_py (me->def);
+}
+
+static PyObject *
+macropy_is_function_like_method (PyObject *self, PyObject *args)
+{
+  macro_object *me = macropy__validate_self (self);
+
+  if (!me)
+    return NULL;
+
+  return macropy__is_function_like_to_py (me->def);
+}
+
+static PyObject *
+macropy_argv_method (PyObject *self, PyObject *args)
+{
+  macro_object *me = macropy__validate_self (self);
+
+  if (!me)
+    return NULL;
+
+  return macropy__argv_to_py (me->def);
+}
+
+static PyObject *
+macropy_include_trail_method (PyObject *self, PyObject *args)
+{
+  macro_object *me = macropy__validate_self (self);
+
+  if (!me)
+    return NULL;
+
+  return macropy__include_trail_to_py (me->def, me->src, me->line);
+}
+
+/* For future API compatibility. */
+static PyObject *
+macropy_is_valid_method (PyObject *self, PyObject *args)
+{
+  macro_object *me = (macro_object *) self;
+
+  if (!me)
+    return NULL;
+
+  if (me->objfile_validator->is_valid)
+    Py_RETURN_TRUE;
+
+  Py_RETURN_FALSE;
+}
+
+/* Helpers for the macropy_str method.  */
+static int
+macropy__concat_chars (PyObject **result, const char *stuff)
+{
+  PyObject *tmp = PyString_FromString (stuff);
+
+  if (!tmp)
+    return -1;
+
+  PyString_ConcatAndDel (result, tmp);
+  if (*result == NULL)
+    return -1;
+
+  return 0;
+}
+
+static int
+macropy__concat_py_obj (PyObject **result, PyObject *stuff)
+{
+  PyObject *tmp;
+
+  if (PyObject_TypeCheck (stuff, &PyString_Type))
+    {
+      PyString_Concat (result, stuff);
+      return *result ? 0 : -1;
+    }
+
+  tmp = PyObject_Str (stuff);
+  if (!tmp)
+    return -1;
+
+  PyString_ConcatAndDel (result, tmp);
+  return *result ? 0 : -1;
+}
+
+static PyObject *
+macropy_str (PyObject *self)
+{
+  PyObject *result = NULL;
+  PyObject *argv = NULL;
+  PyObject *definition = NULL;
+  PyObject *include_trail = NULL;
+  PyObject *is_function_like = NULL;
+  PyObject *name = NULL;
+  macro_object *me = macropy__validate_self (self);
+
+  if (!me)
+    goto err_ret;
+
+  result = PyString_FromString ("<gdb.Macro ");
+  if (!result)
+    goto err_ret;
+
+  argv = macropy__argv_to_py (me->def);
+  if (!argv)
+    goto err_ret;
+
+  definition = macropy__definition_to_py (me->def);
+  if (!definition)
+    goto err_ret;
+
+  include_trail = macropy__include_trail_to_py (me->def, me->src, me->line);
+  if (!include_trail)
+    goto err_ret;
+
+  is_function_like = macropy__is_function_like_to_py (me->def);
+  if (!is_function_like)
+    goto err_ret;
+
+  name = macropy__name_to_py (me->name);
+  if (!name)
+    goto err_ret;
+
+  if (macropy__concat_py_obj (&result, name) != 0)
+    goto err_ret;
+
+  if (is_function_like == Py_True)
+    {
+      Py_ssize_t sz = PyList_Size (argv);
+      Py_ssize_t i;
+
+      if (macropy__concat_chars (&result, "(") != 0)
+        goto err_ret;
+
+      for (i = 0; i < sz; i++)
+        {
+				      /* borrowed */
+	  if (macropy__concat_py_obj (&result, PyList_GetItem (argv, i)) != 0)
+	    goto err_ret;
+
+	  if (i < sz - 1)
+            if (macropy__concat_chars (&result, ", ") != 0)
+	      goto err_ret;
+        }
+
+      if (macropy__concat_chars (&result, ")") != 0)
+	goto err_ret;
+    }
+
+  if (definition != Py_None && PyString_Size (definition))
+    {
+      if (macropy__concat_chars (&result, "=") != 0)
+        goto err_ret;
+
+      if (macropy__concat_py_obj (&result, definition) != 0)
+	goto err_ret;
+    }
+
+  if (macropy__concat_chars (&result, " ") != 0)
+    goto err_ret;
+  if (macropy__concat_chars (&result, "include_trail=") != 0)
+    goto err_ret;
+  if (macropy__concat_py_obj (&result, include_trail) != 0)
+    goto err_ret;
+
+  if (macropy__concat_chars (&result, ">") != 0)
+    goto err_ret;
+
+  goto normal_ret;
+
+  err_ret:
+    Py_XDECREF (result);
+    result = NULL;
+  normal_ret:
+    Py_XDECREF (argv);
+    Py_XDECREF (definition);
+    Py_XDECREF (include_trail);
+    Py_XDECREF (is_function_like);
+    Py_XDECREF (name);
+    return result;
+}
+
+static int
+macropy_compare (PyObject *self, PyObject *o2)
+{
+  PyObject *my_str = macropy_str (self);
+  PyObject *other_str = NULL;
+  PyObject *an_error;
+  int err_code = -1;
+  int comparison_result = -1;
+
+  if (!my_str)
+    goto check_for_errors_and_return;
+
+  if (PyObject_TypeCheck (o2, &macro_object_type))
+    {
+      other_str = macropy_str (o2);
+      if (!other_str)
+	goto check_for_errors_and_return;
+
+      err_code = PyObject_Cmp (my_str, other_str, &comparison_result);
+    }
+  else
+    {
+      err_code = PyObject_Cmp (my_str, o2, &comparison_result);
+    }
+
+  check_for_errors_and_return:
+    Py_XDECREF (my_str);
+    Py_XDECREF (other_str);
+
+    /* Because -1 is also Less Than we cannot use gdbpy_print_stack ()
+       which clears the error if set python print-stack is off.
+       Which would lead to (gdb) python print x == x
+       returning False with no error message displayed.
+
+       alternately if an error is set and the return value is not 1,
+       python will complain.
+
+       Thus if there is an error, we must normalize it
+       making sure that both an error and the return
+       value are -1. Should we encounter one.  */
+    an_error = PyErr_Occurred ();
+    if (an_error == NULL && err_code != -1)
+      {
+        /* a valid comparison */
+        return comparison_result;
+      }
+    else if (an_error == NULL && err_code == -1)
+      {
+	/* an unknown error */
+	PyErr_SetString (PyExc_RuntimeError,
+			 _("mystery error during macro comparison."));
+        return -1;
+      }
+    else /* a known error */
+      return -1;
+}
+
+static long
+macropy_hash (PyObject *o)
+{
+  long result = -1;
+  PyObject *my_str = macropy_str (o);
+
+  if (my_str)
+    result = PyObject_Hash (my_str);
+
+  /* The docs say PyObject_Hash should return -1 on error,
+     Don't believe it because it is not always true,
+     The exception gets raised regardless of return value,
+     But we should follow the documented return strategy of -1 on error.
+
+     gdbpy_print_stack () clears the error if set python print-stack is off,
+     which would cause python later to complain that we returned -1
+     without an error set.  We don't want that.  */
+  if (PyErr_Occurred ())
+    result = -1;
+  Py_XDECREF (my_str);
+  return result;
+}
+
+\f
+
+void
+gdbpy_initialize_macros (void)
+{
+  macro_object_type.tp_new = PyType_GenericNew;
+  macro_validator_object_type.tp_new = PyType_GenericNew;
+
+  if (PyType_Ready (&macro_validator_object_type) < 0)
+    return;
+
+  if (PyType_Ready (&macro_object_type) < 0)
+    return;
+
+  macropy_objfile_data_key
+    = register_objfile_data_with_cleanup (NULL, del_objfile_macro);
+
+  Py_INCREF (&macro_object_type);
+  PyModule_AddObject (gdb_module, "Macro",
+		      (PyObject *) &macro_object_type);
+
+  Py_INCREF (&macro_validator_object_type);
+  PyModule_AddObject (gdb_module, "MacroValidator",
+		      (PyObject *) &macro_validator_object_type);
+}
+
+static PyGetSetDef macro_validator_object_getset[] = {
+  {NULL}  /* Sentinel */
+};
+
+static PyMethodDef macro_validator_object_methods[] = {
+  {NULL}  /* Sentinel */
+};
+
+static PyTypeObject macro_validator_object_type = {
+  PyObject_HEAD_INIT (NULL)
+  0,					  /*ob_size*/
+  "gdb.MacroValidator",			  /*tp_name*/
+  sizeof (macro_validator_object),	  /*tp_basicsize*/
+  0,					  /*tp_itemsize*/
+  macropy_validator_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,			  /*tp_flags*/
+  "GDB macro validator object",		  /*tp_doc */
+  0,					  /*tp_traverse */
+  0,					  /*tp_clear */
+  0,					  /*tp_richcompare */
+  0,					  /*tp_weaklistoffset */
+  0,					  /*tp_iter */
+  0,					  /*tp_iternext */
+  macro_validator_object_methods,	  /*tp_methods */
+  0,					  /*tp_members */
+  macro_validator_object_getset,	  /*tp_getset */
+};
+
+static PyGetSetDef macro_object_getset[] = {
+  {NULL}  /* Sentinel */
+};
+
+static PyMethodDef macro_object_methods[] = {
+  { "argv", macropy_argv_method, METH_NOARGS,
+    "argv () -> List.\n\
+Return a list containing the names of the arguments for a function like \
+macro." },
+  { "definition", macropy_definition_method, METH_NOARGS,
+    "definition () -> String.\n\
+Return the macro's definition." },
+  { "include_trail", macropy_include_trail_method, METH_NOARGS,
+    "include_trail () -> List.\n\
+Return a list containing tuples with the filename and line number describing \
+how and where this macro came to be defined." },
+  { "is_function_like", macropy_is_function_like_method, METH_NOARGS,
+    "is_function_like () -> Bool.\n\
+Return True if the macro is function like, False otherwise." },
+  { "name", macropy_name_method, METH_NOARGS,
+    "name () -> String.\n\
+Return the macro's name." },
+  { "is_valid", macropy_is_valid_method, METH_NOARGS,
+    "is_valid () -> Bool.\n\
+Return whether the macro is valid." },
+  {NULL}  /* Sentinel */
+};
+
+static PyTypeObject macro_object_type = {
+  PyObject_HEAD_INIT (NULL)
+  0,				  /*ob_size*/
+  "gdb.Macro",			  /*tp_name*/
+  sizeof (macro_object),	  /*tp_basicsize*/
+  0,				  /*tp_itemsize*/
+  macropy_dealloc,		  /*tp_dealloc*/
+  0,				  /*tp_print*/
+  0,				  /*tp_getattr*/
+  0,				  /*tp_setattr*/
+  macropy_compare,		  /*tp_compare*/
+  0,				  /*tp_repr*/
+  0,				  /*tp_as_number*/
+  0,				  /*tp_as_sequence*/
+  0,				  /*tp_as_mapping*/
+  macropy_hash,			  /*tp_hash */
+  0,				  /*tp_call*/
+  macropy_str,			  /*tp_str*/
+  0,				  /*tp_getattro*/
+  0,				  /*tp_setattro*/
+  0,				  /*tp_as_buffer*/
+  Py_TPFLAGS_DEFAULT,		  /*tp_flags*/
+  "GDB macro object",		  /*tp_doc */
+  0,				  /*tp_traverse */
+  0,				  /*tp_clear */
+  0,				  /*tp_richcompare */
+  0,				  /*tp_weaklistoffset */
+  0,				  /*tp_iter */
+  0,				  /*tp_iternext */
+  macro_object_methods,		  /*tp_methods */
+  0,				  /*tp_members */
+  macro_object_getset,		  /*tp_getset */
+};
diff --git a/gdb/python/py-macro.h b/gdb/python/py-macro.h
new file mode 100644
index 0000000..46b0293
--- /dev/null
+++ b/gdb/python/py-macro.h
@@ -0,0 +1,62 @@
+/* Python interface to macros.
+
+   Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_PY_MACRO_H
+#define GDB_PY_MACRO_H
+
+#include "objfiles.h"
+#include "macrotab.h"
+
+struct macropy_user_data {
+  PyObject *list;
+  struct objfile *objfile;
+};
+
+/* A macro_callback_fn for use with macro_foreach and friends that adds
+   a gdb.Macro object to the python list USER_DATA->LIST.
+   USER_DATA should be of type struct macropy_user_data.
+
+   A null USER_DATA->OBJFILE is currently be considered an error,
+   this is subject to change.
+
+   Aborts the iteration on error, and forces the macro iterator function
+   to return macro_walk_aborted check the iterators macro_walk_result.
+   On error it is the callers responsibility to dispose of the USER_DATA
+   structure and its contents.  The appropriate python exception
+   that caused the error has already been set.
+*/
+enum macro_walk_status
+pymacro_add_macro_to_list (const char *name,
+		   const struct macro_definition *definition,
+		   struct macro_source_file *source,
+		   int line,
+		   void *user_data);
+
+/* Create a new macro (gdb.Macro) object that encapsulates
+   the gdb macro representation.  OBJFILE is the objfile that contains
+   the macro table.  NAME is the name of the macro.  DEF the macros definition
+   SRC the source file containing the macro. LINE definitions location.  */
+PyObject *
+macropy_object_create (struct objfile *objfile,
+		       const char *name,
+		       const struct macro_definition *def,
+		       struct macro_source_file *src,
+		       int line);
+
+#endif /* GDB_PY_MACRO_H */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 996b23b..532552a 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -211,6 +211,7 @@ void gdbpy_initialize_breakpoint_event (void);
 void gdbpy_initialize_continue_event (void);
 void gdbpy_initialize_exited_event (void);
 void gdbpy_initialize_thread_event (void);
+void gdbpy_initialize_macros (void);
 
 struct cleanup *make_cleanup_py_decref (PyObject *py);
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 03edce9..092b354 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1198,6 +1198,7 @@ Enables or disables printing of Python stack traces."),
   gdbpy_initialize_lazy_string ();
   gdbpy_initialize_thread ();
   gdbpy_initialize_inferior ();
+  gdbpy_initialize_macros ();
   gdbpy_initialize_events ();
 
   gdbpy_initialize_eventregistry ();
-- 
1.7.4.4

[PATCH 7/7] [python] API for macros: Add tests.

[matt rice]

2011-08-23  Matt Rice  <ratmice@gmail.com>

	PR python/10807
	* gdb.python/py-macro.exp: Ditto.
	* gdb.python/py-macro.c: New file.
---
 gdb/testsuite/gdb.python/py-macro.c   |   87 +++++++++
 gdb/testsuite/gdb.python/py-macro.exp |  338 +++++++++++++++++++++++++++++++++
 2 files changed, 425 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-macro.c
 create mode 100644 gdb/testsuite/gdb.python/py-macro.exp

diff --git a/gdb/testsuite/gdb.python/py-macro.c b/gdb/testsuite/gdb.python/py-macro.c
new file mode 100644
index 0000000..3397eb5
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-macro.c
@@ -0,0 +1,87 @@
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef DEF_MACROS
+
+  #ifdef ONE
+    #ifdef FOO
+    #undef FOO
+    #endif
+
+  #define FOO "hello"
+  #else
+    #undef FOO
+  #endif
+
+
+  #ifdef TWO
+    #ifdef FOO
+    #undef FOO
+    #endif
+  #define FOO " "
+  #endif
+
+  #ifdef THREE
+    #ifdef FOO
+    #undef FOO
+    #endif
+  #define FOO(a,b)
+  #endif
+
+  #ifdef FOUR
+    #ifdef FOO
+    #undef FOO
+    #endif
+    #define FOO(a) foo = a
+  #endif
+#else
+
+int main (int argc, const char **argv)
+{
+  char *foo;
+
+  #define DEF_MACROS
+  #define ONE
+  #include "py-macro.c"
+  foo = FOO;
+
+  #define TWO
+  #include "py-macro.c"
+  foo = FOO;
+
+  #define THREE
+  #include "py-macro.c"
+  foo = "world"FOO(0,1);
+
+  #undef THREE
+  #include "py-macro.c"
+  foo = FOO;
+
+  #undef TWO
+  #include "py-macro.c"
+  foo = FOO;
+
+  #undef ONE
+  #include "py-macro.c"
+  foo = (char *)0;
+
+  #define FOUR
+  #include "py-macro.c"
+  FOO ("the end.");
+
+  return 0;
+}
+#endif
+
diff --git a/gdb/testsuite/gdb.python/py-macro.exp b/gdb/testsuite/gdb.python/py-macro.exp
new file mode 100644
index 0000000..203e1b4
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-macro.exp
@@ -0,0 +1,338 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests the mechanism
+# exposing macros to Python.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib gdb-python.exp
+
+gdb_start
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+set testfile "py-macro"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+get_compiler_info ${binfile}
+if [test_compiler_info gcc*] {
+    lappend options additional_flags=-g3
+} else {
+  untested ${testfile}.exp
+  return -1
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} $options] } {
+    untested ${testfile}.exp
+    return -1
+}
+
+# Start with a fresh gdb.
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+
+gdb_py_test_multiple "setup macro test objects" \
+  "python" "" \
+  "def find_objfile():" "" \
+  "  for objfile in gdb.objfiles():" "" \
+  "    if objfile.filename == \"$binfile\":" "" \
+  "      print objfile.filename" "" \
+  "      return objfile" "" \
+  "def here_macro(name):" "" \
+  "  return gdb.decode_line(\"*\$pc\")\[1\]\[0\].macro_named(name)" "" \
+  "macros = list()" "" \
+  "end" ""
+
+gdb_test "python objfile = find_objfile()" "$binfile" "found objfile"
+gdb_test "python print objfile.symtabs()" \
+	 "\\\[\\\]" "symtabs is empty"
+
+if ![runto_main] {
+    untested ${testfile}.exp
+    return -1
+}
+
+gdb_py_test_silent_cmd "python m = objfile.symtabs()\[0\].macros()" "get all macros" 1
+
+gdb_py_test_silent_cmd "python include_guard = here_macro(\"DEF_MACROS\")" \
+		       "get guard macro #1" \
+		       1
+
+gdb_test "python print include_guard.is_valid()" \
+	 "True" \
+	 "guard is valid"
+
+gdb_test "python print include_guard.name() == \"DEF_MACROS\"" \
+	 "True" \
+	 "guard name"
+
+gdb_test "python print include_guard.is_function_like() == False" \
+	 "True" \
+	 "guard isnt function like"
+
+gdb_test "python print include_guard.argv() == None" \
+	 "True" \
+	 "guard has no args"
+
+gdb_test "python print 'foo' + include_guard.definition() + 'bar'" \
+	 "foobar" \
+	 "guard definition is empty"
+
+gdb_test "python print include_guard.include_trail()\[0\]\[0\]" \
+	 "$srcfile.*" \
+	 "guard trail has srcfile"
+
+#<gdb.Macro include_trail=[('./gdb.python/py-macro.c', 8), ('./gdb.python/py-macro.c', 43)] FOO="hello">
+gdb_test "python print include_guard" \
+"<gdb\.Macro DEF_MACROS include_trail=\\\[\\\('$srcdir/$subdir/$srcfile', \[0-9\]+\\\).*\\\]>" \
+"guard string rep"
+
+gdb_py_test_silent_cmd "python macros.append(include_guard)" \
+		       "add to macros list" \
+		       0
+
+gdb_py_test_silent_cmd "python foo = here_macro(\"FOO\")" \
+		       "get FOO macro #1" \
+		       1
+
+gdb_test "python print foo.is_valid()" \
+	 "True" \
+	 "FOO macro #1 is valid"
+
+gdb_test "python print foo.name()" \
+	 "FOO" \
+	 "FOO macro #1 name"
+
+gdb_test "python print foo.is_function_like()" \
+	 "False" \
+	 "FOO macro #1 isnt function like"
+
+gdb_test "python print foo.argv()" \
+	 "None" \
+	 "FOO macro #1 has no args"
+
+gdb_test "python print foo.definition()" \
+	 "\"hello\"" \
+	 "FOO macro #1 definition is \"hello\""
+
+gdb_test "python print include_guard.include_trail()\[0\]\[0\]" \
+	 "$srcfile.*" \
+	 "FOO macro #1 has srcfile"
+
+gdb_test "python print foo" \
+"<gdb\.Macro FOO=\"hello\" include_trail=\\\[\\\('$srcdir/$subdir/$srcfile', \[0-9\]+\\\).*\\\]>" \
+"FOO macro #1 string rep"
+
+gdb_py_test_silent_cmd "python macros.append(foo)" \
+		       "add to macros list" \
+		       0
+
+gdb_test "next" "" ""
+gdb_py_test_silent_cmd "python foo = here_macro(\"FOO\")" \
+		       "get FOO macro #2" \
+		       1
+
+gdb_test "python print foo.is_valid()" \
+	 "True" \
+	 "FOO macro #2 is valid"
+
+gdb_test "python print foo.name()" \
+	 "FOO" \
+	 "FOO macro #2 name"
+
+gdb_test "python print foo.is_function_like()" \
+	 "False" \
+	 "FOO macro #2 isnt function like"
+
+gdb_test "python print foo.argv()" \
+	 "None" \
+	 "FOO macro #2 has no args"
+
+gdb_test "python print foo.definition()" \
+	 "\" \"" \
+	 "FOO macro #2 definition is \" \""
+
+gdb_test "python print foo.include_trail()\[0\]\[0\]" \
+	 "$srcfile.*" \
+	 "FOO macro #2 has srcfile"
+
+gdb_test "python print foo" \
+"<gdb\.Macro FOO=\" \" include_trail=\\\[\\\('$srcdir/$subdir/$srcfile', \[0-9\]+\\\).*\\\]>" \
+"FOO macro #2 string rep"
+
+gdb_py_test_silent_cmd "python macros.append(foo)" \
+		       "add to macros list" \
+		       0
+
+gdb_test "next" "" ""
+gdb_py_test_silent_cmd "python foo = here_macro(\"FOO\")" \
+		       "get FOO macro #3" \
+		       1
+gdb_test "python print foo.is_valid()" \
+	 "True" \
+	 "FOO macro #3 is valid"
+
+gdb_test "python print foo.name()" \
+	 "FOO" \
+	 "FOO macro #3 name"
+
+gdb_test "python print foo.is_function_like()" \
+	 "True" \
+	 "FOO macro #3 is function like"
+
+gdb_test "python print foo.argv()" \
+	 "\\\['a', 'b'\\\]" \
+	 "FOO macro #3 has args"
+
+gdb_test "python print 'foo' + foo.definition() + 'bar'" \
+	 "foobar" \
+	 "FOO macro #3 definition is empty"
+
+gdb_test "python print include_guard.include_trail()\[0\]\[0\]" \
+	 "$srcfile.*" \
+	 "FOO macro #3 has srcfile"
+
+gdb_test "python print foo" \
+"<gdb\.Macro FOO\\\(a, b\\\) include_trail=\\\[\\\('$srcdir/$subdir/$srcfile', \[0-9\]+\\\).*\\\]>" \
+"FOO macro #3 string rep"
+
+gdb_py_test_silent_cmd "python macros.append(foo)" \
+		       "add to macros list" \
+		       0
+gdb_test "next" "" ""
+gdb_test "next" "" ""
+gdb_test "next" "" ""
+gdb_py_test_silent_cmd "python foo = here_macro(\"FOO\")" \
+		       "get FOO macro #4" \
+		       1
+gdb_test "python print foo" \
+	 "None" \
+	 "there is no Foo macro #4"
+
+gdb_test "next" "" ""
+gdb_py_test_silent_cmd "python foo = here_macro(\"FOO\")" \
+		       "get FOO macro #5" \
+		       1
+
+gdb_test "python print foo.is_valid()" \
+	 "True" \
+	 "FOO macro #5 is valid"
+
+gdb_test "python print foo.name()" \
+	 "" \
+	 "FOO macro #5 name"
+
+gdb_test "python print foo.is_function_like()" \
+	 "True" \
+	 "FOO macro #5 is function like"
+
+gdb_test "python print foo.argv()" \
+	 "\\\['a'\\\]" \
+	 "FOO macro #5 has args"
+
+gdb_test "python print foo.definition()" \
+	 "foo = a" \
+	 "FOO macro #5 definition ok"
+
+gdb_test "python print include_guard.include_trail()\[0\]\[0\]" \
+	 "$srcfile.*" \
+	 "FOO macro #5 has srcfile"
+
+gdb_test "python print foo" \
+"<gdb\.Macro FOO\\\(a\\\)=foo = a include_trail=\\\[\\\('$srcdir/$subdir/$srcfile', \[0-9\]+\\\).*\\\]>" \
+"FOO macro #5 string rep"
+gdb_py_test_silent_cmd "python macros.append(foo)" \
+		       "add to macros list" \
+		       0
+
+# this could find some ref count bugs if they were to happen for a singleton.
+gdb_py_test_multiple "run macros a couple of times" \
+  "python" "" \
+  "c = 0" "" \
+  "while c > 3:" "" \
+  "  c = c + 1" "" \
+  "  for macro in objfile.symtabs()\[0\].macros():" "" \
+  "    macro.name()" "" \
+  "    macro.is_function_like()" "" \
+  "    macro.argv()" "" \
+  "    macro.include_path()" "" \
+  "    macro.definition()" "" \
+  "    str(macro)" "" \
+  "    hash(macro)" "" \
+  "end" ""
+
+gdb_py_test_multiple "find selected macros in the big list of macros" \
+  "python" "" \
+  "set1 = set(objfile.symtabs()\[0\].macros())" "" \
+  "set2 = set(macros)" "" \
+  "intersect = (set1 & set2)" "" \
+  "set3 = set(filter(lambda(x): x.name() == \"FOO\", objfile.symtabs()\[0\].macros()))" "" \
+  "intersect2 = (set3 & set2)" "" \
+  "print \"set2\", map(lambda(x): x.name(), set2)" "" \
+  "print \"set3\", map(lambda(x): x.name(), set3)" "" \
+  "print \"intersect\", map(lambda(x): x.name(), intersect)" "" \
+  "print \"intersect2\", map(lambda(x): x.name(), intersect2)" "" \
+  "print \"set2 - intersect\", map(lambda(x): x.name(), set2 - intersect)" "" \
+  "print \"intersect - set2\", map(lambda(x): x.name(), intersect - set2)" "" \
+  "print \"set2 - intersect2\", map(lambda(x): x.name(), set2 - intersect2)" "" \
+  "print \"intersect2 - set2\", map(lambda(x): x.name(), intersect2 - set2)" "" \
+  "end" ""
+
+gdb_test "python print len(set2)" "5" "macro set length 5"
+gdb_test "python print len(set1) > len(set2)" "True" "all macros is longer."
+gdb_test "python print set1 >= set2" "True" "macro set2 is a subset"
+gdb_test "python print len(intersect - set2), len(set2 - intersect)" \
+	 "0 0" \
+	 "macro set intersection equality"
+gdb_test "python print len(intersect2 - set2)" \
+	 "0" \
+	 "macro set intersection equality 2"
+gdb_test "python print len(set2 - intersect2), (set2 - intersect2).pop()" \
+	 "1 <gdb.Macro DEF_MACROS.*>" \
+	 "macro set intersection 3"
+
+gdb_unload
+# test is_valid()
+gdb_test "python print include_guard.is_valid()" "False" \
+	 "guard macro is invalid after unload"
+
+gdb_test "python print foo.is_valid()" "False" \
+	 "foo macro is invalid after unload"
+
+# test individual method's to make sure they throw exceptions.
+gdb_test "python print foo.argv()" "Macro is invalid.*" \
+	 "foo argv method is invalid after unload"
+
+gdb_test "python print foo.definition()" "Macro is invalid.*" \
+	 "foo definition method is invalid after unload"
+
+gdb_test "python print foo.include_trail()" "Macro is invalid.*" \
+	 "foo include_trail method is invalid after unload"
+
+gdb_test "python print foo.is_function_like()" "Macro is invalid.*" \
+	 "foo is_function_like method is invalid after unload"
+
+gdb_test "python print foo.name()" "Macro is invalid.*" \
+	 "foo name method is invalid after unload"
+
+gdb_test "python print str(foo)" "Macro is invalid.*" \
+	 "str(foo) method is invalid after unload"
+
-- 
1.7.4.4

[PATCH 6/7] [python] API for macros: Add docs.

[matt rice]

2011-08-23  Matt Rice  <ratmice@gmail.com>

	* gdb.texinfo (Symbol Tables In Python): Add macros and macro_named methods.
	(Objfiles In Python): Add symtabs method.
	(Macros In Python): Add new section.

2011-08-23  Matt Rice  <ratmice@gmail.com>

	* NEWS: Add macros API, and Objfiles symtabs method.
---
 gdb/NEWS            |    7 ++++
 gdb/doc/gdb.texinfo |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 255a22e..a719b07 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -38,6 +38,13 @@
 
   ** Symbols now provide the "type" attribute, the type of the symbol.
 
+  ** Objfiles now provide a "symtabs" method.
+
+  ** The Macro object is now available for representing preprocessor macros.
+     The following objects now have methods for obtaining macro objects.
+     - Symtab_and_line now has a "macro_named" method
+     - Symtab now has a "macros" method.
+
 * libthread-db-search-path now supports two special values: $sdir and $pdir.
   $sdir specifies the default system locations of shared libraries.
   $pdir specifies the directory where the libpthread used by the application
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6e7bf52..1073a2e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20984,6 +20984,7 @@ situation, a Python @code{KeyboardInterrupt} exception is thrown.
 * Blocks In Python::            Accessing frame blocks from Python.
 * Symbols In Python::           Python representation of symbols.
 * Symbol Tables In Python::     Python representation of symbol tables.
+* Macros In Python::            Python representation of preprocessor macros.
 * Lazy Strings In Python::      Python representation of lazy strings.
 * Breakpoints In Python::       Manipulating breakpoints using Python.
 @end menu
@@ -22986,6 +22987,11 @@ longer.  All other @code{gdb.Objfile} methods will throw an exception
 if it is invalid at the time the method is called.
 @end defmethod
 
+@defmethod Objfile symtabs
+Returns a @code{List} containing the valid @code{gdb.Symtab} objects
+for this @code{gdb.Objfile}.
+@end defmethod
+
 @node Frames In Python
 @subsubsection Accessing inferior stack frames from Python.
 
@@ -23446,6 +23452,17 @@ exist in @value{GDBN} any longer.  All other
 @code{gdb.Symtab_and_line} methods will throw an exception if it is
 invalid at the time the method is called.
 @end defmethod
+
+@defmethod Symtab_and_line macro_named @r{[}name@r{]}
+Returns a @code{gdb.Macro} object, for the macro
+with the given name.
+@end defmethod
+
+@defmethod Symtab_and_line macros
+Returns all of the macros which are defined and not undefined or redefined,
+prior to the the @code{gdb.Symtab_and_line}'s line attribute.
+Thus, all the macros which would be validly usable at that line.
+@end defmethod
 @end table
 
 A @code{gdb.Symtab} object has the following attributes:
@@ -23475,6 +23492,66 @@ if it is invalid at the time the method is called.
 @defmethod Symtab fullname
 Return the symbol table's source absolute file name.
 @end defmethod
+
+@defmethod Symtab macros
+Return all of the macros contained in the symbol table.
+@end defmethod
+@end table
+
+@node Macros In Python
+@subsubsection Python representation of preprocessor macros.
+
+@cindex macros in python
+@tindex gdb.macro
+
+Python code can query information about preprocessor macros using the
+@code{gdb.Macro} class.  For obtaining a @code{gdb.Macro} object see
+@xref{Symbol Tables In Python}.
+
+@code{gdb.Macro} methods may produce a lazily created python copy from
+gdb's internal representation possibly returning different copies
+that contain same value when called multiple times.
+
+The @code{gdb.Macro} class should be considered an abstract class,
+instances possibly being of different types.  Thus, you should always call
+its functions directly from an instance, And never reference a method
+through the @code{gdb.Macro} class object.  For example you should use
+@code{map(lambda macro: macro.name(), macros)} instead of
+@code{map(gdb.Macro.name, macros)}.
+
+@table @code
+@defmethod Macro argv
+Returns a list of the macros arguments if the macro is function like.
+If the macro is not function like, returns @code{None}
+@end defmethod
+
+@defmethod Macro definition
+Returns a string containing the definition of the macro.
+@end defmethod
+
+@defmethod Macro include_trail
+Returns a list of tuples containing the filenames, and line numbers
+of header and source files that correspond to the include directives,
+and location of definition.  The list is sorted starting with the place
+of definition, and ending with the first include directive.
+@end defmethod
+
+@defmethod Macro is_function_like
+Returns @code{True} If the macro is function like.
+@end defmethod
+
+@defmethod Macro is_valid
+Returns @code{True} if the macro is valid.
+All other @code{gdb.Macro} methods will throw an exception if the object is
+invalid at the time the method is called.
+A macro which becomes invalid will never be valid again
+without looking up the macro again to create a new @code{gdb.Macro}
+object.
+@end defmethod
+
+@defmethod Macro name
+Returns the name of the macro.
+@end defmethod
 @end table
 
 @node Breakpoints In Python
-- 
1.7.4.4

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Eli Zaretskii]

> From: matt rice <ratmice@gmail.com>
> Cc: matt rice <ratmice@gmail.com>
> Date: Wed, 24 Aug 2011 08:10:53 -0700
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -38,6 +38,13 @@
>  
>    ** Symbols now provide the "type" attribute, the type of the symbol.
>  
> +  ** Objfiles now provide a "symtabs" method.
> +
> +  ** The Macro object is now available for representing preprocessor macros.
> +     The following objects now have methods for obtaining macro objects.
> +     - Symtab_and_line now has a "macro_named" method
> +     - Symtab now has a "macros" method.

This part is okay, but please add a reference to the section in the
manual where these are described.

> +@defmethod Symtab_and_line macro_named @r{[}name@r{]}
> +Returns a @code{gdb.Macro} object, for the macro
> +with the given name.
> +@end defmethod

This should say what happens if "name" is omitted.  It's optional,
right?

> +@defmethod Symtab_and_line macros
> +Returns all of the macros which are defined and not undefined or redefined,

"not redefined"? what exactly does this mean, and how can GDB
distinguish between something that is defined and something that is
defined and then redefined?

> +prior to the the @code{gdb.Symtab_and_line}'s line attribute.
> +Thus, all the macros which would be validly usable at that line.

I would rephrase:

 Returns all of the macros which are in effect for the source line
 given by the @code{gdb.Symtab_and_line}'s @code{line} attribute.

> +@defmethod Symtab macros
> +Return all of the macros contained in the symbol table.
> +@end defmethod

Return what, exactly? only their names? something else?

> +@code{gdb.Macro} class.  For obtaining a @code{gdb.Macro} object see
> +@xref{Symbol Tables In Python}.

You want "see @ref".  @xref produces a capitalized "Note:", which will
look as bad English; use @xref only at the beginning of a sentence.

> +@code{gdb.Macro} methods may produce a lazily created python copy from
                                                         ^^^^^^
"Python".

> +gdb's internal representation possibly returning different copies
   ^^^
@value{GDBN}

> +The @code{gdb.Macro} class should be considered an abstract class,
> +instances possibly being of different types.  Thus, you should always call
> +its functions directly from an instance, And never reference a method
                                            ^^^
"and"

> +@defmethod Macro argv
> +Returns a list of the macros arguments if the macro is function like.
> +If the macro is not function like, returns @code{None}

 Returns the argument list of the macro, or @code{None} if the macro
 does not accept arguments.

> +@defmethod Macro definition
> +Returns a string containing the definition of the macro.
> +@end defmethod

 Returns a string containing the macro body.

> +@defmethod Macro include_trail
> +Returns a list of tuples containing the filenames, and line numbers
                                                    ^
This comma is redundant.

> +of header and source files that correspond to the include directives,
> +and location of definition.  The list is sorted starting with the place
> +of definition, and ending with the first include directive.
> +@end defmethod

An example of this list of tuples would go a long way towards making
this description more clear.

> +@defmethod Macro is_function_like
> +Returns @code{True} If the macro is function like.
> +@end defmethod

  Returns @code{True} if the macro accepts arguments.

> +@defmethod Macro is_valid
> +Returns @code{True} if the macro is valid.

And presumably False otherwise.

> +All other @code{gdb.Macro} methods will throw an exception if the object is

What exception?  Is that important to document?

> +invalid at the time the method is called.
> +A macro which becomes invalid will never be valid again
> +without looking up the macro again to create a new @code{gdb.Macro}
> +object.
> +@end defmethod

Please tell here what does it mean for a macro to be "valid".  Also,
how to "look up the macro again".  Finally, it sounds like this method
should be the first one documented, as it is the only one that is
always usable.

Thanks.

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Matt Rice]

On Wed, Aug 24, 2011 at 1:09 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: matt rice <ratmice@gmail.com>
>> Cc: matt rice <ratmice@gmail.com>
>> Date: Wed, 24 Aug 2011 08:10:53 -0700
>>
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -38,6 +38,13 @@
>>
>>    ** Symbols now provide the "type" attribute, the type of the symbol.
>>
>> +  ** Objfiles now provide a "symtabs" method.
>> +
>> +  ** The Macro object is now available for representing preprocessor macros.
>> +     The following objects now have methods for obtaining macro objects.
>> +     - Symtab_and_line now has a "macro_named" method
>> +     - Symtab now has a "macros" method.
>
> This part is okay, but please add a reference to the section in the
> manual where these are described.

ok, will send a new patch after the we get the below stuff figured out.

>> +@defmethod Symtab_and_line macro_named @r{[}name@r{]}
>> +Returns a @code{gdb.Macro} object, for the macro
>> +with the given name.
>> +@end defmethod
>
> This should say what happens if "name" is omitted.  It's optional,
> right?

Nope, removed the []'s

>> +@defmethod Symtab_and_line macros
>> +Returns all of the macros which are defined and not undefined or redefined,
>
> "not redefined"? what exactly does this mean, and how can GDB
> distinguish between something that is defined and something that is
> defined and then redefined?

the symtab_and_line method distinguishes something that is defined and
then redefined by using the last defined version of the macro (in this
case)

here's an example and later a question

macro1.h:1:#define AMACRO

macro1.c:1:#include "macro1.h"
macro1.c:2:
macro1.c:3:void foo() {};
macro1.c:4:
macro1.c:5:int main()
macro1.c:6:{
macro1.c:7:  #define A
macro1.c:8:  #define B
macro1.c:9:  bp1:
macro1.c:10:  foo();
macro1.c:11:  #undef A
macro1.c:12:  #undef B
macro1.c:13:  #define B 1
macro1.c:14:  bp2:
macro1.c:15:  foo();
macro1.c:16:  #define C
macro1.c:17:  bp3:
macro1.c:18:  foo(); C; return 0;
macro1.c:19:}

lets say we break at bp1 bp2 and bp3 labels and output the
Symtab_and_line.macros(), and get rid of all of the compiler generated
macros.

macro1.c:10 <gdb.Macro B include_trail=[('/home/ratmice/tests/macro1.c', 8)]>
macro1.c:10 <gdb.Macro A include_trail=[('/home/ratmice/tests/macro1.c', 7)]>
macro1.c:10 <gdb.Macro AMACRO
include_trail=[('/home/ratmice/tests/macro1.h', 1),
('/home/ratmice/tests/macro1.c', 1)]>

macro1.c:15 <gdb.Macro AMACRO
include_trail=[('/home/ratmice/tests/macro1.h', 1),
('/home/ratmice/tests/macro1.c', 1)]>
macro1.c:15 <gdb.Macro B=1 include_trail=[('/home/ratmice/tests/macro1.c', 13)]>

macro1.c:18 <gdb.Macro AMACRO
include_trail=[('/home/ratmice/tests/macro1.h', 1),
('/home/ratmice/tests/macro1.c', 1)]>
macro1.c:18 <gdb.Macro C include_trail=[('/home/ratmice/tests/macro1.c', 16)]>
macro1.c:18 <gdb.Macro B=1 include_trail=[('/home/ratmice/tests/macro1.c', 13)]>


>> +prior to the the @code{gdb.Symtab_and_line}'s line attribute.
>> +Thus, all the macros which would be validly usable at that line.
>
> I would rephrase:
>
>  Returns all of the macros which are in effect for the source line
>  given by the @code{gdb.Symtab_and_line}'s @code{line} attribute.

The problem I was trying to avoid (and which made my documentation for
this method admittedly crappy), is that your rephrased definition
seems to be plausible for the case when the user wants (C) in the
macro1.c:18 case,
e.g. the macros which are used ON the line.  When Symtab_and_line.macros()
outputs all of the macros which were defined before the line, which
are still in effect.

>> +@defmethod Symtab macros
>> +Return all of the macros contained in the symbol table.
>> +@end defmethod
>
> Return what, exactly? only their names? something else?

i'll try 'Return a list of macro objects for all of the macros
contained in the symbol table.'

>> +@defmethod Macro is_function_like
>> +Returns @code{True} If the macro is function like.
>> +@end defmethod
>
>  Returns @code{True} if the macro accepts arguments.

I understand the intent to make this documentation not so self-recursive
I tried to think of a way too, but the problem with this is the rare
function-like macro which accepts no arguments, e.g.

/usr/include/curses.h:#define standout()		wstandout(stdscr)

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Eli Zaretskii]

> Date: Thu, 25 Aug 2011 05:32:46 -0700
> From: Matt Rice <ratmice@gmail.com>
> Cc: gdb-patches@sourceware.org
> 
> macro1.h:1:#define AMACRO
> 
> macro1.c:1:#include "macro1.h"
> macro1.c:2:
> macro1.c:3:void foo() {};
> macro1.c:4:
> macro1.c:5:int main()
> macro1.c:6:{
> macro1.c:7:  #define A
> macro1.c:8:  #define B
> macro1.c:9:  bp1:
> macro1.c:10:  foo();
> macro1.c:11:  #undef A
> macro1.c:12:  #undef B
> macro1.c:13:  #define B 1
> macro1.c:14:  bp2:
> macro1.c:15:  foo();
> macro1.c:16:  #define C
> macro1.c:17:  bp3:
> macro1.c:18:  foo(); C; return 0;
> macro1.c:19:}
> 
> lets say we break at bp1 bp2 and bp3 labels and output the
> Symtab_and_line.macros(), and get rid of all of the compiler generated
> macros.
> 
> macro1.c:10 <gdb.Macro B include_trail=[('/home/ratmice/tests/macro1.c', 8)]>
> macro1.c:10 <gdb.Macro A include_trail=[('/home/ratmice/tests/macro1.c', 7)]>
> macro1.c:10 <gdb.Macro AMACRO
> include_trail=[('/home/ratmice/tests/macro1.h', 1),
> ('/home/ratmice/tests/macro1.c', 1)]>
> 
> macro1.c:15 <gdb.Macro AMACRO
> include_trail=[('/home/ratmice/tests/macro1.h', 1),
> ('/home/ratmice/tests/macro1.c', 1)]>
> macro1.c:15 <gdb.Macro B=1 include_trail=[('/home/ratmice/tests/macro1.c', 13)]>
> 
> macro1.c:18 <gdb.Macro AMACRO
> include_trail=[('/home/ratmice/tests/macro1.h', 1),
> ('/home/ratmice/tests/macro1.c', 1)]>
> macro1.c:18 <gdb.Macro C include_trail=[('/home/ratmice/tests/macro1.c', 16)]>
> macro1.c:18 <gdb.Macro B=1 include_trail=[('/home/ratmice/tests/macro1.c', 13)]>

So far as expected.

> >> +prior to the the @code{gdb.Symtab_and_line}'s line attribute.
> >> +Thus, all the macros which would be validly usable at that line.
> >
> > I would rephrase:
> >
> >  Returns all of the macros which are in effect for the source line
> >  given by the @code{gdb.Symtab_and_line}'s @code{line} attribute.
> 
> The problem I was trying to avoid (and which made my documentation for
> this method admittedly crappy), is that your rephrased definition
> seems to be plausible for the case when the user wants (C) in the
> macro1.c:18 case,
> e.g. the macros which are used ON the line.  When Symtab_and_line.macros()
> outputs all of the macros which were defined before the line, which
> are still in effect.

Sorry, I don't follow.  Did you mean "B" instead of "C"?  What is the
problem you see here with "C"?  It is defined only once, on line 16,
and so is in effect on line 18.  I see no issues here.  What am I
missing?

> >> +@defmethod Symtab macros
> >> +Return all of the macros contained in the symbol table.
> >> +@end defmethod
> >
> > Return what, exactly? only their names? something else?
> 
> i'll try 'Return a list of macro objects for all of the macros
> contained in the symbol table.'

Based on the example above (which I highly recommend to have in the
manual), I'd say "a list of macro objects with their values and
include trail".

> >> +@defmethod Macro is_function_like
> >> +Returns @code{True} If the macro is function like.
> >> +@end defmethod
> >
> >  Returns @code{True} if the macro accepts arguments.
> 
> I understand the intent to make this documentation not so self-recursive
> I tried to think of a way too, but the problem with this is the rare
> function-like macro which accepts no arguments, e.g.
> 
> /usr/include/curses.h:#define standout()		wstandout(stdscr)

This is marginal enough to have a special exception:

  Returns @code{True} if the macro accepts an argument list.  A
  function-like macro that accepts an empty argument list also yields
  @code{True}.

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Matt Rice]

On Thu, Aug 25, 2011 at 10:35 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Thu, 25 Aug 2011 05:32:46 -0700
>> From: Matt Rice <ratmice@gmail.com>
>> Cc: gdb-patches@sourceware.org
>>
>> macro1.h:1:#define AMACRO
>>
>> macro1.c:1:#include "macro1.h"
>> macro1.c:2:
>> macro1.c:3:void foo() {};
>> macro1.c:4:
>> macro1.c:5:int main()
>> macro1.c:6:{
>> macro1.c:7:  #define A
>> macro1.c:8:  #define B
>> macro1.c:9:  bp1:
>> macro1.c:10:  foo();
>> macro1.c:11:  #undef A
>> macro1.c:12:  #undef B
>> macro1.c:13:  #define B 1
>> macro1.c:14:  bp2:
>> macro1.c:15:  foo();
>> macro1.c:16:  #define C
>> macro1.c:17:  bp3:
>> macro1.c:18:  foo(); C; return 0;
>> macro1.c:19:}
>>
>> lets say we break at bp1 bp2 and bp3 labels and output the
>> Symtab_and_line.macros(), and get rid of all of the compiler generated
>> macros.
>>
>> macro1.c:10 <gdb.Macro B include_trail=[('/home/ratmice/tests/macro1.c', 8)]>
>> macro1.c:10 <gdb.Macro A include_trail=[('/home/ratmice/tests/macro1.c', 7)]>
>> macro1.c:10 <gdb.Macro AMACRO
>> include_trail=[('/home/ratmice/tests/macro1.h', 1),
>> ('/home/ratmice/tests/macro1.c', 1)]>
>>
>> macro1.c:15 <gdb.Macro AMACRO
>> include_trail=[('/home/ratmice/tests/macro1.h', 1),
>> ('/home/ratmice/tests/macro1.c', 1)]>
>> macro1.c:15 <gdb.Macro B=1 include_trail=[('/home/ratmice/tests/macro1.c', 13)]>
>>
>> macro1.c:18 <gdb.Macro AMACRO
>> include_trail=[('/home/ratmice/tests/macro1.h', 1),
>> ('/home/ratmice/tests/macro1.c', 1)]>
>> macro1.c:18 <gdb.Macro C include_trail=[('/home/ratmice/tests/macro1.c', 16)]>
>> macro1.c:18 <gdb.Macro B=1 include_trail=[('/home/ratmice/tests/macro1.c', 13)]>
>
> So far as expected.
>
>> >> +prior to the the @code{gdb.Symtab_and_line}'s line attribute.
>> >> +Thus, all the macros which would be validly usable at that line.
>> >
>> > I would rephrase:
>> >
>> >  Returns all of the macros which are in effect for the source line
>> >  given by the @code{gdb.Symtab_and_line}'s @code{line} attribute.
>>
>> The problem I was trying to avoid (and which made my documentation for
>> this method admittedly crappy), is that your rephrased definition
>> seems to be plausible for the case when the user wants (C) in the
>> macro1.c:18 case,
>> e.g. the macros which are used ON the line.  When Symtab_and_line.macros()
>> outputs all of the macros which were defined before the line, which
>> are still in effect.
>
> Sorry, I don't follow.  Did you mean "B" instead of "C"?  What is the
> problem you see here with "C"?  It is defined only once, on line 16,
> and so is in effect on line 18.  I see no issues here.  What am I
> missing?

No, I mean "C".

>> macro1.c:18:  foo(); C; return 0;

So say we are stopped on this line, and have Symtab_and_line for it.
the user wants to know `all the macros that are used at this source location'
In this case the answer would be 'C', and my concern is that 'which
are in effect'
can be misconstrued.  e.g. with the above line in isolation, is 'C' in effect?
how about the following.

Returns all of the macros defined before the source line given by the
@code{gdb.Symtab_and_line}'s @code{line} attribute which are in still
effect.

>> >> +@defmethod Symtab macros
>> >> +Return all of the macros contained in the symbol table.
>> >> +@end defmethod
>> >
>> > Return what, exactly? only their names? something else?
>>
>> i'll try 'Return a list of macro objects for all of the macros
>> contained in the symbol table.'
>
> Based on the example above (which I highly recommend to have in the
> manual), I'd say "a list of macro objects with their values and
> include trail".

hrm, except what is above is the output of the string function,
if you actually print the return value without converting to a string
it prints something like (<gdb.Macro 0x.......>, <gdb.Macro 0x.....>),

But that is fine if you prefer it that way.

>> >> +@defmethod Macro is_function_like
>> >> +Returns @code{True} If the macro is function like.
>> >> +@end defmethod
>> >
>> >  Returns @code{True} if the macro accepts arguments.
>>
>> I understand the intent to make this documentation not so self-recursive
>> I tried to think of a way too, but the problem with this is the rare
>> function-like macro which accepts no arguments, e.g.
>>
>> /usr/include/curses.h:#define standout()              wstandout(stdscr)
>
> This is marginal enough to have a special exception:
>
>  Returns @code{True} if the macro accepts an argument list.  A
>  function-like macro that accepts an empty argument list also yields
>  @code{True}.
>

ok, thanks.

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Eli Zaretskii]

> Date: Fri, 26 Aug 2011 01:04:21 -0700
> From: Matt Rice <ratmice@gmail.com>
> Cc: gdb-patches@sourceware.org
> 
> >> macro1.c:18 <gdb.Macro AMACRO
> >> include_trail=[('/home/ratmice/tests/macro1.h', 1),
> >> ('/home/ratmice/tests/macro1.c', 1)]>
> >> macro1.c:18 <gdb.Macro C include_trail=[('/home/ratmice/tests/macro1.c', 16)]>
> >> macro1.c:18 <gdb.Macro B=1 include_trail=[('/home/ratmice/tests/macro1.c', 13)]>
> > [...]
> >> >  Returns all of the macros which are in effect for the source line
> >> >  given by the @code{gdb.Symtab_and_line}'s @code{line} attribute.
> >>
> >> The problem I was trying to avoid (and which made my documentation for
> >> this method admittedly crappy), is that your rephrased definition
> >> seems to be plausible for the case when the user wants (C) in the
> >> macro1.c:18 case,
> >> e.g. the macros which are used ON the line.  When Symtab_and_line.macros()
> >> outputs all of the macros which were defined before the line, which
> >> are still in effect.
> >
> > Sorry, I don't follow.  Did you mean "B" instead of "C"?  What is the
> > problem you see here with "C"?  It is defined only once, on line 16,
> > and so is in effect on line 18.  I see no issues here.  What am I
> > missing?
> 
> No, I mean "C".
> 
> >> macro1.c:18:  foo(); C; return 0;
> 
> So say we are stopped on this line, and have Symtab_and_line for it.
> the user wants to know `all the macros that are used at this source location'
> In this case the answer would be 'C', and my concern is that 'which
> are in effect'
> can be misconstrued.  e.g. with the above line in isolation, is 'C' in effect?

Now I'm utterly confused.  The output of `macros' shows not just C,
but also AMACRO and B.  All of them are "in effect" in line 18.  So
why are you talking only about C?  The fact that it's actually
mentioned in that line seems irrelevant, because you are documenting
what the `macros' method returns.  What am I missing here?

> how about the following.
> 
> Returns all of the macros defined before the source line given by the
> @code{gdb.Symtab_and_line}'s @code{line} attribute which are in still
> effect.

How is this different from my suggestion above?

> >> >> +@defmethod Symtab macros
> >> >> +Return all of the macros contained in the symbol table.
> >> >> +@end defmethod
> >> >
> >> > Return what, exactly? only their names? something else?
> >>
> >> i'll try 'Return a list of macro objects for all of the macros
> >> contained in the symbol table.'
> >
> > Based on the example above (which I highly recommend to have in the
> > manual), I'd say "a list of macro objects with their values and
> > include trail".
> 
> hrm, except what is above is the output of the string function,
> if you actually print the return value without converting to a string
> it prints something like (<gdb.Macro 0x.......>, <gdb.Macro 0x.....>),

What are the 0x.... numbers here?

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Matt Rice]

On Fri, Aug 26, 2011 at 3:42 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 26 Aug 2011 01:04:21 -0700
>> From: Matt Rice <ratmice@gmail.com>
>> Cc: gdb-patches@sourceware.org
>>
>> >> macro1.c:18 <gdb.Macro AMACRO
>> >> include_trail=[('/home/ratmice/tests/macro1.h', 1),
>> >> ('/home/ratmice/tests/macro1.c', 1)]>
>> >> macro1.c:18 <gdb.Macro C include_trail=[('/home/ratmice/tests/macro1.c', 16)]>
>> >> macro1.c:18 <gdb.Macro B=1 include_trail=[('/home/ratmice/tests/macro1.c', 13)]>
>> > [...]
>> >> >  Returns all of the macros which are in effect for the source line
>> >> >  given by the @code{gdb.Symtab_and_line}'s @code{line} attribute.
>> >>
>> >> The problem I was trying to avoid (and which made my documentation for
>> >> this method admittedly crappy), is that your rephrased definition
>> >> seems to be plausible for the case when the user wants (C) in the
>> >> macro1.c:18 case,
>> >> e.g. the macros which are used ON the line.  When Symtab_and_line.macros()
>> >> outputs all of the macros which were defined before the line, which
>> >> are still in effect.
>> >
>> > Sorry, I don't follow.  Did you mean "B" instead of "C"?  What is the
>> > problem you see here with "C"?  It is defined only once, on line 16,
>> > and so is in effect on line 18.  I see no issues here.  What am I
>> > missing?
>>
>> No, I mean "C".
>>
>> >> macro1.c:18:  foo(); C; return 0;
>>
>> So say we are stopped on this line, and have Symtab_and_line for it.
>> the user wants to know `all the macros that are used at this source location'
>> In this case the answer would be 'C', and my concern is that 'which
>> are in effect'
>> can be misconstrued.  e.g. with the above line in isolation, is 'C' in effect?
>
> Now I'm utterly confused.  The output of `macros' shows not just C,
> but also AMACRO and B.  All of them are "in effect" in line 18.  So
> why are you talking only about C?  The fact that it's actually
> mentioned in that line seems irrelevant, because you are documenting
> what the `macros' method returns.  What am I missing here?

That Symtab_and_line represents that single source line, and if we use
'in effect',
without explicitly qualifying the scope under consideration,
we leave the user to decide under what scope macros are considered to
be in effect.
and the user's idea of that scope may or may not match the actual
scope that the function uses.

e.g. it could be from `0 - line' as it is, or it could be 'macros used
from line - line end'
(which is a function we do not even implement.)


>> how about the following.
>>
>> Returns all of the macros defined before the source line given by the
>> @code{gdb.Symtab_and_line}'s @code{line} attribute which are in still
>> effect.
>
> How is this different from my suggestion above?

It explicitly specifies a scope as 'defined before the source line given...'

>> >> >> +@defmethod Symtab macros
>> >> >> +Return all of the macros contained in the symbol table.
>> >> >> +@end defmethod
>> >> >
>> >> > Return what, exactly? only their names? something else?
>> >>
>> >> i'll try 'Return a list of macro objects for all of the macros
>> >> contained in the symbol table.'
>> >
>> > Based on the example above (which I highly recommend to have in the
>> > manual), I'd say "a list of macro objects with their values and
>> > include trail".
>>
>> hrm, except what is above is the output of the string function,
>> if you actually print the return value without converting to a string
>> it prints something like (<gdb.Macro 0x.......>, <gdb.Macro 0x.....>),
>
> What are the 0x.... numbers here?

The addresses of the python objects.

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Eli Zaretskii]

> Date: Fri, 26 Aug 2011 04:17:24 -0700
> From: Matt Rice <ratmice@gmail.com>
> Cc: gdb-patches@sourceware.org
> 
> That Symtab_and_line represents that single source line, and if we use
> 'in effect',
> without explicitly qualifying the scope under consideration,
> we leave the user to decide under what scope macros are considered to
> be in effect.
> and the user's idea of that scope may or may not match the actual
> scope that the function uses.

I expect the users to know the definition of a macro scope.  Are you
talking about some specific ambiguity, or in general?  If the former,
what is the ambiguity?

> e.g. it could be from `0 - line' as it is, or it could be 'macros used
> from line - line end'
> (which is a function we do not even implement.)

"In effect" means that a macro _can_ be used in this line.  I don't
see any ambiguity here, and it surely isn't the job of the GDB manual
to teach the rules of defining and using macros.

> >> how about the following.
> >>
> >> Returns all of the macros defined before the source line given by the
> >> @code{gdb.Symtab_and_line}'s @code{line} attribute which are in still
> >> effect.
> >
> > How is this different from my suggestion above?
> 
> It explicitly specifies a scope as 'defined before the source line given...'

Which is inaccurate (e.g., what about definitions in other files that
are included by this one?).  Again, it is not our job to document how
macros are defined and used, the reader should already know that,
especially if she is writing a Python script ;-)

> >> >> >> +@defmethod Symtab macros
> >> >> >> +Return all of the macros contained in the symbol table.
> >> >> >> +@end defmethod
> >> >> >
> >> >> > Return what, exactly? only their names? something else?
> >> >>
> >> >> i'll try 'Return a list of macro objects for all of the macros
> >> >> contained in the symbol table.'
> >> >
> >> > Based on the example above (which I highly recommend to have in the
> >> > manual), I'd say "a list of macro objects with their values and
> >> > include trail".
> >>
> >> hrm, except what is above is the output of the string function,
> >> if you actually print the return value without converting to a string
> >> it prints something like (<gdb.Macro 0x.......>, <gdb.Macro 0x.....>),
> >
> > What are the 0x.... numbers here?
> 
> The addresses of the python objects.

Then it should be a "list of macro objects specified by their
addresses".

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Matt Rice]

On Fri, Aug 26, 2011 at 5:07 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 26 Aug 2011 04:17:24 -0700
>> From: Matt Rice <ratmice@gmail.com>
>> Cc: gdb-patches@sourceware.org
>>
>> That Symtab_and_line represents that single source line, and if we use
>> 'in effect',
>> without explicitly qualifying the scope under consideration,
>> we leave the user to decide under what scope macros are considered to
>> be in effect.
>> and the user's idea of that scope may or may not match the actual
>> scope that the function uses.
>
> I expect the users to know the definition of a macro scope.  Are you
> talking about some specific ambiguity, or in general?  If the former,
> what is the ambiguity?
>
>> e.g. it could be from `0 - line' as it is, or it could be 'macros used
>> from line - line end'
>> (which is a function we do not even implement.)
>
> "In effect" means that a macro _can_ be used in this line.  I don't
> see any ambiguity here, and it surely isn't the job of the GDB manual
> to teach the rules of defining and using macros.
>
>> >> how about the following.
>> >>
>> >> Returns all of the macros defined before the source line given by the
>> >> @code{gdb.Symtab_and_line}'s @code{line} attribute which are in still
>> >> effect.
>> >
>> > How is this different from my suggestion above?
>>
>> It explicitly specifies a scope as 'defined before the source line given...'
>
> Which is inaccurate (e.g., what about definitions in other files that
> are included by this one?).  Again, it is not our job to document how
> macros are defined and used, the reader should already know that,
> especially if she is writing a Python script ;-)

at least now I see why you are against adding it...
I think that it's beneath mention that #include causes a macro to be defined.
Anyhow, i don't care enough to argue about it anymore... i'll go with
your original suggestion.

>> >> >> >> +@defmethod Symtab macros
>> >> >> >> +Return all of the macros contained in the symbol table.
>> >> >> >> +@end defmethod
>> >> >> >
>> >> >> > Return what, exactly? only their names? something else?
>> >> >>
>> >> >> i'll try 'Return a list of macro objects for all of the macros
>> >> >> contained in the symbol table.'
>> >> >
>> >> > Based on the example above (which I highly recommend to have in the
>> >> > manual), I'd say "a list of macro objects with their values and
>> >> > include trail".
>> >>
>> >> hrm, except what is above is the output of the string function,
>> >> if you actually print the return value without converting to a string
>> >> it prints something like (<gdb.Macro 0x.......>, <gdb.Macro 0x.....>),
>> >
>> > What are the 0x.... numbers here?
>>
>> The addresses of the python objects.
>
> Then it should be a "list of macro objects specified by their
> addresses".

I don't like this though.... what you're seeing is just a side-effect
of the python print method.
the implementation of a list's str() method to convert to a string
using python's repr()
on it's contents, these are just basic things every python object has,
and will inherit if it doesn't provide a custom implementation.  the
address, and the descriptive string  are things that happen well after
the macros function returns.


(gdb) py print macro_objs
[<gdb.Macro object at 0x7f2c67fc9340>, <gdb.Macro object at 0x7f2c67fc93e8>]
(gdb) py print macro_objs[0]
<gdb.Macro A include_trail=[('/home/ratmice/tests/macro1.c', 7)]>

(gdb) py print repr(macro_objs[0])
<gdb.Macro object at 0x7f2c67fc9340>
(gdb) py print str(macro_objs[0])
<gdb.Macro A include_trail=[('/home/ratmice/tests/macro1.c', 7)]>

(gdb) py help(repr)
repr(...)
    repr(object) -> string

    Return the canonical string representation of the object.
    For most object types, eval(repr(object)) == object.

(gdb) py print help(str)
Help on class str in module __builtin__:

class str(basestring)
 |  str(object) -> string
 |
 |  Return a nice string representation of the object.
 |  If the argument is a string, the return value is the same object.
 |

Re: [PATCH 6/7] [python] API for macros: Add docs.

[Eli Zaretskii]

> Date: Fri, 26 Aug 2011 07:06:40 -0700
> From: Matt Rice <ratmice@gmail.com>
> Cc: gdb-patches@sourceware.org
> 
> > Then it should be a "list of macro objects specified by their
> > addresses".
> 
> I don't like this though.... what you're seeing is just a side-effect
> of the python print method.

Then your proposal, viz.

  Return a list of macro objects for all of the macros contained in
  the symbol table.

is the correct one.

Re: [PATCH 1/7] [python] API for macros: abort or continuing macro iterators.

[Tom Tromey]

>>>>> "matt" == matt rice <ratmice@gmail.com> writes:

matt> 2011-08-23  Matt Rice  <ratmice@gmail.com>
matt> 	* macrocmd.c (print_macro_callback): Return a walk status
matt> 	and continue indefinitely.
matt> 	(print_one_macro): Ditto.
matt> 	* macrotab.c (foreach_macro): Return based on callback
matt> 	status.
matt> 	(foreach_macro_in_scope): Ditto.
matt> 	(macro_for_each): Return a macro walk completion result.
matt> 	(macro_for_each_in_scope): Ditto.
matt> 	* macrotab.h (macro_walk_status): New enum.
matt> 	(macro_walk_result): Ditto.
matt> 	(macro_callback_fn): Return a macro_walk_status.
matt> 	(macro_for_each, macro_for_each_in_scope): Return a macro_walk_result.
matt> 	* symtab.c (add_macro_name): Return a walk status and
matt> 	continue indefinitely.

This one is ok, but please do not commit it until the whole series is
ready.

In the future it would help if each patch had a description that was a
bit longer than just the Subject: line.

Tom

Re: [PATCH 2/7] [python] API for macros: memory management quirks.

[Tom Tromey]

>>>>> "matt" == matt rice <ratmice@gmail.com> writes:

matt> 2011-08-23  Matt Rice  <ratmice@gmail.com
matt> 	* dwarf2read.c (macro_start_file): Update args in call to
matt> 	new_macro_table.
matt> 	* macroscope.c (_initialize_macroscope): Ditto.
matt> 	* macrotab.c (struct macro_table): Remove obstack and bcache,
matt> 	add objfile.
matt> 	(macro_alloc): Replace obstack and bcache usage with those from
matt> 	the objfile.
matt> 	(macro_free, macro_bcache_free, macro_allow_redefinitions): Ditto
matt> 	(macro_bcache, new_macro_table): Ditto, and add assertions.
matt> 	(macro_table_objfile): New function.
matt> 	* macrotab.h: Replace forward declarations of bcache and obstack
matt> 	with objfile.
matt> 	(enum macro_table_type, macro_table_objfile): Add.

A couple minor nits on this one.

matt> +new_macro_table (struct objfile *objfile, enum macro_table_type table_type)

I don't think we need the new enum or the second argument here.
Checking whether OBJFILE==NULL is good enough.

matt> +struct objfile * macro_table_objfile (struct macro_table *table);

No space after the *.

Ok with these changes, contingent on the whole series being ok'd.

Tom

Re: [PATCH 0/7] [python] API for macros

[Phil Muldoon]

matt rice <ratmice@gmail.com> writes:

> take 2 on the python macro API...
> In addition to the testsuite, I tested with the last release of
> gcc following: http://gcc.gnu.org/wiki/DebuggingGCC
>
> using variations of the do_stuff function in following script...
> I didn't save exact timing #'s but it was something like
>  2 mins to count all 600+ million macros
>  5 mins to filter all by name with macro.name().startswith()
>  15 mins to call the str() method (which calls all other methods).

I'm more concerned about having 600 million Python objects lying around
indefinitely ;)  (When a macro is invalidated, the Python object still
has to exist so that the user can still call the APIs (to figure out,
hey this macro is now invalid.))

But I looked at your code, and to be honest, this area of GDB is brand
new to me, so I don't feel very qualified to review it.  I will try.  My
main overall concern is removing the macro cache.  Given that we cannot
count the numbers before your change (well not easily, as there is no
real way to script them), I'm a little bit concerned if the above
numbers are significantly impacted by the removal of the macro cache.

> I'd tried doing a deep copy in the gdb.Macro class,
> to avoid all the objfile/obstack/bcache horse pucky evident in this series,
> but i killed it before it completed when working with gcc...

Given that macros can be extremely prevalent in some projects, I think a
deep copy would not be the way to proceed anyway.

> it's not really timing equivalent to that last 15 minutes case since
> we use lots more memory having a deep copy of all the macros in a symtab in a
> list.  Where the 15 minute version does a deep copy, with only one macro's
> contents in memory at a time.
>
> thus, I think it is useful even for large projects (if used with care.)
> this will fall over if someone has has way too many
> macros in a single symtab.  should that happen we can add a macro_map()
> that behaves sort of like the python map function.

We should add it now, IMO, instead of waiting for it to fail later.  I'm
not sure of the exact requirements for the number of macros in symtab to
qualify for this case, but given how widely used GDB is, it will fail,
sooner or later.


> I think a list is the most straight forward approach for general usage,
> and has been shown to work even with projects that use macros extensively.

You did not note your machine specifications, btw.

> With regards to the hash/compare methods, the implementation of those
> is up for debate, I see at least 3 valid ways to compare them and have only one
> comparison function.  right now I have it compare deeply e.g. including the whole include_trail
> other options that seem valid, compare the name,args,defininition, 
> and compare the name,args,definition and partial include_trail
> (just definition location) since they pretty much are equal before expansion,
> and expansion is an 'expression' thing.

I'd rather correct hash uniqueness over comparable performance here.

> There are some implementation quirks described in the documentation,
> some of these are so in the future we can add a gdb.UserMacro which does
> a deep-copy on initialization, I wasn't going to add that unless someone
> requests it.  Python doesn't seem to have any form of java's `interface',
> or abstract base classing at least within our version range,
> we could also hack up something ugly in the future to avoid this documentation,
> (read union'ify the class implementation,
>  or make gdb.Macro have a pointer to a gdb.ObjfileMacro
>  and do the lambda macro: macro.method() inside gdb.Macro)
> I'd personally just leave it there to give us future choice on the matter.
> we can always remove that from the docs if we implement it in a way


I really think we ought to do this now, not what you wanted to hear, I
know.  But I think it would be genuinely useful.  No hacks, though! ;)

Cheers,

Phil

Re: [PATCH 1/7] [python] API for macros: abort or continuing macro iterators.

[Phil Muldoon]

matt rice <ratmice@gmail.com> writes:

> diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
> index d1ac7fa..e929fe0 100644
> --- a/gdb/macrocmd.c
> +++ b/gdb/macrocmd.c
> @@ -220,13 +220,15 @@ info_macro_command (char *name, int from_tty)
>     Otherwise USER_DATA is considered to be a string, printing
>     only macros who's NAME matches USER_DATA.  Other arguments are
>     routed to print_macro_definition.  */
> -static void
> +static enum macro_walk_status
>  print_macro_callback (const char *name, const struct macro_definition *macro,
>  		   struct macro_source_file *source, int line,
>  		   void *user_data)
>  {
>    if (! user_data || strcmp (user_data, name) == 0)
>      print_macro_definition (name, macro, source, line);
> +
> +  return macro_walk_continue;
>  }

Is the unconditional return here due to it being part of a recursive
call chain? I'm not sure what value the unconditional return of an enum
constant over the previous "void" gets us? Why return anything at all?
The previous version assumed the callers knew how to deal with the void
return, but I am not sure what your version achieves by unconditionally
returning macro_walk_continue?

  
>  /* Implementation of the "info definitions" command. */
> @@ -435,7 +437,7 @@ macro_undef_command (char *exp, int from_tty)
>  }
>  
>  
> -static void
> +static enum macro_walk_status
>  print_one_macro (const char *name, const struct macro_definition *macro,
>  		 struct macro_source_file *source, int line,
>  		 void *ignore)
> @@ -452,6 +454,7 @@ print_one_macro (const char *name, const struct macro_definition *macro,
>        fprintf_filtered (gdb_stdout, ")");
>      }
>    fprintf_filtered (gdb_stdout, " %s\n", macro->replacement);
> +  return macro_walk_continue;
>  }

Same as above. If you decide to keep them please update the comment
headers with a description of the return value.  This function,  and
numerous other places too.
  

Re: [PATCH 2/7] [python] API for macros: memory management quirks.

[Phil Muldoon]

matt rice <ratmice@gmail.com> writes:

> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index f66c1d9..c56f431 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -14351,8 +14351,7 @@ macro_start_file (int file, int line,
>    /* We don't create a macro table for this compilation unit
>       at all until we actually get a filename.  */
>    if (! pending_macros)
> -    pending_macros = new_macro_table (&objfile->objfile_obstack,
> -                                      objfile->macro_cache);
> +    pending_macros = new_macro_table (objfile, macro_table_type_from_objfile);

The comments in macrotab.h need to be updated with the new parameter
info.  This and others.

This is code I am not very sure of, having never dabbled in this area.
So this review would benefit from a maintainer that is more familiar
with this code.  But what benefit do we accrue now that we purely store
macros in the objfile's obstack over (the now defunct) bcache?  Was
there an underlying reason for dual this you discovered?  It seems odd that
in the pre-patched version that struct macro_table had two strategies
for this. Contextually, why did you elect to use the obstack in the
objfile, instead of the previous ancillary related structure?

> diff --git a/gdb/macrotab.c b/gdb/macrotab.c
> index efcf835..b2825d2 100644
> --- a/gdb/macrotab.c
> +++ b/gdb/macrotab.c
> @@ -35,13 +35,10 @@
>  
>  struct macro_table
>  {
> -  /* The obstack this table's data should be allocated in, or zero if
> -     we should use xmalloc.  */
> -  struct obstack *obstack;
> -
> -  /* The bcache we should use to hold macro names, argument names, and
> -     definitions, or zero if we should use xmalloc.  */
> -  struct bcache *bcache;
> +  /* The objfile who's obstack this table's data should be allocated in,
> +     and bcache we should use to hold macro names, argument
> +     names, and definitions, or zero if we should use xmalloc. */
> +  struct objfile *objfile;


Comment seems wrong, as bcache data structure no longer exists.
Additionally, (not in the code, but in your email), a narrative why we
are removing the bcache is needed.  
  


>  static void
>  macro_bcache_free (struct macro_table *t, void *obj)
>  {
> -  if (t->bcache)
> +  if (t->objfile)
>      /* There are cases where we need to remove entries from a macro
>         table, even when reading debugging information.  This should be
>         rare, and there's no easy way to free data from a bcache, so we
> @@ -147,6 +146,12 @@ macro_bcache_free (struct macro_table *t, void *obj)
>      xfree (obj);
>  }

Updated comments needed.  Referral to bcache.
  
> +/* Create a new, empty macro table.  Allocate it in OBJFILE's obstack,
> +   or use xmalloc if OBJFILE is zero.  Use OBJFILE's bcache to store
> +   all macro names, arguments, definitions, and anything else that
> +   might be the same amongst compilation units in an executable file;
> +   if OBJFILE is zero, don't cache these things.
> +
> +   Note that, if OBJFILE is non-zero, then removing information from the
> +   table may leak memory.  Neither obstacks nor bcaches really allow
> +   you to remove information, so although we can update the data
> +   structure to record the change, we can't free the old data.
> +   At the moment, since we only provide obstacks and bcaches for macro
> +   tables for symtabs, this isn't a problem; only odd debugging
> +   information makes a definition and then deletes it at the same
> +   source location (although 'gcc -DFOO -UFOO -DFOO=2' does do that
> +   in GCC 4.1.2.).  */
> +struct macro_table *new_macro_table (struct objfile *objfile,
> +				     enum macro_table_type table_type);

Comments need to be updated.
  
Cheers,

Phil

Re: [PATCH 3/7] [python] API for macros: Add gdb.Macro class.

[Phil Muldoon]

matt rice <ratmice@gmail.com> writes:

> +#include "defs.h"
> +#include "python-internal.h"
> +#include "py-macro.h"
> +#include "macrotab.h"
> +#include "bcache.h"
> +#include "objfiles.h"
> +
> +/* The macro_object_validator type has the ability to detect when an object
> +   has been invalidated and nothing more.  The more featured validation
> +   methods of pyst, pysal, provide no benefit for macros because once
> +   we have been invalidated, we have no way to know what macro the object
> +   once represented.  */
> +typedef struct
> +{
> +  PyObject_HEAD;
> +  int is_valid;
> +  struct objfile *objfile;
> +} macro_validator_object;

^^ Comments on each member, please.

> +typedef struct
> +{
> +  PyObject_HEAD;
> +
> +  /* A macro definition object representing this macro.  */
> +  const struct macro_definition *def;
> +  /* The name of the macro */
> +  const char *name;
> +  /* The file where the macro resides and its include trail.  */
> +  struct macro_source_file *src;
> +  /* the line location in the 'SRC' file.  */

Capital 'T'.

> +  int line;
> +  /* This is used to tell us if the macro has been invalidated

Missing period.

> +     The objfile who's obstack and bcache the mdef, and src, and name
> +     fields reside in is stored here.  Those fields may be dangling
> +     pointers if the objfile is NULL.
> +     An invalid macro cannot become valid once again.  */
> +  macro_validator_object *objfile_validator;
> +} macro_object;

I thought you removed the bcache?

> +macropy__definition_to_py (const struct macro_definition *macro)

A small nit for me personally, but all of the other Python API classes
use one "_" to separate the function library name from the function
name.  This and others.


> +macropy_object_create (struct objfile *objfile,
> +		       const char *name,
> +		       const struct macro_definition *def,
> +		       struct macro_source_file *src,
> +		       int line)
> +{
> +  macro_object *macro_obj;
> +
> +  macro_obj = PyObject_New (macro_object, &macro_object_type);
> +
> +  if (macro_obj)
> +    {
> +      macro_validator_object *validator;
> +
> +      macro_obj->objfile_validator = NULL;
> +
> +      /* check our arguments, Don't check line because 0 is a valid line.  */

Capital "C". "." instead of ",".


> +macropy__validate_self (PyObject *self)
> +{
> +  macro_object *me;
> +
> +  if (! PyObject_TypeCheck (self, &macro_object_type))
> +    {
> +      PyErr_SetString (PyExc_RuntimeError,
> +			 _("Typecheck failure converting macro."));
> +      return NULL;
> +    }
> +
> +  me = (macro_object *) self;
> +
> +  if (me->objfile_validator->is_valid == 0)
> +    {
> +      PyErr_SetString (PyExc_RuntimeError, _("Macro is invalid."));
> +      return NULL;
> +    }
> +
> +  return me;
> +}

As you are pointing to an existing copy of self, and returning it, you
*may* need to incref.  It depends if any of the functions downstream
decref it.  Please check.

> +static int
> +macropy_compare (PyObject *self, PyObject *o2)
> +{
> +  PyObject *my_str = macropy_str (self);
> +  PyObject *other_str = NULL;
> +  PyObject *an_error;
> +  int err_code = -1;
> +  int comparison_result = -1;
> +
> +  if (!my_str)
> +    goto check_for_errors_and_return;
> +
> +  if (PyObject_TypeCheck (o2, &macro_object_type))
> +    {
> +      other_str = macropy_str (o2);
> +      if (!other_str)
> +	goto check_for_errors_and_return;
> +
> +      err_code = PyObject_Cmp (my_str, other_str, &comparison_result);
> +    }

Use PyObject_Compare

> +  else
> +    {
> +      err_code = PyObject_Cmp (my_str, o2, &comparison_result);
> +    }

Ditto.

> +  check_for_errors_and_return:
> +    Py_XDECREF (my_str);
> +    Py_XDECREF (other_str);
> +
> +    /* Because -1 is also Less Than we cannot use gdbpy_print_stack ()
> +       which clears the error if set python print-stack is off.
> +       Which would lead to (gdb) python print x == x
> +       returning False with no error message displayed.
> +
> +       alternately if an error is set and the return value is not 1,
> +       python will complain.
> +
> +       Thus if there is an error, we must normalize it
> +       making sure that both an error and the return
> +       value are -1. Should we encounter one.  */
> +    an_error = PyErr_Occurred ();
> +    if (an_error == NULL && err_code != -1)
> +      {
> +        /* a valid comparison */
> +        return comparison_result;
> +      }
> +    else if (an_error == NULL && err_code == -1)
> +      {
> +	/* an unknown error */
> +	PyErr_SetString (PyExc_RuntimeError,
> +			 _("mystery error during macro comparison."));


Don't overwrite the exception here.  And if PyObject_Compare has
returned -1, there will be an exception so this can't happen.  You could
probably just remove the condition entirely and just fold over to the
else below.

> +        return -1;
> +      }
> +    else /* a known error */
> +      return -1;
> +}
> +


> +static long
> +macropy_hash (PyObject *o)
> +{
> +  long result = -1;
> +  PyObject *my_str = macropy_str (o);
> +
> +  if (my_str)
> +    result = PyObject_Hash (my_str);
> +
> +  /* The docs say PyObject_Hash should return -1 on error,
> +     Don't believe it because it is not always true,
> +     The exception gets raised regardless of return value,
> +     But we should follow the documented return strategy of -1 on error.

Even if you are right, are you right for all versions of Python?  I
guess the PyErr_Occurred check is harmless.

> +static PyMethodDef macro_object_methods[] = {
> +  { "argv", macropy_argv_method, METH_NOARGS,
> +    "argv () -> List.\n\
> +Return a list containing the names of the arguments for a function like \
> +macro." },

"function-like", imo.

> +  { "definition", macropy_definition_method, METH_NOARGS,
> +    "definition () -> String.\n\
> +Return the macro's definition." },
> +  { "include_trail", macropy_include_trail_method, METH_NOARGS,
> +    "include_trail () -> List.\n\
> +Return a list containing tuples with the filename and line number describing \
> +how and where this macro came to be defined." },

Your help and your return explanation don't match.  It should return a
Tuple (if it doesn't, please fix it).  And please change  -> List to ->
Tuple.


> +  struct objfile *objfile;
> +};
> +
> +/* A macro_callback_fn for use with macro_foreach and friends that adds
> +   a gdb.Macro object to the python list USER_DATA->LIST.
> +   USER_DATA should be of type struct macropy_user_data.
> +
> +   A null USER_DATA->OBJFILE is currently be considered an error,
> +   this is subject to change.
> +
> +   Aborts the iteration on error, and forces the macro iterator function
> +   to return macro_walk_aborted check the iterators macro_walk_result.
> +   On error it is the callers responsibility to dispose of the USER_DATA
> +   structure and its contents.  The appropriate python exception
> +   that caused the error has already been set.
> +*/
> +enum macro_walk_status
> +pymacro_add_macro_to_list (const char *name,
> +		   const struct macro_definition *definition,
> +		   struct macro_source_file *source,
> +		   int line,
> +		   void *user_data);
> +
> +/* Create a new macro (gdb.Macro) object that encapsulates
> +   the gdb macro representation.  OBJFILE is the objfile that contains
> +   the macro table.  NAME is the name of the macro.  DEF the macros definition
> +   SRC the source file containing the macro. LINE definitions location.  */
> +PyObject *
> +macropy_object_create (struct objfile *objfile,
> +		       const char *name,
> +		       const struct macro_definition *def,
> +		       struct macro_source_file *src,
> +		       int line);
> +
> +#endif /* GDB_PY_MACRO_H */

Any reason why these need their own include?  (over python-internal.h).
If these are called from anywhere other than python/* then they need to
go into python-internal.h to protect versions of GDB that do not have
Python scripting enabled.  (python.h conditionalizes this).


Cheers,

Phil

Re: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Phil Muldoon]

matt rice <ratmice@gmail.com> writes:

> +{
> +  struct symtab *st = symtab_object_to_symtab (self);

STPY_REQUIRE_VALID will do this for you.

>  
> @@ -242,6 +274,75 @@ salpy_is_valid (PyObject *self, PyObject *args)
>    Py_RETURN_TRUE;
>  }
>  
> +static PyObject *
> +salpy_macros (PyObject *self, PyObject *args)
> +{
> +  struct symtab_and_line *sal;
> +  PyObject *result;
> +  enum macro_walk_result walk_result;
> +  struct macropy_user_data mud;
> +  struct macro_scope *ms;
> +
> +  SALPY_REQUIRE_VALID (self, sal);
> +
> +  ms = sal_macro_scope (*sal);

This can return NULL.

> +  result = PyList_New (0);
> +  if (result == NULL)
> +    return NULL;
> +
> +  mud.list = result;
> +  mud.objfile = macro_table_objfile (ms->file->table);
> +  walk_result = macro_for_each_in_scope (ms->file, ms->line,
> +					 pymacro_add_macro_to_list, &mud);

So you need to check ms before you call this ^^ code.


> @@ -477,6 +578,9 @@ Return true if this symbol table is valid, false if not." },
>    { "fullname", stpy_fullname, METH_NOARGS,
>      "fullname () -> String.\n\
>  Return the symtab's full source filename." },
> +  { "macros", stpy_macros, METH_NOARGS,
> +    "macros () -> List.\n\
> +Return a list of macros in the symtab." },

I think this should return a Tuple.  Tuples are immutable, and unless
you for see a use for the user to manipulate the List, we should guard
against it.  If you agree, please alter stpy_macros too.


> +macro cannot be found." },
> +  { "macros", salpy_macros, METH_NOARGS,
> +    "macros () -> List.\n\
> +Return all the macros which are available at the symbol table and line \
> +object's location." },

See above.

Re: [PATCH 5/7] [python] API for macros: gdb.Objfile symtabs method.

[Phil Muldoon]

matt rice <ratmice@gmail.com> writes:

> +  list = PyList_New (0);
> +  if (!list)
> +    return NULL;
> +
> +  symtabs = obj->objfile->symtabs;
> +  while(symtabs)
> +    {
> +      py_symtab = symtab_to_symtab_object (symtabs);
> +      if (! py_symtab)
> +        goto fail;
> +
> +      if (PyList_Append (list, py_symtab) != 0)
> +        goto fail;
> +
> +      Py_DECREF (py_symtab);
> +
> +      symtabs = symtabs->next;
> +    }
> +  return list;

If there are no symtabs, why return an empty list?  Would Py_None make
more sense here.  And same rules apply to returning a Tuple, too, as
others.

> +  { "symtabs", objfpy_symtabs, METH_NOARGS,
> +    "symtabs () -> List.\n\
> +A List containing the object file's valid symtabs." },

See above.
  
> --- /dev/null
> +++ b/gdb/python/py-symtab.h
> @@ -0,0 +1,26 @@
> +/* Python interface to Symtabs and Symtab_and_line's.
> +
> +   Copyright (C) 2011 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GDB_PY_SYMTAB_H
> +#define GDB_PY_SYMTAB_H
> +
> +PyObject *
> +symtab_to_symtab_object (struct symtab *symtab);
> +
> +#endif /* GDB_PY_SYMTAB_H */

This is already exported in python-internal.h? Why do we need to
re-export it here?

Cheers,

Phil

Re: [PATCH 7/7] [python] API for macros: Add tests.

[Phil Muldoon]

matt rice <ratmice@gmail.com> writes:

I have missed it, but I did not see a statement regarding any
regressions in the suite, and the arches tested.

Cheers,

Phil

Re: [PATCH 7/7] [python] API for macros: Add tests.

[Tom Tromey]

>>>>> "matt" == matt rice <ratmice@gmail.com> writes:

matt> 2011-08-23  Matt Rice  <ratmice@gmail.com>
matt> 	PR python/10807
matt> 	* gdb.python/py-macro.exp: Ditto.
matt> 	* gdb.python/py-macro.c: New file.
matt> +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} $options] } {
matt> +    untested ${testfile}.exp
matt> +    return -1
matt> +}
matt> +
matt> +# Start with a fresh gdb.
matt> +gdb_exit
matt> +gdb_start
matt> +gdb_reinitialize_dir $srcdir/$subdir
matt> +gdb_load ${binfile}

This stanza isn't needed -- prepare_for_testing calls clean_restart,
which does all this.

Tom

Re: [PATCH 5/7] [python] API for macros: gdb.Objfile symtabs method.

[Tom Tromey]

>>>>> "matt" == matt rice <ratmice@gmail.com> writes:

matt> 2011-08-23  Matt Rice  <ratmice@gmail.com>
matt> 	* python/py-symtab.h: New file.  Make symtab_to_symtab_object public.
matt> 	* python/py-objfile.c (objfpy_symtabs): New method.
matt> 	(objfile_object_methods): Ditto.

A couple notes on this.

matt> +static PyObject *
matt> +objfpy_symtabs (PyObject *self, PyObject *ignore)
matt> +{
matt> +  objfile_object *obj = (objfile_object *) self;
matt> +  struct symtab *symtabs;
matt> +  PyObject *list;
matt> +  PyObject *py_symtab;
matt> +
matt> +  if (! obj->objfile)
matt> +    return Py_None;

You need Py_RETURN_NONE.

I think most of our objects will throw an exception if you try to
operate on them when they are in the 'invalid' state; if that is true
then I think this should conform.

matt> +  while(symtabs)

Missing space.

matt> +  { "symtabs", objfpy_symtabs, METH_NOARGS,
matt> +    "symtabs () -> List.\n\
matt> +A List containing the object file's valid symtabs." },

I don't mind this method per se, but I think people will find it
confusing, as it only returns full symtabs -- but there is no direct way
for the Python programmer to cause symtab expansion.

matt> +PyObject *
matt> +symtab_to_symtab_object (struct symtab *symtab);

In addition to what Phil said, the norm is not to split declarations
after the type -- only definitions.

Tom

Re: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Tom Tromey]

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

Phil> I think this should return a Tuple.  Tuples are immutable, and unless
Phil> you for see a use for the user to manipulate the List, we should guard
Phil> against it.  If you agree, please alter stpy_macros too.

Using a tuple means you have to iterate twice -- once to count the
objects and once to make the tuple.

Since we make a new object every time this method is called, returning a
list doesn't seem any worse than a tuple.

Tom

Re: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> I think this should return a Tuple.  Tuples are immutable, and unless
> Phil> you for see a use for the user to manipulate the List, we should guard
> Phil> against it.  If you agree, please alter stpy_macros too.
>
> Using a tuple means you have to iterate twice -- once to count the
> objects and once to make the tuple.

You can use a list, and then convert it to a tuple:

PyList_AsTuple

We already use the above.

If you wanted to, you could use PyTuple_Ruse, as long as there is 
one reference to the tuple.  But building as a list, and returning as a
tuple seems reasonable to me.

Cheers

Phil

Re: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Phil Muldoon]

Phil Muldoon <pmuldoon@redhat.com> writes:

> Tom Tromey <tromey@redhat.com> writes:
>
>>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>>
>> Phil> I think this should return a Tuple.  Tuples are immutable, and unless
>> Phil> you for see a use for the user to manipulate the List, we should guard
>> Phil> against it.  If you agree, please alter stpy_macros too.
>>
>> Using a tuple means you have to iterate twice -- once to count the
>> objects and once to make the tuple.
>
> You can use a list, and then convert it to a tuple:
>
> PyList_AsTuple
>
> We already use the above.
>
> If you wanted to, you could use PyTuple_Ruse, as long as there is 
> one reference to the tuple.  

Apologies for the typos, but the above should read:

 If you wanted too, you could use PyTuple_Resize, as long as there is 
 only one reference to the tuple.  

Re: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Tom Tromey]

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

Phil> You can use a list, and then convert it to a tuple:
Phil> PyList_AsTuple

Sure, but that just adds another allocation.
I don't understand what the benefit is.

Tom

Re: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> You can use a list, and then convert it to a tuple:
> Phil> PyList_AsTuple
>
> Sure, but that just adds another allocation.
> I don't understand what the benefit is.

There are loads of articles discussing around the old adage: "Tuples are
heterogeneous data structures, lists are homogeneous sequences" yadda
ya.  I don't really mind that distinction, and, I am not strongly swayed
one way or the other whether we should return a tuple or a list.  My
only distinction is that as a tuple is immutable, n'th element will
always be n'th element, and so can used a key to a dictionary.

So I don't want to bike-shed this, I always return a tuple where the
data won't change.  Once we return a list/tuple of macros, that
list/tuple won't change ever. To get a changed list of data, we would
have to make another call to the API.  To me, that is a constant list,
and should be a tuple.  That's the rule I use.  And it sends a
message as part of the API, "this is the data that I have now, it won't
change unless you call me again".

That being said, if the performance/memory usage question is relevant,
then yeah, converting a list to a tuple probably should be avoided.

Cheers,

Phil

Re: [PATCH 0/7] [python] API for macros

[Matt Rice]

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

On Tue, Aug 30, 2011 at 2:32 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> matt rice <ratmice@gmail.com> writes:
>
>> take 2 on the python macro API...
>> In addition to the testsuite, I tested with the last release of
>> gcc following: http://gcc.gnu.org/wiki/DebuggingGCC
>>
>> using variations of the do_stuff function in following script...
>> I didn't save exact timing #'s but it was something like
>>  2 mins to count all 600+ million macros
>>  5 mins to filter all by name with macro.name().startswith()
>>  15 mins to call the str() method (which calls all other methods).
>
> I'm more concerned about having 600 million Python objects lying around
> indefinitely ;)  (When a macro is invalidated, the Python object still
> has to exist so that the user can still call the APIs (to figure out,
> hey this macro is now invalid.))

yeah... I can't store 600 million valid or invalid macros in memory...
that's about 31.2 gigs of macro objects for x86_64. macro_object = 56 bytes..
(16 for python object overhead + 36 bytes for macro stuff + 4 for
structure alignment)
that's *not* including any of the strings for the actual macro
contents, just the top-level structure..
It can possibly be shrunk by 8 bytes

thus in my scripts at least using gcc as a test case, they have to
iterate over each symtab's macros, and let python deallocate a good
portion of the macros...
e.g. in the adding all TREE_* macros to `a_set', I hold on to a total
of about 300k macros or 16 megs or so...

> But I looked at your code, and to be honest, this area of GDB is brand
> new to me, so I don't feel very qualified to review it.  I will try.  My
> main overall concern is removing the macro cache.

I actually didn't remove the macro cache, it's still there, we replaced it
with a pointer to the Objfile, e.g. new_macro_table(objfile)
where we previously had new_macro_table(&objfile->obstack,
objfile->macro_cache);

this is because we rely on being able to do the objfile->data free callbacks
for macro invalidation, because we don't want people creating
objfile-less macros with bcaches and obstacks.

>  Given that we cannot
> count the numbers before your change (well not easily, as there is no
> real way to script them), I'm a little bit concerned if the above
> numbers are significantly impacted by the removal of the macro cache.

see above about the macro cache.

>> I'd tried doing a deep copy in the gdb.Macro class,
>> to avoid all the objfile/obstack/bcache horse pucky evident in this series,
>> but i killed it before it completed when working with gcc...
>
> Given that macros can be extremely prevalent in some projects, I think a
> deep copy would not be the way to proceed anyway.

yep... it was ignorant hope, because that's the `cleanest` way to get
a macro object implementation
that works with both 'user-defined' and 'from-debug info' macros

>> it's not really timing equivalent to that last 15 minutes case since
>> we use lots more memory having a deep copy of all the macros in a symtab in a
>> list.  Where the 15 minute version does a deep copy, with only one macro's
>> contents in memory at a time.
>>
>> thus, I think it is useful even for large projects (if used with care.)
>> this will fall over if someone has has way too many
>> macros in a single symtab.  should that happen we can add a macro_map()
>> that behaves sort of like the python map function.
>
> We should add it now, IMO, instead of waiting for it to fail later.  I'm
> not sure of the exact requirements for the number of macros in symtab to
> qualify for this case, but given how widely used GDB is, it will fail,
> sooner or later.

k, I'll probably add this as maybe macros(filter=some_filter_func)
(None by default), where if filter returns None, nothing gets appended
to the list/tuple/to be determined
and if it returns an object that object gets added to the
list/tuple/to be determined

after thinking about this some more it needs to be done with extreme care,
to avoid the possibility of the user causing any lookups while iterating.

As well as not being reentrant, the iterators can be foiled by any
function which causes a macro lookup in the same table... the current
returning a complete list approach doesn't suffer from this issue.

even if we can avoid this on gdb.Macro methods, if 'some_filter_func'
calls macros(some_other_filter_func), or gdb.execute ("info macro"),
or expand a macro,
those will mess up the initial call to macro_foreach*, causing either
incorrect results or possibly an infinite loop.  because the nature of
a splay tree lookup causes modifications to the splay tree itself.

>> I think a list is the most straight forward approach for general usage,
>> and has been shown to work even with projects that use macros extensively.
>
> You did not note your machine specifications, btw.

amd64 phenom II 3ghz (3 cores... but its single threaded so that doesn't matter)
4G of cheap ram, nothing spectacular

>> With regards to the hash/compare methods, the implementation of those
>> is up for debate, I see at least 3 valid ways to compare them and have only one
>> comparison function.  right now I have it compare deeply e.g. including the whole include_trail
>> other options that seem valid, compare the name,args,defininition,
>> and compare the name,args,definition and partial include_trail
>> (just definition location) since they pretty much are equal before expansion,
>> and expansion is an 'expression' thing.
>
> I'd rather correct hash uniqueness over comparable performance here.

Ahh... the problem is 'correct hash uniqueness' is situation dependent,
The above have little to do with performance,

I've attached a script which shows the only way I could figure out to
do a custom
macro comparison with the current gdb.Macro nomacros.c is just a file
with a single function named 'gimmie_a_text_section'
no #includes, no macros defined.

because the compiler itself defines a bunch of macros, I use a file
with no #includes and no macros a baseline,
 and put the macros for 'nomacros.c' into the baseline_macros set.
then subtract that set from the current list of macros,  getting only
the actual #included and #define'd macros... including the
include_trail in this causes those macros to compare as !=.

thus the script needs to provide a comparison that only compares macro
name/args/contents.
I wasn't able to figure out how to a) subclass gdb.Macro, or b) monkey
patch the gdb.Macro comparison function

>> There are some implementation quirks described in the documentation,
>> some of these are so in the future we can add a gdb.UserMacro which does
>> a deep-copy on initialization, I wasn't going to add that unless someone
>> requests it.  Python doesn't seem to have any form of java's `interface',
>> or abstract base classing at least within our version range,
>> we could also hack up something ugly in the future to avoid this documentation,
>> (read union'ify the class implementation,
>>  or make gdb.Macro have a pointer to a gdb.ObjfileMacro
>>  and do the lambda macro: macro.method() inside gdb.Macro)
>> I'd personally just leave it there to give us future choice on the matter.
>> we can always remove that from the docs if we implement it in a way
>
>
> I really think we ought to do this now, not what you wanted to hear, I
> know.  But I think it would be genuinely useful.  No hacks, though! ;)

hrm, I'll have to think about how to do it then, as quickly thinking
about it i discovered
i'd only considered user-defined macros from the 'how user-defined
macros affect the from-debuginfo macros API'

I don't think there is a way to do it with the 'No Hacks'
qualification, without rewriting how gdb/macrotab does stuff.

again the choice currently is:
a) make user defined and from-debuginfo objects 2 different kinds of
objects with seperate implementations.
and document them.

b) do a) but add an interim object, to rid ourselves of the python api
limitations of a)

struct macro_object {
  PyObject_HEAD
  PyObject *user_defined_or_debuginfo_macro_object;
};

this adds (16 + 8 = 24 bytes), to each macro, 8 bytes short of 1/2 the
size of the current macro_object.

c) unionize the implementation, and deal with the consequences in the
macro_object methods.
struct macro_object {
    union user_defined_or_from_debuginfo_macro_object_contents *macro;
    int macro_kind:1;
};

I'm not yet sure the affect of this on the size of 'from-debuginfo'
macro objects yet.
but from the code side, it means we to dual implement each method
based on 'kind'.

d) fix macrotab.c so we can use the same API for user-defined and
from-debuginfo macros.
(this then affects all macrotab usage everywhere, and may not really
be possible)...

---

the latter option here is obviously the way to go, it's also the
largest and most intrusive of the options.
In my current patch, I've left all these options open, documented 'a)'
as a possibility,
but nowhere do we exercise this so we aren't stuck with this API
limitation forever (in the python script writing side).

a) is an option i'd prefer to avoid at all costs, but has the least
effect on the gdb implementation side.
b) is the cleanest to implement but I hate to increase the size of
each macro object by that much. e.g. our 31G of 600 mil macros becomes
44.7
c) is going to be ugly in the python macro object implementation,
and may/may not also increase the macro object size, the `ugly' bit
has kept me from figuring out the
affect on macro_object structure size..  i'd hazard guess that we'd
break even or add a pointer, or fill up the unused space used for
structure alignment...
d) is just going to be a large project...

i'd prefer to avoid a, b, and c and the only way to avoid it is to
implement d...
I'll probably spend a little time seeing if we can't implement c, in a
moderately acceptable way,
if it proves possible to get a smaller 'from-debuginfo' macro object
than with b)

---
more about d)....

the problem with d) is figuring out a way to do macro invalidation on
user-defined macros,
gdb.Macro does validation on a per-macro table basis, all macros in
the same macro table are invalidated at the same time.

with user-defined macros any macro can be invalidated at any time.
currently there's no code in place to do invalidation of user-defined
macros, we don't have anything like the objfile->data free
callback....
thus, the short-term plan was to do a deep copy, this keeps the macro
from being xfree'd...
to check if it is valid, do a lookup of the macro by its name and see
if the macro and the looked up macro compare equally.  == is a valid
macro != is an invalid.

user defined macros also need additional API for
defining/redefining/undefining, where 'from-debuginfo' macros do
not... I imagine these will not be methods of a gdb.Macro object
though.

anyhow... figuring out the best way to unify (or if the best way is to
not unify) these two similar but different things from the python API
perspective is something i'd really hoped to avoid... not because it
requires work, but because I don't want to set in stone any
limitations which we may be able to get rid of in the future, but
still be stuck with from a python perspective.

the very best we can hope to achieve is a gdb.Macro object which is
the same for user-defined and from-debuginfo macros, and seperate
implementations of gdb.MacroValidator and
gdb.UserDefinedMacroValidator.  so i need to figure out the best way
to achieve this in macrotab.c without adversely affecting
from-debuginfo macros.

sorry for the length.

[-- Attachment #2: macro.gdb --]
[-- Type: application/octet-stream, Size: 2336 bytes --]

set confirm off
set pagination off
python
class MyMacro:
  "macro with custom comparison... "
  def __init__(self, macro):
     self.macro = macro;
     return None 
  def __cmp__(self, other):
      str1 = ""
      str1 += str(self.argv())
      str1 += self.name()
      str1 += self.definition()

      if other.argv and other.name and other.definition:
        str2 = ""
        str2 += str(other.argv())
        str2 += other.name()
        str2 += other.definition()
        return cmp(str1, str2)
      return cmp(str1, str)
      
  def __hash__(self):
    return hash(str(self.macro.argv()) + self.macro.name() + self.macro.definition())
  def argv(self):
    return self.macro.argv()
  def name(self):
    return self.macro.name()
  def definition(self):
    return self.macro.definition()
  def is_function_like(self):
    return self.macro.is_function_like()
  def include_trail():
    return self.macro.include_trail()
  def __str__(self):
    return str(self.macro)

def my_macros(macros):
  if macros != None:
    return map(lambda m: MyMacro(m), macros)
  else:
    return list()

# set up a baseline for an empty c file that defines no macros.
baseline = None
last_set = None

# The compiler in general has decide to define a bunch of macros.
# here we use an empty c file with no includes, or #defines
# and use it as a baseline this gives us the macros we are interested in.
def baseline_macros():
  global baseline;
  if baseline != None:
    return baseline
  else: 
    for i in gdb.objfiles():
      for s in i.symtabs():
        if s.filename.find("nomacros.c") >= 0:
          baseline = set(my_macros(s.macros()))
    return baseline


def here_macros():
  global last_set
  tmp = last_set
  sal = gdb.decode_line("*$pc")[1][0];
  macros = set(my_macros(sal.macros())) - baseline_macros()
  for m in macros:
    print sal.symtab.filename + ":" + str(sal.line) + " " + str(m)

def all_here_macros():
  macros = my_macros(gdb.decode_line("*$pc")[1][0].symtab.macros())
  for m in set(macros) - baseline_macros():
    print "all: " + str(m)
end

set breakpoint pending on
break main:bp1
commands
py here_macros();
py all_here_macros()
end
break main:bp2
commands
py here_macros();
py all_here_macros()
end
break main:bp3
commands
py here_macros()
py all_here_macros()
end
info line *&gimmie_a_text_section
start

Re: [PATCH 1/7] [python] API for macros: abort or continuing macro iterators.

[Matt Rice]

On Tue, Aug 30, 2011 at 4:09 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> matt rice <ratmice@gmail.com> writes:
>
>> diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
>> index d1ac7fa..e929fe0 100644
>> --- a/gdb/macrocmd.c
>> +++ b/gdb/macrocmd.c
>> @@ -220,13 +220,15 @@ info_macro_command (char *name, int from_tty)
>>     Otherwise USER_DATA is considered to be a string, printing
>>     only macros who's NAME matches USER_DATA.  Other arguments are
>>     routed to print_macro_definition.  */
>> -static void
>> +static enum macro_walk_status
>>  print_macro_callback (const char *name, const struct macro_definition *macro,
>>                  struct macro_source_file *source, int line,
>>                  void *user_data)
>>  {
>>    if (! user_data || strcmp (user_data, name) == 0)
>>      print_macro_definition (name, macro, source, line);
>> +
>> +  return macro_walk_continue;
>>  }
>
> Is the unconditional return here due to it being part of a recursive
> call chain? I'm not sure what value the unconditional return of an enum
> constant over the previous "void" gets us? Why return anything at all?
> The previous version assumed the callers knew how to deal with the void
> return, but I am not sure what your version achieves by unconditionally
> returning macro_walk_continue?

This is just matching pre-existing behavior from before macro
iteration could abort/continue.
the 'void' return was hard coded as 'return 0', in foreach_macro and
foreach_macro_in_scope
where now they return 'status == macro_walk_abort'

where 0 means continue and != 0 abort as interpreted by
src/libiberty/splay-tree.c:splay_tree_foreach_helper

>>  /* Implementation of the "info definitions" command. */
>> @@ -435,7 +437,7 @@ macro_undef_command (char *exp, int from_tty)
>>  }
>>
>>
>> -static void
>> +static enum macro_walk_status
>>  print_one_macro (const char *name, const struct macro_definition *macro,
>>                struct macro_source_file *source, int line,
>>                void *ignore)
>> @@ -452,6 +454,7 @@ print_one_macro (const char *name, const struct macro_definition *macro,
>>        fprintf_filtered (gdb_stdout, ")");
>>      }
>>    fprintf_filtered (gdb_stdout, " %s\n", macro->replacement);
>> +  return macro_walk_continue;
>>  }
>
> Same as above. If you decide to keep them please update the comment
> headers with a description of the return value.  This function,  and
> numerous other places too.

will do, though, these are documented in macrotab.h:macro_callback_fn,
i should at least
mention they are implementations of those, and arguments/return values
are documented there.
does that sound alright?

Re: [PATCH 2/7] [python] API for macros: memory management quirks.

[Matt Rice]

On Tue, Aug 30, 2011 at 4:47 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> matt rice <ratmice@gmail.com> writes:
>
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index f66c1d9..c56f431 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -14351,8 +14351,7 @@ macro_start_file (int file, int line,
>>    /* We don't create a macro table for this compilation unit
>>       at all until we actually get a filename.  */
>>    if (! pending_macros)
>> -    pending_macros = new_macro_table (&objfile->objfile_obstack,
>> -                                      objfile->macro_cache);
>> +    pending_macros = new_macro_table (objfile, macro_table_type_from_objfile);
>
> The comments in macrotab.h need to be updated with the new parameter
> info.  This and others.
>
> This is code I am not very sure of, having never dabbled in this area.
> So this review would benefit from a maintainer that is more familiar
> with this code.  But what benefit do we accrue now that we purely store
> macros in the objfile's obstack over (the now defunct) bcache?  Was
> there an underlying reason for dual this you discovered?  It seems odd that
> in the pre-patched version that struct macro_table had two strategies
> for this. Contextually, why did you elect to use the obstack in the
> objfile, instead of the previous ancillary related structure?

forgot reply-to all, Phil (you'll receive 2).

I didn't actually change this except to remove the pointers in macro_table,
we still use both objfile->obstack and objfile->macro_cache.



>> diff --git a/gdb/macrotab.c b/gdb/macrotab.c
>> index efcf835..b2825d2 100644
>> --- a/gdb/macrotab.c
>> +++ b/gdb/macrotab.c
>> @@ -35,13 +35,10 @@
>>
>>  struct macro_table
>>  {
>> -  /* The obstack this table's data should be allocated in, or zero if
>> -     we should use xmalloc.  */
>> -  struct obstack *obstack;
>> -
>> -  /* The bcache we should use to hold macro names, argument names, and
>> -     definitions, or zero if we should use xmalloc.  */
>> -  struct bcache *bcache;
>> +  /* The objfile who's obstack this table's data should be allocated in,
>> +     and bcache we should use to hold macro names, argument
>> +     names, and definitions, or zero if we should use xmalloc. */
>> +  struct objfile *objfile;
>
>
> Comment seems wrong, as bcache data structure no longer exists.
> Additionally, (not in the code, but in your email), a narrative why we
> are removing the bcache is needed.

It could definitely be clearer.
e.g. The objfile who's obstack and bcache.
i'll make it more clear that this is OBJFILE's bcache in the rest
of these comments.

as per why:
we are replacing bcache and obstack with objfile->bcache, and objfile->obstack
so that we can safely use register_objfile_data(), for validation.

Re: [PATCH 3/7] [python] API for macros: Add gdb.Macro class.

[Matt Rice]

On Tue, Aug 30, 2011 at 5:45 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> matt rice <ratmice@gmail.com> writes:

>> +macropy__definition_to_py (const struct macro_definition *macro)
>
> A small nit for me personally, but all of the other Python API classes
> use one "_" to separate the function library name from the function
> name.  This and others.

Hmm... It seems as though all the Python API classes use 'foopy_' prefixes
_only_ for the 'public methods available through python',  all of
these 2x"_" functions
are private functions, I understand that static functions such as
these do not pollute
the global namespace, but I find the tab completion behavior when
debugging gdb itself
that having prefixes on these helps considerably.

Other python API seems to use random function names for these
(symtab_, set_, sal_, etc).
thus I use the 2x_ mainly because of the 'beginning with _'
reservation for compiler/libc.

Anyhow, it doesn't seem like removing one of the '_'s is really the thing to do,
maybe macropy_pvt_* (i'll see if that works but some of the functions
are getting fairly long.)

>> +macropy__validate_self (PyObject *self)
>> +{
>> +  macro_object *me;
>> +
>> +  if (! PyObject_TypeCheck (self, &macro_object_type))
>> +    {
>> +      PyErr_SetString (PyExc_RuntimeError,
>> +                      _("Typecheck failure converting macro."));
>> +      return NULL;
>> +    }
>> +
>> +  me = (macro_object *) self;
>> +
>> +  if (me->objfile_validator->is_valid == 0)
>> +    {
>> +      PyErr_SetString (PyExc_RuntimeError, _("Macro is invalid."));
>> +      return NULL;
>> +    }
>> +
>> +  return me;
>> +}
>
> As you are pointing to an existing copy of self, and returning it, you
> *may* need to incref.  It depends if any of the functions downstream
> decref it.  Please check.

this is basically the same as the FOOPY_REQUIRE_VALID macro's
it shouldn't need to, for macros we don't really have a
symtab_object_to_symtab or sal_object_to_sal
type of function to do the type check in, putting that into e.g.
MACROPY_REQUIRE_VALID requires
using the PyObject * argument twice, I also found the assignment in
the macro to be weird :/

I should probably rename it macropy__require_valid().

>> +static int
>> +macropy_compare (PyObject *self, PyObject *o2)
>> +{
>> +  PyObject *my_str = macropy_str (self);
>> +  PyObject *other_str = NULL;
>> +  PyObject *an_error;
>> +  int err_code = -1;
>> +  int comparison_result = -1;
>> +
>> +  if (!my_str)
>> +    goto check_for_errors_and_return;
>> +
>> +  if (PyObject_TypeCheck (o2, &macro_object_type))
>> +    {
>> +      other_str = macropy_str (o2);
>> +      if (!other_str)
>> +     goto check_for_errors_and_return;
>> +
>> +      err_code = PyObject_Cmp (my_str, other_str, &comparison_result);
>> +    }
>
> Use PyObject_Compare

Hmm, I'm very skeptical of PyObject_Compare() because of its lack of a
way to identify if there was an error
that may have been cleared by something, e.g. gdb_py_print_stack()...
this may not be possible, but it may be if we figure out a way to have
Macro's comparison method 'monkey patchable'.

>> +  else
>> +    {
>> +      err_code = PyObject_Cmp (my_str, o2, &comparison_result);
>> +    }
>
> Ditto.
>
>> +  check_for_errors_and_return:
>> +    Py_XDECREF (my_str);
>> +    Py_XDECREF (other_str);
>> +
>> +    /* Because -1 is also Less Than we cannot use gdbpy_print_stack ()
>> +       which clears the error if set python print-stack is off.
>> +       Which would lead to (gdb) python print x == x
>> +       returning False with no error message displayed.
>> +
>> +       alternately if an error is set and the return value is not 1,
>> +       python will complain.
>> +
>> +       Thus if there is an error, we must normalize it
>> +       making sure that both an error and the return
>> +       value are -1. Should we encounter one.  */
>> +    an_error = PyErr_Occurred ();
>> +    if (an_error == NULL && err_code != -1)
>> +      {
>> +        /* a valid comparison */
>> +        return comparison_result;
>> +      }
>> +    else if (an_error == NULL && err_code == -1)
>> +      {
>> +     /* an unknown error */
>> +     PyErr_SetString (PyExc_RuntimeError,
>> +                      _("mystery error during macro comparison."));
>
>
> Don't overwrite the exception here.  And if PyObject_Compare has
> returned -1, there will be an exception so this can't happen.  You could
> probably just remove the condition entirely and just fold over to the
> else below.

that comment should also read 'and the return value is not -1' instead
of 'not 1'...

as per the above, we aren't overwriting the exception, in this case
'an_error' is the current exception
and it is NULL, while PyObject_Cmp returned -1 signalling to us that
there was an error.

if we return -1 without an error set they compare as 'less than',
and the user may have no idea that an error was even set.

unless we can prove this can never ever happen with any version of
python I prefer PyObject_Cmp,
which has the error code return value in place of PyObject_Compare
which relies on an PyErr_Occurred

e.g. If i figure out a way to add monkey patching of '__cmp__' on
gdb.Macro objects, its plausible that a user defined comparison
function could end up in a call to gdb_py_print_stack(), and a silent
failure.

>> +        return -1;
>> +      }
>> +    else /* a known error */
>> +      return -1;
>> +}
>> +
>
>
>> +static long
>> +macropy_hash (PyObject *o)
>> +{
>> +  long result = -1;
>> +  PyObject *my_str = macropy_str (o);
>> +
>> +  if (my_str)
>> +    result = PyObject_Hash (my_str);
>> +
>> +  /* The docs say PyObject_Hash should return -1 on error,
>> +     Don't believe it because it is not always true,
>> +     The exception gets raised regardless of return value,
>> +     But we should follow the documented return strategy of -1 on error.
>
> Even if you are right, are you right for all versions of Python?  I
> guess the PyErr_Occurred check is harmless.

no idea about all versions, i checked with 2.7, concurr it's harmless.
I'll try and make it less definitive/mention the version.


>> +static PyMethodDef macro_object_methods[] = {
>> +  { "argv", macropy_argv_method, METH_NOARGS,
>> +    "argv () -> List.\n\
>> +Return a list containing the names of the arguments for a function like \
>> +macro." },
>
> "function-like", imo.
>
>> +  { "definition", macropy_definition_method, METH_NOARGS,
>> +    "definition () -> String.\n\
>> +Return the macro's definition." },
>> +  { "include_trail", macropy_include_trail_method, METH_NOARGS,
>> +    "include_trail () -> List.\n\
>> +Return a list containing tuples with the filename and line number describing \
>> +how and where this macro came to be defined." },
>
> Your help and your return explanation don't match.  It should return a
> Tuple (if it doesn't, please fix it).  And please change  -> List to ->
> Tuple.

They do, but it's awkward
e.g. a list OF tuples, so [tuple, tuple, tuple]

>> +  struct objfile *objfile;
>> +};
>> +
>> +/* A macro_callback_fn for use with macro_foreach and friends that adds
>> +   a gdb.Macro object to the python list USER_DATA->LIST.
>> +   USER_DATA should be of type struct macropy_user_data.
>> +
>> +   A null USER_DATA->OBJFILE is currently be considered an error,
>> +   this is subject to change.
>> +
>> +   Aborts the iteration on error, and forces the macro iterator function
>> +   to return macro_walk_aborted check the iterators macro_walk_result.
>> +   On error it is the callers responsibility to dispose of the USER_DATA
>> +   structure and its contents.  The appropriate python exception
>> +   that caused the error has already been set.
>> +*/
>> +enum macro_walk_status
>> +pymacro_add_macro_to_list (const char *name,
>> +                const struct macro_definition *definition,
>> +                struct macro_source_file *source,
>> +                int line,
>> +                void *user_data);
>> +
>> +/* Create a new macro (gdb.Macro) object that encapsulates
>> +   the gdb macro representation.  OBJFILE is the objfile that contains
>> +   the macro table.  NAME is the name of the macro.  DEF the macros definition
>> +   SRC the source file containing the macro. LINE definitions location.  */
>> +PyObject *
>> +macropy_object_create (struct objfile *objfile,
>> +                    const char *name,
>> +                    const struct macro_definition *def,
>> +                    struct macro_source_file *src,
>> +                    int line);
>> +
>> +#endif /* GDB_PY_MACRO_H */
>
> Any reason why these need their own include?  (over python-internal.h).

Nope, just didn't see these all collected there.

> If these are called from anywhere other than python/*

they are not.

Re: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Matt Rice]

On Tue, Aug 30, 2011 at 1:34 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Phil Muldoon <pmuldoon@redhat.com> writes:
>
>> Tom Tromey <tromey@redhat.com> writes:
>>
>>>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>>>
>>> Phil> I think this should return a Tuple.  Tuples are immutable, and unless
>>> Phil> you for see a use for the user to manipulate the List, we should guard
>>> Phil> against it.  If you agree, please alter stpy_macros too.
>>>
>>> Using a tuple means you have to iterate twice -- once to count the
>>> objects and once to make the tuple.
>>
>> You can use a list, and then convert it to a tuple:
>>
>> PyList_AsTuple
>>
>> We already use the above.
>>
>> If you wanted to, you could use PyTuple_Ruse, as long as there is
>> one reference to the tuple.
>
> Apologies for the typos, but the above should read:
>
>  If you wanted too, you could use PyTuple_Resize, as long as there is
>  only one reference to the tuple.


I will try these out, (convert list->tuple and PyTuple_Resize()),
but PyTuple_Resize is going to use realloc, which could end up moving it...

As Tom mentioned, we create a new list every time, so really its not
going to foul us up if the user does modify it.
What would be preferred is an immutable list with a constant time
append only exposed to the c api.
but python doesn't really have one of these.

I'll give it a shot and see how it performs, but my gut says list.

Re: [PATCH 5/7] [python] API for macros: gdb.Objfile symtabs method.

[Matt Rice]

On Tue, Aug 30, 2011 at 6:07 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> matt rice <ratmice@gmail.com> writes:
>
> If there are no symtabs, why return an empty list?  Would Py_None make
> more sense here.  And same rules apply to returning a Tuple, too, as
> others.
>

the main reason here to return an empty list is just because things like

(gdb) py for i in None: print "foo"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: 'NoneType' object is not iterable
Error while executing Python code.

thus you require an 'if' and python's if doesn't work as a one liner...
such as

(gdb) py for i in list(): print "foo"
(gdb)

If you would still like me to change it, no problem.

Re: [PATCH 5/7] [python] API for macros: gdb.Objfile symtabs method.

[Matt Rice]

On Tue, Aug 30, 2011 at 10:34 AM, Tom Tromey <tromey@redhat.com> wrote:
> I don't mind this method per se, but I think people will find it
> confusing, as it only returns full symtabs -- but there is no direct way
> for the Python programmer to cause symtab expansion.

Yeah, I'll just drop this patch from the series for now,
and leave it for another series that is more complete.

RE: [PATCH 5/7] [python] API for macros: gdb.Objfile symtabs method.

[Paul_Koning]

>> If there are no symtabs, why return an empty list?  Would Py_None make 
>> more sense here.  And same rules apply to returning a Tuple, too, as 
>> others.
>
>the main reason here to return an empty list is just because things like
>
>(gdb) py for i in None: print "foo"
>Traceback (most recent call last):
>  File "<string>", line 1, in <module>
>TypeError: 'NoneType' object is not iterable Error while executing Python code.

There's a precedent: gdb.Type.fields() returns an empty list if the type doesn't have fields.  I was wondering why that is, but your explanation gives the reason.

	paul

RE: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Paul_Koning]

>What would be preferred is an immutable list with a constant time append only exposed to the c api.
>but python doesn't really have one of these.

What about PyTuple_SetItem?  It appends an item to a tuple; the description doesn't say if it's constant time, but given how it's meant to be used one would hope it is.

	paul

RE: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Paul_Koning]

Never mind, I misread the Python doc.

	paul

-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Koning, Paul
Sent: Thursday, September 01, 2011 9:07 PM
To: ratmice@gmail.com; pmuldoon@redhat.com
Cc: tromey@redhat.com; gdb-patches@sourceware.org
Subject: RE: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

>What would be preferred is an immutable list with a constant time append only exposed to the c api.
>but python doesn't really have one of these.

What about PyTuple_SetItem?  It appends an item to a tuple; the description doesn't say if it's constant time, but given how it's meant to be used one would hope it is.

	paul

Re: [PATCH 4/7] [python] API for macros: Add methods to get a gdb.Macro.

[Phil Muldoon]

Matt Rice <ratmice@gmail.com> writes:

>>>>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>>>>
>>>> Phil> I think this should return a Tuple.  Tuples are immutable, and unless
>>>> Phil> you for see a use for the user to manipulate the List, we should guard
>>>> Phil> against it.  If you agree, please alter stpy_macros too.
>>>>
>>>> Using a tuple means you have to iterate twice -- once to count the
>>>> objects and once to make the tuple.
>>>
>>> You can use a list, and then convert it to a tuple:
>>>
>>> PyList_AsTuple
>>>
>>> We already use the above.
>>>
>>> If you wanted to, you could use PyTuple_Ruse, as long as there is
>>> one reference to the tuple.
>>
>> Apologies for the typos, but the above should read:
>>
>>  If you wanted too, you could use PyTuple_Resize, as long as there is
>>  only one reference to the tuple.
>
>
> I will try these out, (convert list->tuple and PyTuple_Resize()),
> but PyTuple_Resize is going to use realloc, which could end up moving it...

Actually, just leave them.  I thought about it some more, and with the
macro counts being so incredibly large in some projects, I think just
returning a list here would avoid large allocs/reallocs.  So Tom was
right, I was wrong.  All normal ;)

Cheers

Phil