[PATCH 03/11] Iterate over 'struct varobj_item' instead of PyObject

[PATCH 03/11] Iterate over 'struct varobj_item' instead of PyObject

[Yao Qi]

In previous patch, "saved_item" is still a PyOjbect and iteration is
still performed over PyObject.  This patch continues to decouple
iteration from python code, so it changes its type to "struct
varobj_item *", so that the iterator itself is independent of python.

gdb:

2013-11-24  Pedro Alves  <pedro@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* python/py-varobj.c (py_varobj_iter_next): Move some code
	from varobj.c.
	(py_varobj_get_iterator):
	* varobj-iter.h (struct varobj_item): New.  Moved from
	varobj.c.
	* varobj.c: Move "varobj-iter.h" inclusion earlier.
	(struct varobj_item): Remove.
	(varobj_clear_saved_item): New function.
	(update_dynamic_varobj_children): Move python-related code to
	py-varobj.c.
	(free_variable): Call varobj_clear_saved_item.
---
 gdb/python/py-varobj.c |   17 ++++++++++-
 gdb/varobj-iter.h      |   12 ++++++-
 gdb/varobj.c           |   73 ++++++++++++++++-------------------------------
 3 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index 6588fc2..506e865 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -51,6 +51,9 @@ py_varobj_iter_next (struct varobj_iter *self)
   struct py_varobj_iter *t = (struct py_varobj_iter *) self;
   struct cleanup *back_to;
   PyObject *item;
+  PyObject *py_v;
+  varobj_item *vitem;
+  const char *name = NULL;
 
   back_to = varobj_ensure_python_env (self->var);
 
@@ -98,9 +101,21 @@ py_varobj_iter_next (struct varobj_iter *self)
 	}
     }
 
+  if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
+    {
+      gdbpy_print_stack ();
+      error (_("Invalid item from the child list"));
+    }
+
+  vitem = xmalloc (sizeof *vitem);
+  vitem->value = convert_value_from_python (py_v);
+  if (vitem->value == NULL)
+    gdbpy_print_stack ();
+  vitem->name = xstrdup (name);
+
   self->next_raw_index++;
   do_cleanups (back_to);
-  return item;
+  return vitem;
 }
 
 /* The 'vtable' of pretty-printed python varobj iterators.  */
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
index 59be278..b4bda82 100644
--- a/gdb/varobj-iter.h
+++ b/gdb/varobj-iter.h
@@ -14,9 +14,17 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-struct varobj_iter_ops;
+/* A node or item of varobj, composed by the name and the value.  */
+
+typedef struct varobj_item
+{
+  /* Name of this item.  */
+  char *name;
+  /* Value of this item.  */
+  struct value *value;
+} varobj_item;
 
-typedef PyObject varobj_item;
+struct varobj_iter_ops;
 
 /* A dynamic varobj iterator "class".  */
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 7a8305b..28e934c 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -33,6 +33,7 @@
 #include "vec.h"
 #include "gdbthread.h"
 #include "inferior.h"
+#include "varobj-iter.h"
 
 #if HAVE_PYTHON
 #include "python/python.h"
@@ -41,8 +42,6 @@
 typedef int PyObject;
 #endif
 
-#include "varobj-iter.h"
-
 /* Non-zero if we want to see trace of varobj level stuff.  */
 
 unsigned int varobjdebug = 0;
@@ -110,16 +109,6 @@ struct varobj_root
   struct varobj_root *next;
 };
 
-/* A node or item of varobj, composed by the name and the value.  */
-
-struct varobj_item
-{
-  /* Name of this item.  */
-  char *name;
-  /* Value of this item.  */
-  struct value *value;
-};
-
 /* Dynamic part of varobj.  */
 
 struct varobj_dynamic
@@ -787,6 +776,18 @@ varobj_get_iterator (struct varobj *var)
 requested an interator from a non-dynamic varobj"));
 }
 
+/* Release and clear VAR's saved item, if any.  */
+
+static void
+varobj_clear_saved_item (struct varobj_dynamic *var)
+{
+  if (var->saved_item != NULL)
+    {
+      value_free (var->saved_item->value);
+      xfree (var->saved_item);
+      var->saved_item = NULL;
+    }
+}
 #endif
 
 static int
@@ -801,14 +802,8 @@ update_dynamic_varobj_children (struct varobj *var,
 				int to)
 {
 #if HAVE_PYTHON
-  struct cleanup *back_to;
   int i;
 
-  if (!gdb_python_initialized)
-    return 0;
-
-  back_to = varobj_ensure_python_env (var);
-
   *cchanged = 0;
 
   if (update_children || var->dynamic->child_iter == NULL)
@@ -816,16 +811,12 @@ update_dynamic_varobj_children (struct varobj *var,
       varobj_iter_delete (var->dynamic->child_iter);
       var->dynamic->child_iter = varobj_get_iterator (var);
 
-      Py_XDECREF (var->dynamic->saved_item);
-      var->dynamic->saved_item = NULL;
+      varobj_clear_saved_item (var->dynamic);
 
       i = 0;
 
       if (var->dynamic->child_iter == NULL)
-	{
-	  do_cleanups (back_to);
-	  return 0;
-	}
+	return 0;
     }
   else
     i = VEC_length (varobj_p, var->children);
@@ -834,10 +825,10 @@ update_dynamic_varobj_children (struct varobj *var,
      are more children.  */
   for (; to < 0 || i < to + 1; ++i)
     {
-      PyObject *item;
+      varobj_item *item;
 
       /* See if there was a leftover from last time.  */
-      if (var->dynamic->saved_item)
+      if (var->dynamic->saved_item != NULL)
 	{
 	  item = var->dynamic->saved_item;
 	  var->dynamic->saved_item = NULL;
@@ -845,6 +836,10 @@ update_dynamic_varobj_children (struct varobj *var,
       else
 	{
 	  item = varobj_iter_next (var->dynamic->child_iter);
+	  /* Release vitem->value so its lifetime is not bound to the
+	     execution of a command.  */
+	  if (item != NULL && item->value != NULL)
+	    release_value_or_incref (item->value);
 	}
 
       if (item == NULL)
@@ -857,36 +852,19 @@ update_dynamic_varobj_children (struct varobj *var,
       /* We don't want to push the extra child on any report list.  */
       if (to < 0 || i < to)
 	{
-	  PyObject *py_v;
-	  const char *name;
-	  struct varobj_item varobj_item;
-	  struct cleanup *inner;
 	  int can_mention = from < 0 || i >= from;
 
-	  inner = make_cleanup_py_decref (item);
-
-	  if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
-	    {
-	      gdbpy_print_stack ();
-	      error (_("Invalid item from the child list"));
-	    }
-
-	  varobj_item.value = convert_value_from_python (py_v);
-	  if (varobj_item.value == NULL)
-	    gdbpy_print_stack ();
-	  varobj_item.name = xstrdup (name);
-
 	  install_dynamic_child (var, can_mention ? changed : NULL,
 				 can_mention ? type_changed : NULL,
 				 can_mention ? new : NULL,
 				 can_mention ? unchanged : NULL,
 				 can_mention ? cchanged : NULL, i,
-				 &varobj_item);
-	  do_cleanups (inner);
+				 item);
+
+	  xfree (item);
 	}
       else
 	{
-	  Py_XDECREF (var->dynamic->saved_item);
 	  var->dynamic->saved_item = item;
 
 	  /* We want to truncate the child list just before this
@@ -2180,12 +2158,11 @@ free_variable (struct varobj *var)
 
       Py_XDECREF (var->dynamic->constructor);
       Py_XDECREF (var->dynamic->pretty_printer);
-      Py_XDECREF (var->dynamic->child_iter);
-      Py_XDECREF (var->dynamic->saved_item);
       do_cleanups (cleanup);
     }
 #endif
 
+  varobj_clear_saved_item (var->dynamic);
   value_free (var->value);
 
   /* Free the expression if this is a root variable.  */
-- 
1.7.7.6

[PATCH 10/11] Match dynamic="1" in the output of -var-list-children

[Yao Qi]

When I play with pretty-printer and available-children-only, I get the
following output,

-var-list-children  ss1
^done,numchild="2",children=[child={name="ss1.a",exp="a",numchild="0",type="struct s",thread-id="1",dynamic="1"},child={name="ss1.b",exp="b",numchild="0",type="struct s",thread-id="1",dynamic="1"}],has_more="0"

existing proc mi_child_regexp doesn't append "dynamic=1" to the pattern,
so it doesn't match output.  This patch adds "dynamic=1" to the pattern.

I am not satisfied with the regexp construction for each child in
mi_child_regexp.  In each list, there are three mandatory fields,
"name", "exp", and "numchild".  There are also some optional fields,
"value", "type" and "dynamic".  The current regexp construction code
is hard to be extended (add for "dynamic").  I suggest that we can
pass the list to mi_list_varobj_children like this,

 { name exp numchild optional }

the list has four elements, and OPTIONAL is an array, which index can
be "value", "type" or "dynamic".  The existing usage of
mi_list_varobj_children like:

mi_list_varobj_children "struct_declarations" {
    {struct_declarations.integer integer 0 int}
} "test"

will be rewritten to:

mi_list_varobj_children "struct_declarations" {
    {struct_declarations.integer integer 0 {type int}}
} "test"

if we want to match "dynamic" attribute, we can write:

mi_list_varobj_children "struct_declarations" {
    {struct_declarations.integer integer 0 {type int dynamic 1}}
} "test"

Since mi_list_varobj_children has been widely used in test suite, I'd
like to defer the change after this patch series.

gdb/testsuite:

2013-11-24  Yao Qi  <yao@codesourcery.com>

	* lib/mi-support.exp (mi_child_regexp): Append 'dynamic="1"' to
	children_exp.
---
 gdb/testsuite/lib/mi-support.exp |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 0c3cdbe..99ff9a5 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1410,21 +1410,25 @@ proc mi_child_regexp {children add_child} {
 	set name [lindex $item 0]
 	set exp [lindex $item  1]
 	set numchild [lindex $item 2]
+
+	set line "$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\""
+
 	if {[llength $item] == 5} {
 	    set type [lindex $item 3]
 	    set value [lindex $item 4]
 
-	    lappend children_exp\
-		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\",value=\"$value\",type=\"$type\"(,thread-id=\"\[0-9\]+\")?}"
+	    append line ",value=\"$value\",type=\"$type\""
 	} elseif {[llength $item] == 4} {
 	    set type [lindex $item 3]
 
-	    lappend children_exp\
-		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\",type=\"$type\"(,thread-id=\"\[0-9\]+\")?}"
+	    append line ",type=\"$type\""
 	} else {
-	    lappend children_exp\
-		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\"(,thread-id=\"\[0-9\]+\")?}"
 	}
+
+	append line \
+	    "(,thread-id=\"\[0-9\]+\")?(,dynamic=\"1\")?}"
+
+	lappend children_exp $line
     }
     return [join $children_exp ","]
 }
-- 
1.7.7.6

[PATCH 05/11] Rename varobj_pretty_printed_p to varobj_is_dynamic_p

[Yao Qi]

We think varobj with --available-children-only behaves like a dynamic
varobj, so dyanmic varobj is not pretty-printer specific.  We rename
varobj_pretty_printed_p to varobj_is_dynamic_p, so that we can handle
available-children-only checking in varobj_is_dynamic_p in the next
patch.

gdb:

2013-11-24  Yao Qi  <yao@codesourcery.com>

	* varobj.c (varobj_pretty_printed_p): Rename to ...
	(varobj_is_dynamic_p): ... this.  New function.
	* varobj.h (varobj_pretty_printed_p): Remove declaration.
	(varobj_is_dynamic_p): Declare.
	* mi/mi-cmd-var.c (print_varobj): Caller update.
	(mi_print_value_p, varobj_update_one): Likewise.
---
 gdb/mi/mi-cmd-var.c |    6 +++---
 gdb/varobj.c        |    4 +++-
 gdb/varobj.h        |    2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 84cdc15..2201630 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -88,7 +88,7 @@ print_varobj (struct varobj *var, enum print_values print_values,
       xfree (display_hint);
     }
 
-  if (varobj_pretty_printed_p (var))
+  if (varobj_is_dynamic_p (var))
     ui_out_field_int (uiout, "dynamic", 1);
 }
 
@@ -352,7 +352,7 @@ mi_print_value_p (struct varobj *var, enum print_values print_values)
   if (print_values == PRINT_ALL_VALUES)
     return 1;
 
-  if (varobj_pretty_printed_p (var))
+  if (varobj_is_dynamic_p (var))
     return 1;
 
   type = varobj_get_gdb_type (var);
@@ -777,7 +777,7 @@ varobj_update_one (struct varobj *var, enum print_values print_values,
 	  xfree (display_hint);
 	}
 
-      if (varobj_pretty_printed_p (r->varobj))
+      if (varobj_is_dynamic_p (r->varobj))
 	ui_out_field_int (uiout, "dynamic", 1);
 
       varobj_get_child_range (r->varobj, &from, &to);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index e362229..948f80c 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1066,8 +1066,10 @@ varobj_get_attributes (struct varobj *var)
   return attributes;
 }
 
+/* Return true if VAR is a dynamic varobj.  */
+
 int
-varobj_pretty_printed_p (struct varobj *var)
+varobj_is_dynamic_p (struct varobj *var)
 {
   return var->dynamic->pretty_printer != NULL;
 }
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 0f68311..60ace6f 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -306,7 +306,7 @@ extern void varobj_enable_pretty_printing (void);
 
 extern int varobj_has_more (struct varobj *var, int to);
 
-extern int varobj_pretty_printed_p (struct varobj *var);
+extern int varobj_is_dynamic_p (struct varobj *var);
 
 extern  struct cleanup *varobj_ensure_python_env (struct varobj *var);
 
-- 
1.7.7.6

[PATCH 06/11] Use varobj_is_dynamic_p more widely

[Yao Qi]

Use varobj_is_dynamic_p more widely so that the callers of
varobj_is_dynamic_p are unchanged when we add available-children-only
stuff in varobj_is_dynamic_p.

gdb:

2013-11-24  Yao Qi  <yao@codesourcery.com>

	* varobj.c (varobj_get_num_children): Call
	varobj_is_dynamic_p.
	(varobj_list_children): Likewise.
	(varobj_update): Likewise.  Update comments.
---
 gdb/varobj.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gdb/varobj.c b/gdb/varobj.c
index 948f80c..0e0be6c 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -895,7 +895,7 @@ varobj_get_num_children (struct varobj *var)
 {
   if (var->num_children == -1)
     {
-      if (var->dynamic->pretty_printer != NULL)
+      if (varobj_is_dynamic_p (var))
 	{
 	  int dummy;
 
@@ -922,7 +922,7 @@ varobj_list_children (struct varobj *var, int *from, int *to)
 
   var->dynamic->children_requested = 1;
 
-  if (var->dynamic->pretty_printer != NULL)
+  if (varobj_is_dynamic_p (var))
     {
       /* This, in theory, can result in the number of children changing without
 	 frontend noticing.  But well, calling -var-list-children on the same
@@ -1715,10 +1715,9 @@ varobj_update (struct varobj **varp, int explicit)
 	    }
 	}
 
-      /* We probably should not get children of a varobj that has a
-	 pretty-printer, but for which -var-list-children was never
-	 invoked.  */
-      if (v->dynamic->pretty_printer != NULL)
+      /* We probably should not get children of a dynamic varobj, but
+	 for which -var-list-children was never invoked.  */
+      if (varobj_is_dynamic_p (v))
 	{
 	  VEC (varobj_p) *changed = 0, *type_changed = 0, *unchanged = 0;
 	  VEC (varobj_p) *new = 0;
-- 
1.7.7.6

[PATCH 07/11] MI option --available-children-only

[Yao Qi]

This patch adds option --available-children-only to three MI commands,
parse it, and set corresponding field of 'struct varobj_dynamic'.

gdb:

2013-11-24  Pedro Alves  <pedro@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmd-var.c (mi_cmd_var_create): Use mi_getopt to parse
	options.  Handle "--available-children-only".
	(mi_cmd_var_info_num_children): Likewise.
	(mi_cmd_var_list_children): Likewise.
	* varobj.c (struct varobj_dynamic) <available_children_only>:
	New.
	(new_variable, new_root_variable): Update declaration.
	(varobj_is_dynamic_p): Return true if available-children-only
	is true.
	(varobj_create): Add new argument "available_children_only".
	Callers update.
	(varobj_get_num_children): Handle "available_children_only"
	(varobj_set_available_children_only): New function.
	(varobj_list_children): Handle "available_children_only".
	(install_new_value): Likewise.
	(varobj_update): Likewise.
	(new_variable): Add new argument "available_children_only".
	(new_root_variable): Likewise.
	(varobj_invalidate_iter): Likewise.
	* varobj.h (varobj_create): Update declaration.
	(varobj_set_available_children_only): Declare.
---
 gdb/mi/mi-cmd-var.c |  128 ++++++++++++++++++++++++++++++++++++++++++++-------
 gdb/varobj.c        |   51 +++++++++++++++-----
 gdb/varobj.h        |   11 ++++-
 3 files changed, 160 insertions(+), 30 deletions(-)

diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 2201630..552a527 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -105,19 +105,50 @@ mi_cmd_var_create (char *command, char **argv, int argc)
   char *expr;
   struct cleanup *old_cleanups;
   enum varobj_type var_type;
+  int available_children_only = 0;
+  int oind = 0;
+  enum opt
+    {
+      AVAILABLE_CHILDREN_ONLY,
+    };
+  static const struct mi_opt opts[] =
+    {
+      {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
+      { 0, 0, 0 }
+    };
 
-  if (argc != 3)
-    error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
+  /* Parse arguments.  In this instance we are just looking for
+     --available_children_only.  */
+  while (1)
+    {
+      char *oarg;
+      int opt = mi_getopt ("-var-create", argc, argv,
+			   opts, &oind, &oarg);
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case AVAILABLE_CHILDREN_ONLY:
+	  available_children_only = 1;
+	  break;
+	}
+    }
 
-  name = xstrdup (argv[0]);
+
+  if (argc - oind != 3)
+    error (_("Usage: -var-create [--available-children-only] "
+	     "NAME FRAME EXPRESSION"));
+
+
+  name = xstrdup (argv[oind]);
   /* Add cleanup for name. Must be free_current_contents as name can
      be reallocated.  */
   old_cleanups = make_cleanup (free_current_contents, &name);
 
-  frame = xstrdup (argv[1]);
+  frame = xstrdup (argv[oind + 1]);
   make_cleanup (xfree, frame);
 
-  expr = xstrdup (argv[2]);
+  expr = xstrdup (argv[oind + 2]);
   make_cleanup (xfree, expr);
 
   if (strcmp (name, "-") == 0)
@@ -143,7 +174,8 @@ mi_cmd_var_create (char *command, char **argv, int argc)
 		    "Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
 			name, frame, hex_string (frameaddr), expr);
 
-  var = varobj_create (name, expr, frameaddr, var_type);
+  var = varobj_create (name, expr, frameaddr, var_type,
+		       available_children_only);
 
   if (var == NULL)
     error (_("-var-create: unable to create variable object"));
@@ -328,12 +360,43 @@ mi_cmd_var_info_num_children (char *command, char **argv, int argc)
 {
   struct ui_out *uiout = current_uiout;
   struct varobj *var;
+  int available_children_only = 0;
+  int oind = 0;
+  enum opt
+    {
+      AVAILABLE_CHILDREN_ONLY,
+    };
+  static const struct mi_opt opts[] =
+    {
+      {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
+      { 0, 0, 0 }
+    };
 
-  if (argc != 1)
-    error (_("-var-info-num-children: Usage: NAME."));
+  /* Parse arguments.  In this instance we are just looking for
+     --available_children_only.  */
+  while (1)
+    {
+      char *oarg;
+      int opt = mi_getopt ("-var-info-num-children", argc, argv,
+			   opts, &oind, &oarg);
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case AVAILABLE_CHILDREN_ONLY:
+	  available_children_only = 1;
+	  break;
+	}
+    }
+
+  if (argc - oind != 1)
+    error (_("-var-info-num-children: Usage: "
+	     "[--available-children-only] NAME."));
 
   /* Get varobj handle, if a valid var obj name was specified.  */
-  var = varobj_get_handle (argv[0]);
+  var = varobj_get_handle (argv[oind]);
+
+  varobj_set_available_children_only (var, available_children_only);
 
   ui_out_field_int (uiout, "numchild", varobj_get_num_children (var));
 }
@@ -381,18 +444,47 @@ mi_cmd_var_list_children (char *command, char **argv, int argc)
   int ix;
   int from, to;
   char *display_hint;
+  int available_children_only = 0;
+  int oind = 0;
+  enum opt
+    {
+      AVAILABLE_CHILDREN_ONLY,
+    };
+  static const struct mi_opt opts[] =
+    {
+      {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
+      { 0, 0, 0 }
+    };
+
+  /* Parse arguments.  In this instance we are just looking for
+     --available_children_only.  */
+  while (1)
+    {
+      char *oarg;
+      int opt = mi_getopt_allow_unknown ("-var-list-children", argc, argv,
+					 opts, &oind, &oarg);
+
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case AVAILABLE_CHILDREN_ONLY:
+	  available_children_only = 1;
+	  break;
+	}
+    }
 
-  if (argc < 1 || argc > 4)
-    error (_("-var-list-children: Usage: "
+  if (argc - oind < 1 || argc - oind > 4)
+    error (_("-var-list-children: Usage: [--available-children-only] "
 	     "[PRINT_VALUES] NAME [FROM TO]"));
 
   /* Get varobj handle, if a valid var obj name was specified.  */
-  if (argc == 1 || argc == 3)
-    var = varobj_get_handle (argv[0]);
+  if (argc - oind == 1 || argc - oind == 3)
+    var = varobj_get_handle (argv[oind]);
   else
-    var = varobj_get_handle (argv[1]);
+    var = varobj_get_handle (argv[oind + 1]);
 
-  if (argc > 2)
+  if (argc - oind > 2)
     {
       from = atoi (argv[argc - 2]);
       to = atoi (argv[argc - 1]);
@@ -403,10 +495,12 @@ mi_cmd_var_list_children (char *command, char **argv, int argc)
       to = -1;
     }
 
+  varobj_set_available_children_only (var, available_children_only);
+
   children = varobj_list_children (var, &from, &to);
   ui_out_field_int (uiout, "numchild", to - from);
-  if (argc == 2 || argc == 4)
-    print_values = mi_parse_print_values (argv[0]);
+  if (argc - oind == 2 || argc - oind == 4)
+    print_values = mi_parse_print_values (argv[oind]);
   else
     print_values = PRINT_NO_VALUES;
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 0e0be6c..1fd9fd2 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -119,6 +119,10 @@ struct varobj_dynamic
      can avoid that.  */
   int children_requested;
 
+  /* True if this varobj only includes available/collected objects in
+     its list of children.  */
+  int available_children_only;
+
   /* The pretty-printer constructor.  If NULL, then the default
      pretty-printer will be looked up.  If None, then no
      pretty-printer will be installed.  */
@@ -175,9 +179,9 @@ create_child_with_value (struct varobj *parent, int index,
 
 /* Utility routines */
 
-static struct varobj *new_variable (void);
+static struct varobj *new_variable (int available_children_only);
 
-static struct varobj *new_root_variable (void);
+static struct varobj *new_root_variable (int available_children_only);
 
 static void free_variable (struct varobj *var);
 
@@ -285,14 +289,14 @@ find_frame_addr_in_frame_chain (CORE_ADDR frame_addr)
 }
 
 struct varobj *
-varobj_create (char *objname,
-	       char *expression, CORE_ADDR frame, enum varobj_type type)
+varobj_create (char *objname, char *expression, CORE_ADDR frame,
+	       enum varobj_type type, int available_children_only)
 {
   struct varobj *var;
   struct cleanup *old_chain;
 
   /* Fill out a varobj structure for the (root) variable being constructed.  */
-  var = new_root_variable ();
+  var = new_root_variable (available_children_only);
   old_chain = make_cleanup_free_variable (var);
 
   if (expression != NULL)
@@ -911,6 +915,23 @@ varobj_get_num_children (struct varobj *var)
   return var->num_children >= 0 ? var->num_children : 0;
 }
 
+void
+varobj_set_available_children_only (struct varobj *var, int available_only)
+{
+  if (var->dynamic->available_children_only != available_only)
+    {
+      /* If there are any children now, wipe them.  */
+      varobj_delete (var, NULL, /* children only */ 1);
+      var->num_children = -1;
+      var->dynamic->available_children_only = available_only;
+
+      /* We're starting over, so get rid of any iterator.  */
+      varobj_iter_delete (var->dynamic->child_iter);
+      var->dynamic->child_iter = NULL;
+      varobj_clear_saved_item (var->dynamic);
+    }
+}
+
 /* Creates a list of the immediate children of a variable object;
    the return code is the number of such children or -1 on error.  */
 
@@ -1071,7 +1092,8 @@ varobj_get_attributes (struct varobj *var)
 int
 varobj_is_dynamic_p (struct varobj *var)
 {
-  return var->dynamic->pretty_printer != NULL;
+  return (var->dynamic->pretty_printer != NULL
+	  || var->dynamic->available_children_only);
 }
 
 char *
@@ -2049,7 +2071,7 @@ create_child_with_value (struct varobj *parent, int index,
   struct varobj *child;
   char *childs_name;
 
-  child = new_variable ();
+  child = new_variable (parent->dynamic->available_children_only);
 
   /* NAME is allocated by caller.  */
   child->name = item->name;
@@ -2086,8 +2108,9 @@ create_child_with_value (struct varobj *parent, int index,
  */
 
 /* Allocate memory and initialize a new variable.  */
+
 static struct varobj *
-new_variable (void)
+new_variable (int available_children_only)
 {
   struct varobj *var;
 
@@ -2116,15 +2139,17 @@ new_variable (void)
   var->dynamic->pretty_printer = 0;
   var->dynamic->child_iter = 0;
   var->dynamic->saved_item = 0;
+  var->dynamic->available_children_only = available_children_only;
 
   return var;
 }
 
 /* Allocate memory and initialize a new root variable.  */
+
 static struct varobj *
-new_root_variable (void)
+new_root_variable (int available_children_only)
 {
-  struct varobj *var = new_variable ();
+  struct varobj *var = new_variable (available_children_only);
 
   var->root = (struct varobj_root *) xmalloc (sizeof (struct varobj_root));
   var->root->lang_ops = NULL;
@@ -2396,7 +2421,8 @@ value_of_root (struct varobj **var_handle, int *type_changed)
       char *old_type, *new_type;
 
       tmp_var = varobj_create (NULL, var->name, (CORE_ADDR) 0,
-			       USE_SELECTED_FRAME);
+			       USE_SELECTED_FRAME,
+			       var->dynamic->available_children_only);
       if (tmp_var == NULL)
 	{
 	  return NULL;
@@ -2756,7 +2782,8 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
       /* Try to create a varobj with same expression.  If we succeed
 	 replace the old varobj, otherwise invalidate it.  */
       tmp_var = varobj_create (NULL, var->name, (CORE_ADDR) 0,
-			       USE_CURRENT_FRAME);
+			       USE_CURRENT_FRAME,
+			       var->dynamic->available_children_only);
       if (tmp_var != NULL) 
 	{ 
 	  tmp_var->obj_name = xstrdup (var->obj_name);
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 60ace6f..9beec79 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -225,7 +225,8 @@ const struct lang_varobj_ops ada_varobj_ops;
 
 extern struct varobj *varobj_create (char *objname,
 				     char *expression, CORE_ADDR frame,
-				     enum varobj_type type);
+				     enum varobj_type type,
+				     int available_children_only);
 
 extern char *varobj_gen_name (void);
 
@@ -310,6 +311,14 @@ extern int varobj_is_dynamic_p (struct varobj *var);
 
 extern  struct cleanup *varobj_ensure_python_env (struct varobj *var);
 
+/* Marks the varobj VAR as listing available children only or not,
+   depending on the boolean AVAILABLE_ONLY.  If the
+   available-only-ness of the varobj changes compared to its present
+   state with this call, the varobj's current list of children is
+   deleted.  */
+extern void varobj_set_available_children_only (struct varobj *var,
+						int available_only);
+
 extern int varobj_default_value_is_changeable_p (struct varobj *var);
 extern int varobj_value_is_changeable_p (struct varobj *var);
 
-- 
1.7.7.6

[PATCH 01/11] Use 'struct varobj_item' to represent name and value pair

[Yao Qi]

Hi,
name and value pair is widely used in varobj.c.  This patch is to add
a new struct varobj_item to represent them, so that the number of
function arguments can be reduced.  Finally, the iteration is done on
'struct varobj_item' instead of PyObject after this patch series.

gdb:

2013-11-24  Yao Qi  <yao@codesourcery.com>

	* varobj.c (struct varobj_item): New structure.
	(create_child_with_value): Update declaration.
	(varobj_add_child): Replace arguments 'name' and 'value' with
	'item'.  Callers update.
	(install_dynamic_child): Likewise.
	(update_dynamic_varobj_children): Likewise.
	(varobj_add_child): Likewise.
	(create_child_with_value): Likewise.
---
 gdb/varobj.c |   64 +++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/gdb/varobj.c b/gdb/varobj.c
index b78969a..4a10617 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -108,6 +108,16 @@ struct varobj_root
   struct varobj_root *next;
 };
 
+/* A node or item of varobj, composed by the name and the value.  */
+
+struct varobj_item
+{
+  /* Name of this item.  */
+  char *name;
+  /* Value of this item.  */
+  struct value *value;
+};
+
 /* Dynamic part of varobj.  */
 
 struct varobj_dynamic
@@ -169,8 +179,8 @@ static void uninstall_variable (struct varobj *);
 static struct varobj *create_child (struct varobj *, int, char *);
 
 static struct varobj *
-create_child_with_value (struct varobj *parent, int index, char *name,
-			 struct value *value);
+create_child_with_value (struct varobj *parent, int index,
+			 struct varobj_item *item);
 
 /* Utility routines */
 
@@ -214,8 +224,7 @@ static int is_root_p (struct varobj *var);
 #if HAVE_PYTHON
 
 static struct varobj *varobj_add_child (struct varobj *var,
-					char *name,
-					struct value *value);
+					struct varobj_item *item);
 
 #endif /* HAVE_PYTHON */
 
@@ -714,13 +723,12 @@ install_dynamic_child (struct varobj *var,
 		       VEC (varobj_p) **unchanged,
 		       int *cchanged,
 		       int index,
-		       char *name,
-		       struct value *value)
+		       struct varobj_item *item)
 {
   if (VEC_length (varobj_p, var->children) < index + 1)
     {
       /* There's no child yet.  */
-      struct varobj *child = varobj_add_child (var, name, value);
+      struct varobj *child = varobj_add_child (var, item);
 
       if (new)
 	{
@@ -731,14 +739,14 @@ install_dynamic_child (struct varobj *var,
   else
     {
       varobj_p existing = VEC_index (varobj_p, var->children, index);
-      int type_updated = update_type_if_necessary (existing, value);
+      int type_updated = update_type_if_necessary (existing, item->value);
 
       if (type_updated)
 	{
 	  if (type_changed)
 	    VEC_safe_push (varobj_p, *type_changed, existing);
 	}
-      if (install_new_value (existing, value, 0))
+      if (install_new_value (existing, item->value, 0))
 	{
 	  if (!type_updated && changed)
 	    VEC_safe_push (varobj_p, *changed, existing);
@@ -889,7 +897,7 @@ update_dynamic_varobj_children (struct varobj *var,
 	{
 	  PyObject *py_v;
 	  const char *name;
-	  struct value *v;
+	  struct varobj_item varobj_item;
 	  struct cleanup *inner;
 	  int can_mention = from < 0 || i >= from;
 
@@ -901,15 +909,17 @@ update_dynamic_varobj_children (struct varobj *var,
 	      error (_("Invalid item from the child list"));
 	    }
 
-	  v = convert_value_from_python (py_v);
-	  if (v == NULL)
+	  varobj_item.value = convert_value_from_python (py_v);
+	  if (varobj_item.value == NULL)
 	    gdbpy_print_stack ();
+	  varobj_item.name = xstrdup (name);
+
 	  install_dynamic_child (var, can_mention ? changed : NULL,
 				 can_mention ? type_changed : NULL,
 				 can_mention ? new : NULL,
 				 can_mention ? unchanged : NULL,
 				 can_mention ? cchanged : NULL, i,
-				 xstrdup (name), v);
+				 &varobj_item);
 	  do_cleanups (inner);
 	}
       else
@@ -1028,11 +1038,11 @@ varobj_list_children (struct varobj *var, int *from, int *to)
 #if HAVE_PYTHON
 
 static struct varobj *
-varobj_add_child (struct varobj *var, char *name, struct value *value)
+varobj_add_child (struct varobj *var, struct varobj_item *item)
 {
-  varobj_p v = create_child_with_value (var, 
+  varobj_p v = create_child_with_value (var,
 					VEC_length (varobj_p, var->children), 
-					name, value);
+					item);
 
   VEC_safe_push (varobj_p, var->children, v);
   return v;
@@ -2098,13 +2108,17 @@ uninstall_variable (struct varobj *var)
 static struct varobj *
 create_child (struct varobj *parent, int index, char *name)
 {
-  return create_child_with_value (parent, index, name, 
-				  value_of_child (parent, index));
+  struct varobj_item item;
+
+  item.name = name;
+  item.value = value_of_child (parent, index);
+
+  return create_child_with_value (parent, index, &item);
 }
 
 static struct varobj *
-create_child_with_value (struct varobj *parent, int index, char *name,
-			 struct value *value)
+create_child_with_value (struct varobj *parent, int index,
+			 struct varobj_item *item)
 {
   struct varobj *child;
   char *childs_name;
@@ -2112,7 +2126,7 @@ create_child_with_value (struct varobj *parent, int index, char *name,
   child = new_variable ();
 
   /* NAME is allocated by caller.  */
-  child->name = name;
+  child->name = item->name;
   child->index = index;
   child->parent = parent;
   child->root = parent->root;
@@ -2120,22 +2134,22 @@ create_child_with_value (struct varobj *parent, int index, char *name,
   if (varobj_is_anonymous_child (child))
     childs_name = xstrprintf ("%s.%d_anonymous", parent->obj_name, index);
   else
-    childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
+    childs_name = xstrprintf ("%s.%s", parent->obj_name, item->name);
   child->obj_name = childs_name;
 
   install_variable (child);
 
   /* Compute the type of the child.  Must do this before
      calling install_new_value.  */
-  if (value != NULL)
+  if (item->value != NULL)
     /* If the child had no evaluation errors, var->value
        will be non-NULL and contain a valid type.  */
-    child->type = value_actual_type (value, 0, NULL);
+    child->type = value_actual_type (item->value, 0, NULL);
   else
     /* Otherwise, we must compute the type.  */
     child->type = (*child->root->lang_ops->type_of_child) (child->parent,
 							   child->index);
-  install_new_value (child, value, 1);
+  install_new_value (child, item->value, 1);
 
   return child;
 }
-- 
1.7.7.6

[PATCH 02/11] Generalize varobj iterator

[Yao Qi]

This patch generalizes varobj iterator, in a python-independent way.
Note varobj_item is still a typedef of PyObject, we can only focus on
API changes, and leave the data type changes to the next patch.  As a
result, we include "varobj-iter.h" after the typedef of PyObject in
varobj.c, but it is an intermediate state.  Finally, varobj-iter.h is
independent of PyObject.

This change is helpful to move some python-related code out of
varobj.c.

gdb:

2013-11-24  Pedro Alves  <pedro@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
	(SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
	(HFILES_NO_SRCDIR): Add "varobj-iter.h".
	(py-varobj.o): New rule.
	* python/py-varobj.c: New file.
	* python/python-internal.h (py_varobj_get_iterator): Declare.
	* varobj-iter.h: New file.
	* varobj.c: Include "varobj-iter.h"
	(struct varobj) <child_iter>: Change its type from "PyObject *"
	to "struct varobj_iter *".
	<saved_item>: Likewise.
	[HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
	[HAVE_PYTHON] (varobj_get_iterator): New function.
	(update_dynamic_varobj_children) [HAVE_PYTHON]: Move
	python-specific code to python/py-varobj.c.
	(install_visualizer): Call varobj_iter_delete instead of
	Py_XDECREF.
	* varobj.h (varobj_ensure_python_env): Declare.
---
 gdb/Makefile.in              |   13 +++-
 gdb/python/py-varobj.c       |  183 ++++++++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |    4 +
 gdb/varobj-iter.h            |   62 ++++++++++++++
 gdb/varobj.c                 |  111 ++++++++------------------
 gdb/varobj.h                 |    2 +
 6 files changed, 295 insertions(+), 80 deletions(-)
 create mode 100644 gdb/python/py-varobj.c
 create mode 100644 gdb/varobj-iter.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0591279..7d8a365 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -312,7 +312,8 @@ SUBDIR_PYTHON_OBS = \
 	py-threadevent.o \
 	py-type.o \
 	py-utils.o \
-	py-value.o
+	py-value.o \
+	py-varobj.o
 
 SUBDIR_PYTHON_SRCS = \
 	python/python.c \
@@ -348,7 +349,8 @@ SUBDIR_PYTHON_SRCS = \
 	python/py-threadevent.c \
 	python/py-type.c \
 	python/py-utils.c \
-	python/py-value.c
+	python/py-value.c \
+	python/py-varobj.c
 SUBDIR_PYTHON_DEPS =
 SUBDIR_PYTHON_LDFLAGS=
 SUBDIR_PYTHON_CFLAGS=
@@ -797,7 +799,8 @@ proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
 ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
 exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
 i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
-ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \
+ia64-tdep.h ada-lang.h ada-varobj.h varobj.h varobj-iter.h frv-tdep.h \
+nto-tdep.h serial.h \
 c-lang.h d-lang.h go-lang.h frame.h event-loop.h block.h cli/cli-setshow.h \
 cli/cli-decode.h cli/cli-cmds.h cli/cli-utils.h \
 cli/cli-script.h macrotab.h symtab.h common/version.h \
@@ -2285,6 +2288,10 @@ py-value.o: $(srcdir)/python/py-value.c
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-value.c
 	$(POSTCOMPILE)
 
+py-varobj.o: $(srcdir)/python/py-varobj.c
+	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-varobj.c
+	$(POSTCOMPILE)
+
 #
 # Dependency tracking.  Most of this is conditional on GNU Make being
 # found by configure; if GNU Make is not found, we fall back to a
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
new file mode 100644
index 0000000..6588fc2
--- /dev/null
+++ b/gdb/python/py-varobj.c
@@ -0,0 +1,183 @@
+/* Copyright (C) 2013 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/>.  */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "varobj.h"
+#include "varobj-iter.h"
+
+/* A dynamic varobj iterator "class" for python pretty-printed
+   varobjs.  This inherits struct varobj_iter.  */
+
+struct py_varobj_iter
+{
+  /* The 'base class'.  */
+  struct varobj_iter base;
+
+  /* The python iterator returned by the printer's 'children' method,
+     or NULL if not available.  */
+  PyObject *iter;
+};
+
+/* Implementation of the 'dtor' method of pretty-printed varobj
+   iterators.  */
+
+static void
+py_varobj_iter_dtor (struct varobj_iter *self)
+{
+  struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
+
+  Py_XDECREF (dis->iter);
+}
+
+/* Implementation of the 'next' method of pretty-pretty varobj
+   iterators.  */
+
+static varobj_item *
+py_varobj_iter_next (struct varobj_iter *self)
+{
+  struct py_varobj_iter *t = (struct py_varobj_iter *) self;
+  struct cleanup *back_to;
+  PyObject *item;
+
+  back_to = varobj_ensure_python_env (self->var);
+
+  item = PyIter_Next (t->iter);
+
+  if (item == NULL)
+    {
+      /* Normal end of iteration.  */
+      if (!PyErr_Occurred ())
+	return NULL;
+
+      /* If we got a memory error, just use the text as the item.  */
+      if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
+	{
+	  PyObject *type, *value, *trace;
+	  char *name_str, *value_str;
+
+	  PyErr_Fetch (&type, &value, &trace);
+	  value_str = gdbpy_exception_to_string (type, value);
+	  Py_XDECREF (type);
+	  Py_XDECREF (value);
+	  Py_XDECREF (trace);
+	  if (value_str == NULL)
+	    {
+	      gdbpy_print_stack ();
+	      return NULL;
+	    }
+
+	  name_str = xstrprintf ("<error at %d>",
+				 self->next_raw_index++);
+	  item = Py_BuildValue ("(ss)", name_str, value_str);
+	  xfree (name_str);
+	  xfree (value_str);
+	  if (item == NULL)
+	    {
+	      gdbpy_print_stack ();
+	      return NULL;
+	    }
+	}
+      else
+	{
+	  /* Any other kind of error.  */
+	  gdbpy_print_stack ();
+	  return NULL;
+	}
+    }
+
+  self->next_raw_index++;
+  do_cleanups (back_to);
+  return item;
+}
+
+/* The 'vtable' of pretty-printed python varobj iterators.  */
+
+static const struct varobj_iter_ops py_varobj_iter_ops =
+{
+  py_varobj_iter_dtor,
+  py_varobj_iter_next
+};
+
+/* Constructor of pretty-printed varobj iterators.  VAR is the varobj
+   whose children the iterator will be iterating over.  PYITER is the
+   python iterator actually responsible for the iteration.  */
+
+static void
+py_varobj_iter_ctor (struct py_varobj_iter *self,
+		      struct varobj *var, PyObject *pyiter)
+{
+  self->base.var = var;
+  self->base.ops = &py_varobj_iter_ops;
+  self->base.next_raw_index = 0;
+  self->iter = pyiter;
+}
+
+/* Allocate and construct a pretty-printed varobj iterator.  VAR is
+   the varobj whose children the iterator will be iterating over.
+   PYITER is the python iterator actually responsible for the
+   iteration.  */
+
+static struct py_varobj_iter *
+py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
+{
+  struct py_varobj_iter *self;
+
+  self = xmalloc (sizeof (struct py_varobj_iter));
+  py_varobj_iter_ctor (self, var, pyiter);
+  return self;
+}
+
+/* Return a new pretty-printed varobj iterator suitable to iterate
+   over VAR's children.  */
+
+struct varobj_iter *
+py_varobj_get_iterator (struct varobj *var, PyObject *printer)
+{
+  PyObject *children;
+  int i;
+  PyObject *iter;
+  struct py_varobj_iter *py_iter;
+  struct cleanup *back_to = varobj_ensure_python_env (var);
+
+  if (!PyObject_HasAttr (printer, gdbpy_children_cst))
+    {
+      do_cleanups (back_to);
+      return NULL;
+    }
+
+  children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
+					 NULL);
+  if (children == NULL)
+    {
+      gdbpy_print_stack ();
+      error (_("Null value returned for children"));
+    }
+
+  make_cleanup_py_decref (children);
+
+  iter = PyObject_GetIter (children);
+ if (iter == NULL)
+    {
+      gdbpy_print_stack ();
+      error (_("Could not get children iterator"));
+    }
+
+  py_iter = py_varobj_iter_new (var, iter);
+
+  do_cleanups (back_to);
+
+  return &py_iter->base;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 7d8c4ad..93f9c0a 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -494,4 +494,8 @@ int gdb_pymodule_addobject (PyObject *module, const char *name,
 			    PyObject *object)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 
+struct varobj_iter;
+struct varobj;
+struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
+					    PyObject *printer);
 #endif /* GDB_PYTHON_INTERNAL_H */
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
new file mode 100644
index 0000000..59be278
--- /dev/null
+++ b/gdb/varobj-iter.h
@@ -0,0 +1,62 @@
+/* Iterator of varobj.
+   Copyright (C) 2013 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/>.  */
+
+struct varobj_iter_ops;
+
+typedef PyObject varobj_item;
+
+/* A dynamic varobj iterator "class".  */
+
+struct varobj_iter
+{
+  /* The 'vtable'.  */
+  const struct varobj_iter_ops *ops;
+
+  /* The varobj this iterator is listing children for.  */
+  struct varobj *var;
+
+  /* The next raw index we will try to check is available.  If is
+     equal to number_of_children, then we've already iterated the
+     whole set.  */
+  int next_raw_index;
+};
+
+/* The vtable of the varobj iterator class.  */
+struct varobj_iter_ops
+{
+  /* Destructor.  Releases everything from SELF (but not SELF
+     itself).  */
+  void (*dtor) (struct varobj_iter *self);
+
+  /* Returns the next object or NULL if it has reached the end.  */
+  varobj_item *(*next) (struct varobj_iter *self);
+};
+
+/* Returns the next varobj or NULL if it has reached the end.  */
+
+#define varobj_iter_next(ITER)	(ITER)->ops->next (ITER)
+
+/* Delete a varobj_iter object.  */
+
+#define varobj_iter_delete(ITER)	       \
+  do					       \
+    {					       \
+      if (ITER)			       \
+	{				       \
+	  (ITER)->ops->dtor (ITER);	       \
+	  xfree (ITER);		       \
+	}				       \
+    } while (0)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 4a10617..7a8305b 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -41,6 +41,8 @@
 typedef int PyObject;
 #endif
 
+#include "varobj-iter.h"
+
 /* Non-zero if we want to see trace of varobj level stuff.  */
 
 unsigned int varobjdebug = 0;
@@ -139,14 +141,14 @@ struct varobj_dynamic
 
   /* The iterator returned by the printer's 'children' method, or NULL
      if not available.  */
-  PyObject *child_iter;
+  struct varobj_iter *child_iter;
 
   /* We request one extra item from the iterator, so that we can
      report to the caller whether there are more items than we have
      already reported.  However, we don't want to install this value
      when we read it, because that will mess up future updates.  So,
      we stash it here instead.  */
-  PyObject *saved_item;
+  varobj_item *saved_item;
 };
 
 struct cpstack
@@ -255,7 +257,7 @@ is_root_p (struct varobj *var)
 #ifdef HAVE_PYTHON
 /* Helper function to install a Python environment suitable for
    use during operations on VAR.  */
-static struct cleanup *
+struct cleanup *
 varobj_ensure_python_env (struct varobj *var)
 {
   return ensure_python_env (var->root->exp->gdbarch,
@@ -772,6 +774,19 @@ dynamic_varobj_has_child_method (struct varobj *var)
   return result;
 }
 
+/* Dynamic varobj's iterator factory.  Returns an iterator object
+   suitable for iterating over VAR's children.  */
+
+static struct varobj_iter *
+varobj_get_iterator (struct varobj *var)
+{
+  if (var->dynamic->pretty_printer)
+    return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
+
+  gdb_assert_not_reached (_("\
+requested an interator from a non-dynamic varobj"));
+}
+
 #endif
 
 static int
@@ -787,9 +802,7 @@ update_dynamic_varobj_children (struct varobj *var,
 {
 #if HAVE_PYTHON
   struct cleanup *back_to;
-  PyObject *children;
   int i;
-  PyObject *printer = var->dynamic->pretty_printer;
 
   if (!gdb_python_initialized)
     return 0;
@@ -797,37 +810,22 @@ update_dynamic_varobj_children (struct varobj *var,
   back_to = varobj_ensure_python_env (var);
 
   *cchanged = 0;
-  if (!PyObject_HasAttr (printer, gdbpy_children_cst))
-    {
-      do_cleanups (back_to);
-      return 0;
-    }
 
   if (update_children || var->dynamic->child_iter == NULL)
     {
-      children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
-					     NULL);
+      varobj_iter_delete (var->dynamic->child_iter);
+      var->dynamic->child_iter = varobj_get_iterator (var);
 
-      if (!children)
-	{
-	  gdbpy_print_stack ();
-	  error (_("Null value returned for children"));
-	}
+      Py_XDECREF (var->dynamic->saved_item);
+      var->dynamic->saved_item = NULL;
 
-      make_cleanup_py_decref (children);
+      i = 0;
 
-      Py_XDECREF (var->dynamic->child_iter);
-      var->dynamic->child_iter = PyObject_GetIter (children);
       if (var->dynamic->child_iter == NULL)
 	{
-	  gdbpy_print_stack ();
-	  error (_("Could not get children iterator"));
+	  do_cleanups (back_to);
+	  return 0;
 	}
-
-      Py_XDECREF (var->dynamic->saved_item);
-      var->dynamic->saved_item = NULL;
-
-      i = 0;
     }
   else
     i = VEC_length (varobj_p, var->children);
@@ -837,7 +835,6 @@ update_dynamic_varobj_children (struct varobj *var,
   for (; to < 0 || i < to + 1; ++i)
     {
       PyObject *item;
-      int force_done = 0;
 
       /* See if there was a leftover from last time.  */
       if (var->dynamic->saved_item)
@@ -846,52 +843,17 @@ update_dynamic_varobj_children (struct varobj *var,
 	  var->dynamic->saved_item = NULL;
 	}
       else
-	item = PyIter_Next (var->dynamic->child_iter);
-
-      if (!item)
 	{
-	  /* Normal end of iteration.  */
-	  if (!PyErr_Occurred ())
-	    break;
-
-	  /* If we got a memory error, just use the text as the
-	     item.  */
-	  if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
-	    {
-	      PyObject *type, *value, *trace;
-	      char *name_str, *value_str;
-
-	      PyErr_Fetch (&type, &value, &trace);
-	      value_str = gdbpy_exception_to_string (type, value);
-	      Py_XDECREF (type);
-	      Py_XDECREF (value);
-	      Py_XDECREF (trace);
-	      if (!value_str)
-		{
-		  gdbpy_print_stack ();
-		  break;
-		}
-
-	      name_str = xstrprintf ("<error at %d>", i);
-	      item = Py_BuildValue ("(ss)", name_str, value_str);
-	      xfree (name_str);
-	      xfree (value_str);
-	      if (!item)
-		{
-		  gdbpy_print_stack ();
-		  break;
-		}
-
-	      force_done = 1;
-	    }
-	  else
-	    {
-	      /* Any other kind of error.  */
-	      gdbpy_print_stack ();
-	      break;
-	    }
+	  item = varobj_iter_next (var->dynamic->child_iter);
 	}
 
+      if (item == NULL)
+	{
+	  /* Iteration is done.  Remove iterator from VAR.  */
+	  varobj_iter_delete (var->dynamic->child_iter);
+	  var->dynamic->child_iter = NULL;
+	  break;
+	}
       /* We don't want to push the extra child on any report list.  */
       if (to < 0 || i < to)
 	{
@@ -931,9 +893,6 @@ update_dynamic_varobj_children (struct varobj *var,
 	     element.  */
 	  break;
 	}
-
-      if (force_done)
-	break;
     }
 
   if (i < VEC_length (varobj_p, var->children))
@@ -952,8 +911,6 @@ update_dynamic_varobj_children (struct varobj *var,
     *cchanged = 1;
 
   var->num_children = VEC_length (varobj_p, var->children);
- 
-  do_cleanups (back_to);
 
   return 1;
 #else
@@ -1244,7 +1201,7 @@ install_visualizer (struct varobj_dynamic *var, PyObject *constructor,
   Py_XDECREF (var->pretty_printer);
   var->pretty_printer = visualizer;
 
-  Py_XDECREF (var->child_iter);
+  varobj_iter_delete (var->child_iter);
   var->child_iter = NULL;
 }
 
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 978d9b9..0f68311 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -308,6 +308,8 @@ extern int varobj_has_more (struct varobj *var, int to);
 
 extern int varobj_pretty_printed_p (struct varobj *var);
 
+extern  struct cleanup *varobj_ensure_python_env (struct varobj *var);
+
 extern int varobj_default_value_is_changeable_p (struct varobj *var);
 extern int varobj_value_is_changeable_p (struct varobj *var);
 
-- 
1.7.7.6

[PATCH 08/11] Iterator varobj_items by their availability

[Yao Qi]

This patch adds a new implementation of varobj iterator, which is
based on the availability of children.

gdb:

2013-10-24  Pedro Alves  <pedro@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SFILES): Add varobj-iter-avail.c.
	(COMMON_OBS): Add varobj-iter-avail.o.
	* varobj-iter-avail.c: New file.
	* varobj-iter.h (avail_varobj_get_iterator): Declare.
	* varobj.c (dynamic_varobj_has_child_method):
	(varobj_get_iterator): Add one more argument 'lang_ops' and
	Caller update.
	Return iterator for available children.
---
 gdb/Makefile.in         |    4 +-
 gdb/varobj-iter-avail.c |  160 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/varobj-iter.h       |    4 +
 gdb/varobj.c            |    8 ++-
 4 files changed, 172 insertions(+), 4 deletions(-)
 create mode 100644 gdb/varobj-iter-avail.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 7d8a365..6d84def 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -772,7 +772,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	ui-out.c utils.c ui-file.h ui-file.c \
 	user-regs.c \
 	valarith.c valops.c valprint.c value.c varobj.c common/vec.c \
-	xml-tdesc.c xml-support.c \
+	varobj-iter-avail.c xml-tdesc.c xml-support.c \
 	inferior.c gdb_usleep.c \
 	record.c record-full.c gcore.c \
 	jit.c \
@@ -933,7 +933,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	ada-lang.o c-lang.o d-lang.o f-lang.o objc-lang.o \
 	ada-tasks.o ada-varobj.o c-varobj.o \
 	ui-out.o cli-out.o \
-	varobj.o vec.o \
+	varobj.o varobj-iter-avail.o vec.o \
 	go-lang.o go-valprint.o go-typeprint.o \
 	jv-lang.o jv-valprint.o jv-typeprint.o jv-varobj.o \
 	m2-lang.o opencl-lang.o p-lang.o p-typeprint.o p-valprint.o \
diff --git a/gdb/varobj-iter-avail.c b/gdb/varobj-iter-avail.c
new file mode 100644
index 0000000..0cd2e23
--- /dev/null
+++ b/gdb/varobj-iter-avail.c
@@ -0,0 +1,160 @@
+/* Copyright (C) 2013 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/>.  */
+
+#include "defs.h"
+#include "gdb_assert.h"
+#include "value.h"
+#include "valprint.h"
+#include "varobj.h"
+#include "varobj-iter.h"
+
+/* A dynamic varobj iterator "class" for available-children-only
+   varobjs.  This inherits struct varobj_iter.  */
+
+struct avail_varobj_iter
+{
+  /* The 'base class'.  */
+  struct varobj_iter base;
+
+  const struct lang_varobj_ops *lang_ops;
+};
+
+/* Returns true if VAL is not interesting for an
+   available-children-only varobj.  */
+
+static int
+varobj_value_unavailable (struct value *val)
+{
+  volatile struct gdb_exception e;
+  int unavail = 0;
+
+  gdb_assert (val != NULL);
+
+  /* value_entirely_unavailable may need to try reading the value,
+     which may throw for example, a memory error, e.g., when not
+     inspecting a traceframe, we try to read the pointee of a dangling
+     or NULL pointer.  */
+  TRY_CATCH (e, RETURN_MASK_ERROR)
+    {
+      struct type *type = value_type (val);
+
+      unavail = (value_entirely_unavailable (val)
+		 /* A scalar object that does not have all bits available
+		    is also considered unavailable, because all bits
+		    contribute to its representation.  */
+		 || (val_print_scalar_type_p (type)
+		     && !value_bytes_available (val,
+						value_embedded_offset (val),
+						TYPE_LENGTH (type))));
+    }
+  if (e.reason < 0)
+    return 1;
+  else
+    return unavail;
+}
+
+/* Implementation of the `next' method for available-children-only
+   varobj iterators.  */
+
+static varobj_item *
+avail_varobj_iter_next (struct varobj_iter *self)
+{
+  struct avail_varobj_iter *dis = (struct avail_varobj_iter *) self;
+  int num_children = dis->lang_ops->number_of_children (self->var);
+  int raw_index = self->next_raw_index;
+  varobj_item *item = NULL;
+
+  for (; raw_index < num_children; raw_index++)
+    {
+      struct value *child_value
+	= dis->lang_ops->value_of_child (self->var, raw_index);
+
+      /* The "fake" children will have NULL values.  */
+      if (child_value == NULL || !varobj_value_unavailable (child_value))
+	{
+	  item = xmalloc (sizeof *item);
+
+	  item->name
+	    = dis->lang_ops->name_of_child (self->var, raw_index);
+	  item->value = child_value;
+
+	  raw_index++;
+	  self->next_raw_index = raw_index;
+	  break;
+	}
+    }
+  return item;
+}
+
+/* Implementation of the `dtor' method of available-children-only
+   varobj iterators.  */
+
+static void
+avail_varobj_iter_dtor (struct varobj_iter *self)
+{
+  /* nothing to do */
+}
+
+/* The 'vtable' of available-children-only varobj iterators.  */
+
+static const struct varobj_iter_ops avail_varobj_iter_ops =
+{
+  avail_varobj_iter_dtor,
+  avail_varobj_iter_next
+};
+
+/* Constructor of available-children-only varobj iterators.  VAR is
+   the varobj whose children the iterator will be iterating over.  */
+
+static void
+avail_varobj_iter_ctor (struct avail_varobj_iter *self, struct varobj *var,
+			const struct lang_varobj_ops *lang_ops)
+{
+  self->base.var = var;
+  self->base.ops = &avail_varobj_iter_ops;
+  self->base.next_raw_index = 0;
+  self->lang_ops = lang_ops;
+}
+/* Allocate and construct an available-children-only varobj iterator.
+   VAR is the varobj whose children the iterator will be iterating
+   over.  */
+
+static struct avail_varobj_iter *
+avail_varobj_iter_new (struct varobj *var, const struct lang_varobj_ops *lang_ops)
+{
+  struct avail_varobj_iter *self;
+
+  self = xmalloc (sizeof (struct avail_varobj_iter));
+  avail_varobj_iter_ctor (self, var, lang_ops);
+  return self;
+}
+
+/* Return a new available-children-only varobj iterator suitable to
+   iterate over VAR's children.  */
+
+struct varobj_iter *
+avail_varobj_get_iterator (struct varobj *var,
+			   const struct lang_varobj_ops *lang_ops)
+{
+  struct avail_varobj_iter *iter;
+
+  /* Avoid the needless allocation/dealocation of the iterator if
+     there are no raw children to iterator over anyway.  */
+  if (lang_ops->number_of_children (var) == 0)
+    return NULL;
+
+  iter = avail_varobj_iter_new (var, lang_ops);
+  return &iter->base;
+}
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
index b4bda82..c466225 100644
--- a/gdb/varobj-iter.h
+++ b/gdb/varobj-iter.h
@@ -68,3 +68,7 @@ struct varobj_iter_ops
 	  xfree (ITER);		       \
 	}				       \
     } while (0)
+
+struct varobj_iter *
+  avail_varobj_get_iterator (struct varobj *var,
+			     const struct lang_varobj_ops *lang_ops);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 1fd9fd2..ba93eb5 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -768,8 +768,11 @@ dynamic_varobj_has_child_method (struct varobj *var)
    suitable for iterating over VAR's children.  */
 
 static struct varobj_iter *
-varobj_get_iterator (struct varobj *var)
+varobj_get_iterator (struct varobj *var, const struct lang_varobj_ops *lang_ops)
 {
+  if (var->dynamic->available_children_only)
+    return avail_varobj_get_iterator (var, lang_ops);
+
 #if HAVE_PYTHON
   if (var->dynamic->pretty_printer)
     return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
@@ -810,7 +813,8 @@ update_dynamic_varobj_children (struct varobj *var,
   if (update_children || var->dynamic->child_iter == NULL)
     {
       varobj_iter_delete (var->dynamic->child_iter);
-      var->dynamic->child_iter = varobj_get_iterator (var);
+      var->dynamic->child_iter = varobj_get_iterator (var,
+						      var->root->lang_ops);
 
       varobj_clear_saved_item (var->dynamic);
 
-- 
1.7.7.6

[PATCH 11/11] Test case

[Yao Qi]

This patch adds two test cases for the new option.
mi-available-children-only-cxx.exp focuses on the C++ fake children.

gdb/testsuite:

2013-11-24  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/available-children-only.c: New.
	* gdb.trace/mi-available-children-only.exp: New.
	* gdb.trace/mi-available-children-only-cxx.exp: New.
	* gdb.trace/available-children-only.cc: New.
---
 gdb/testsuite/gdb.trace/available-children-only.c  |   69 +++++++
 gdb/testsuite/gdb.trace/available-children-only.cc |   45 +++++
 .../gdb.trace/mi-available-children-only-cxx.exp   |  126 +++++++++++++
 .../gdb.trace/mi-available-children-only.exp       |  198 ++++++++++++++++++++
 4 files changed, 438 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.trace/available-children-only.c
 create mode 100644 gdb/testsuite/gdb.trace/available-children-only.cc
 create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
 create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only.exp

diff --git a/gdb/testsuite/gdb.trace/available-children-only.c b/gdb/testsuite/gdb.trace/available-children-only.c
new file mode 100644
index 0000000..1697a8b
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/available-children-only.c
@@ -0,0 +1,69 @@
+/* Copyright 2013 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/>.  */
+
+/* Two trace actions are set, and they collect different fields to
+   reproduce the case that children appear in the different orders.  */
+
+struct simple
+{
+  int a; /* Collected by both.  */
+  int b; /* Collected by action 2.  */
+  struct
+  {
+    struct
+    {
+      int g; /* Collected by action 1.  */
+      int h; /* Collected by action 2.  */
+    } s3;
+    int d; /* Collected by action 1.  */
+  } s1;
+
+  struct
+  {
+    int e;
+    int f; /* Collected by action 1.  */
+  } s2;
+};
+
+struct simple s;
+
+static void
+marker1 (void)
+{
+}
+
+static void
+marker2 (void)
+{
+}
+
+static void
+end (void)
+{
+}
+
+int
+main (void)
+{
+  marker1 ();
+
+  marker2 ();
+
+  end ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/available-children-only.cc b/gdb/testsuite/gdb.trace/available-children-only.cc
new file mode 100644
index 0000000..2ff4a4c
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/available-children-only.cc
@@ -0,0 +1,45 @@
+/* Copyright 2013 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/>.  */
+
+class Fake
+{
+ private:
+  int pri1; /* Collected.  */
+
+ public:
+  int pub1;
+
+};
+
+Fake fake;
+
+static void
+marker (void)
+{}
+
+static void
+end (void)
+{}
+
+int
+main (void)
+{
+  marker ();
+
+  end ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp b/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
new file mode 100644
index 0000000..f72a13f
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
@@ -0,0 +1,126 @@
+# Copyright 2013 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/>.
+
+# Test for C++ fake children.
+
+load_lib mi-support.exp
+load_lib trace-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile available-children-only.cc
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	   executable {debug c++}] != "" } {
+    untested "Couldn't compile ${srcfile}"
+    return -1
+}
+
+# Test target supports tracepoints or not.
+
+clean_restart $testfile
+
+if ![runto_main] {
+    fail "Can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "Current target does not support trace"
+    return -1
+}
+gdb_exit
+
+if [mi_gdb_start] {
+    continue
+}
+
+mi_run_to_main
+
+mi_gdb_test "-break-insert -a marker" "\\^done.*" \
+    "trace marker"
+
+mi_gdb_test "-break-commands 2 \"collect fake.pri1\" \"end\" " \
+    {\^done} "set action on marker"
+
+mi_gdb_test "-trace-start" ".*\\^done.*" "trace start"
+mi_continue_to end
+mi_gdb_test "-trace-stop" "\\^done.*" "trace stop"
+# Save trace frames to trace file.
+set tracefile [standard_output_file ${testfile}]
+mi_gdb_test "-trace-save ${tracefile}.tfile" \
+    ".*" \
+    "save tfile trace"
+
+# In live target, '--available-children-only' shouldn't have any
+# effects.
+
+mi_gdb_test "-trace-find frame-number 0" \
+    ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+    "-trace-find frame-number 0"
+
+mi_gdb_test "-var-create --available-children-only fake @ fake" \
+    {\^done,name="fake",numchild="0",value=".*",type="Fake",dynamic="1",has_more="1"} \
+    "-var-create --available-children-only fake"
+
+mi_list_varobj_children { fake --available-children-only } {
+    { fake.public public 0 }
+    { fake.private private 0 }
+} "-var-list-children --available-children-only fake"
+
+mi_gdb_test "-var-info-num-children --available-children-only fake" \
+    "\\^done,numchild=\"2\"" \
+    "-var-info-num-children --available-children-only fake"
+
+mi_gdb_test "-var-delete fake" {\^done,ndeleted="3"} "-var-delete fake"
+
+# Select a traceframe, and '--available-children-only' have some
+# effects.
+
+proc check_with_traceframe { } {
+    global decimal
+
+    with_test_prefix "w/ setting traceframe" {
+	mi_gdb_test "-trace-find frame-number 0" \
+	    ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+	    "-trace-find frame-number 0"
+
+	mi_gdb_test "-var-create --available-children-only fake @ fake" \
+	    {\^done,name="fake",numchild="0",value=".*",type="Fake",dynamic="1",has_more="1"} \
+	    "-var-create --available-children-only fake"
+
+	mi_list_varobj_children "fake" {
+	    { fake.public public 1 }
+	    { fake.private private 1 }
+	} "-var-list-children fake"
+
+	mi_gdb_test "-var-info-num-children fake" \
+	    "\\^done,numchild=\"2\"" "-var-info-num-children fake"
+
+	mi_list_varobj_children {"fake" "--available-children-only" } {
+	    { fake.public public 0 }
+	    { fake.private private 0 }
+	} "-var-list-children --available-children-only fake"
+
+	mi_gdb_test "-var-info-num-children --available-children-only fake" \
+	    "\\^done,numchild=\"2\"" \
+	    "-var-info-num-children --available-children-only fake"
+
+	mi_gdb_test "-var-delete fake" {\^done,ndeleted="3"} "-var-delete fake"
+    }
+    mi_gdb_test "-trace-find none" ".*\\^done,found=\"0\".*" \
+	"-trace-find none"
+}
+
+check_with_traceframe
diff --git a/gdb/testsuite/gdb.trace/mi-available-children-only.exp b/gdb/testsuite/gdb.trace/mi-available-children-only.exp
new file mode 100644
index 0000000..73f85d7
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/mi-available-children-only.exp
@@ -0,0 +1,198 @@
+# Copyright 2013 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/>.
+
+load_lib mi-support.exp
+load_lib trace-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile available-children-only.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested mi-available-children-only.exp
+    return -1
+}
+
+# Test target supports tracepoints or not.
+
+clean_restart $testfile
+
+if ![runto_main] {
+    fail "Can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "Current target does not support trace"
+    return -1
+}
+gdb_exit
+
+if [mi_gdb_start] {
+    continue
+}
+
+mi_run_to_main
+mi_gdb_test "-break-insert -a marker1" "\\^done.*" \
+    "trace marker1"
+
+mi_gdb_test "-break-commands 2 \"collect s.a\" \"collect s.s1.d\" \"collect s.s1.s3.g\" \"collect s.s2.f\" \"end\" " \
+    {\^done} "set action on marker1"
+
+mi_gdb_test "-break-insert -a marker2" "\\^done.*" \
+    "trace marker2"
+
+mi_gdb_test "-break-commands 3 \"collect s.a\" \"collect s.b\" \"collect s.s1.s3.h\" \"end\" " \
+    {\^done} "set action on marker2"
+
+mi_gdb_test "-trace-start" ".*\\^done.*" "trace start"
+mi_continue_to end
+mi_gdb_test "-trace-stop" "\\^done.*" "trace stop"
+
+# Save trace frames to trace file.
+set tracefile [standard_output_file ${testfile}]
+mi_gdb_test "-trace-save ${tracefile}.tfile" \
+    ".*" \
+    "save tfile trace"
+
+# In live target, '--available-children-only' shouldn't have any
+# effects.
+mi_gdb_test "-var-create --available-children-only s1 @ s" \
+    {\^done,name="s1",numchild="0",value=".*",type="struct simple",dynamic="1",has_more="1"} \
+    "-var-create --available-children-only s1"
+
+mi_list_varobj_children  { "s1" "--available-children-only" } {
+    { s1.a a 0 int }
+    { s1.b b 0 int }
+    { s1.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+    { s1.s2 s2 0 "struct \\{\\.\\.\\.\\}" }
+} "-var-list-children --available-children-only s1"
+
+mi_gdb_test "-var-info-num-children --available-children-only s1" \
+    "\\^done,numchild=\"4\"" \
+    "-var-info-num-children --available-children-only s1"
+
+mi_gdb_test "-var-delete s1" {\^done,ndeleted="5"} "-var-delete s1"
+
+# Select a traceframe, and '--available-children-only' have some
+# effects.
+
+proc check_with_traceframe { } {
+    global decimal
+
+    mi_gdb_test "-trace-find frame-number 0" \
+	".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+	"-trace-find frame-number 0"
+
+    with_test_prefix "traceframe 0" {
+	mi_gdb_test "-var-create --available-children-only s2 @ s" \
+	    {\^done,name="s2",numchild="0",value=".*",type="struct simple",dynamic="1",has_more="1"} \
+	    "-var-create --available-children-only s2"
+
+	mi_gdb_test "-var-list-children s2" \
+	    "\\^done,numchild=\"4\",.*,has_more=\"0\"" \
+	    "-var-list-children s2"
+
+	mi_gdb_test "-var-info-num-children s2" \
+	    "\\^done,numchild=\"4\"" \
+	    "-var-info-num-children s2"
+
+	# "s2" should have children "a", "s1" and "s2".
+	mi_list_varobj_children  { "s2" "--available-children-only" } {
+	    { s2.a a 0 int }
+	    { s2.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+	    { s2.s2 s2 0 "struct \\{\\.\\.\\.\\}" }
+	} "-var-list-children --available-children-only s2"
+
+	mi_gdb_test "-var-info-num-children --available-children-only s2" \
+	    "\\^done,numchild=\"3\"" \
+	    "-var-info-num-children --available-children-only s2"
+
+	mi_list_varobj_children  { "s2.s1" "--available-children-only" } {
+	    { s2.s1.s3 s3 0 "struct \\{\\.\\.\\.\\}" }
+	    { s2.s1.d d 0 int }
+	} "-var-list-children --available-children-only s2.s1"
+
+    }
+
+    mi_gdb_test "-trace-find frame-number 1" \
+	".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"1\",frame=\{.*" \
+	"-trace-find frame-number 1"
+
+    with_test_prefix "traceframe 1" {
+	mi_list_varobj_children  { "s2" "--available-children-only" } {
+	    { s2.a a 0 int }
+	    { s2.b b 0 int }
+	    { s2.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+	} "-var-list-children --available-children-only s2"
+
+	mi_list_varobj_children  { "s2.s1" "--available-children-only" } {
+	    { s2.s1.s3 s3 0 "struct \\{\\.\\.\\.\\}" }
+	} "-var-list-children --available-children-only s2.s1"
+
+	mi_list_varobj_children  { "s2.s1.s3" "--available-children-only" } {
+	    { s2.s1.s3.h h 0 int }
+	} "-var-list-children --available-children-only s2.s1.s3"
+
+	mi_gdb_test "-var-info-num-children --available-children-only s2" \
+	    "\\^done,numchild=\"3\"" \
+	    "-var-info-num-children --available-children-only s2"
+
+	mi_gdb_test "-var-delete s2" {\^done,ndeleted="6"} "-var-delete s2"
+    }
+
+    mi_gdb_test "-trace-find none" ".*\\^done,found=\"0\".*" \
+	"-trace-find none"
+}
+
+check_with_traceframe
+
+# Test when changing target from remote to ${target}.
+
+proc test_from_remote { target } {
+    global mi_gdb_prompt decimal
+    global tracefile
+
+    with_test_prefix "remote to ${target}" {
+	# Change target to ${target}.
+	mi_gdb_test "-target-select ${target} ${tracefile}.${target}" ".*\\^connected.*" \
+	    "change target to ${target}"
+
+	with_test_prefix "w/ setting traceframe" {
+	    check_with_traceframe
+	}
+    }
+}
+
+test_from_remote "tfile"
+
+proc test_from_exec { target } {
+    global binfile
+    global tracefile
+
+    mi_gdb_exit
+    if [mi_gdb_start] {
+	return
+    }
+    mi_gdb_load ${binfile}
+
+    with_test_prefix "exec to ${target}" {
+	mi_gdb_test "-target-select ${target} ${tracefile}.${target}" ".*\\^connected.*" \
+	    "change target to ${target}"
+
+	check_with_traceframe
+    }
+}
+
+test_from_exec "tfile"
-- 
1.7.7.6

[PATCH 04/11] Remove #if HAVE_PYTHON

[Yao Qi]

This patch removes some unnecessary "#if HAVE_PYTHON" so that more
code is generalized.

gdb:

2013-11-24  Pedro Alves  <pedro@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* varobj.c: Remove "#if HAVE_PYTHON" and "#endif".
	(varobj_get_iterator): Wrap up code for pretty-printer by
	"#if HAVE_PYTHON" and "#endif".
	(update_dynamic_varobj_children): Likewise.
---
 gdb/varobj.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/gdb/varobj.c b/gdb/varobj.c
index 28e934c..e362229 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -212,13 +212,9 @@ static char *my_value_of_variable (struct varobj *var,
 
 static int is_root_p (struct varobj *var);
 
-#if HAVE_PYTHON
-
 static struct varobj *varobj_add_child (struct varobj *var,
 					struct varobj_item *item);
 
-#endif /* HAVE_PYTHON */
-
 /* Private data */
 
 /* Mappings of varobj_display_formats enums to gdb's format codes.  */
@@ -701,8 +697,6 @@ varobj_restrict_range (VEC (varobj_p) *children, int *from, int *to)
     }
 }
 
-#if HAVE_PYTHON
-
 /* A helper for update_dynamic_varobj_children that installs a new
    child when needed.  */
 
@@ -747,6 +741,8 @@ install_dynamic_child (struct varobj *var,
     }
 }
 
+#if HAVE_PYTHON
+
 static int
 dynamic_varobj_has_child_method (struct varobj *var)
 {
@@ -762,6 +758,7 @@ dynamic_varobj_has_child_method (struct varobj *var)
   do_cleanups (back_to);
   return result;
 }
+#endif
 
 /* Dynamic varobj's iterator factory.  Returns an iterator object
    suitable for iterating over VAR's children.  */
@@ -769,8 +766,10 @@ dynamic_varobj_has_child_method (struct varobj *var)
 static struct varobj_iter *
 varobj_get_iterator (struct varobj *var)
 {
+#if HAVE_PYTHON
   if (var->dynamic->pretty_printer)
     return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
+#endif
 
   gdb_assert_not_reached (_("\
 requested an interator from a non-dynamic varobj"));
@@ -788,7 +787,6 @@ varobj_clear_saved_item (struct varobj_dynamic *var)
       var->saved_item = NULL;
     }
 }
-#endif
 
 static int
 update_dynamic_varobj_children (struct varobj *var,
@@ -801,7 +799,6 @@ update_dynamic_varobj_children (struct varobj *var,
 				int from,
 				int to)
 {
-#if HAVE_PYTHON
   int i;
 
   *cchanged = 0;
@@ -891,9 +888,6 @@ update_dynamic_varobj_children (struct varobj *var,
   var->num_children = VEC_length (varobj_p, var->children);
 
   return 1;
-#else
-  gdb_assert_not_reached ("should never be called if Python is not enabled");
-#endif
 }
 
 int
@@ -970,8 +964,6 @@ varobj_list_children (struct varobj *var, int *from, int *to)
   return var->children;
 }
 
-#if HAVE_PYTHON
-
 static struct varobj *
 varobj_add_child (struct varobj *var, struct varobj_item *item)
 {
@@ -983,8 +975,6 @@ varobj_add_child (struct varobj *var, struct varobj_item *item)
   return v;
 }
 
-#endif /* HAVE_PYTHON */
-
 /* Obtain the type of an object Variable as a string similar to the one gdb
    prints on the console.  */
 
-- 
1.7.7.6

[PATCH 09/11] Delete varobj's children on traceframe is changed.

[Yao Qi]

Hi,
The memory availability varies on trace frames.  When
--available-children-only is used, the varobj tree structure changes
when trace frame is changed.  GDB has to remove varobj's children if
it is marked as 'available_children_only'.  For example, in traceframe
1, foo.a and foo.c is collected, and in traceframe 2, foo.b is
collected,

struct foo
{
  int a; /* Collected in traceframe 1 */
  int b; /* Collected in traceframe 2 */
  int c; /* Collected in traceframe 1 */
};

When available-children-only is used, the expected result is that in
traceframe 1, foo has two children (a and c), and foo has one child
(b) in traceframe 2.  Without this patch, foo has a, b, and c in
traceframe 2, which is wrong.

In this patch, we install a traceframe_changed observer to clear
varobjs marked as 'available_children_only'.

gdb:

2013-11-24  Yao Qi  <yao@codesourcery.com>

	* varobj.c: Include "observer.h".
	(varobj_delete_if_available_children_only): New function.
	(varobj_traceframe_changed): New function.
	(_initialize_varobj): Install varobj_traceframe_changed to
	traceframe_changed observer.
---
 gdb/varobj.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/gdb/varobj.c b/gdb/varobj.c
index ba93eb5..4b201df 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -24,6 +24,7 @@
 #include "gdbcmd.h"
 #include "block.h"
 #include "valprint.h"
+#include "observer.h"
 
 #include "gdb_assert.h"
 #include <string.h>
@@ -2749,6 +2750,32 @@ all_root_varobjs (void (*func) (struct varobj *var, void *data), void *data)
       (*func) (var_root->rootvar, data);
     }
 }
+
+/* Delete VAR's children if it is marked as 'available_children_only'.  */
+
+static void
+varobj_delete_if_available_children_only (struct varobj *var, void *data)
+{
+  if (var->dynamic->available_children_only)
+    {
+      varobj_delete (var, NULL, /* children only */ 1);
+      var->num_children = -1;
+
+      /* We're starting over, so get rid of any iterator.  */
+      varobj_iter_delete (var->dynamic->child_iter);
+      var->dynamic->child_iter = NULL;
+      varobj_clear_saved_item (var->dynamic);
+    }
+}
+
+/* Installed on traceframe_changed observer.  */
+
+static void
+varobj_traceframe_changed (int tfnum, int tpnum)
+{
+  all_root_varobjs (varobj_delete_if_available_children_only , NULL);
+}
+
 \f
 extern void _initialize_varobj (void);
 void
@@ -2766,6 +2793,8 @@ _initialize_varobj (void)
 			     _("When non-zero, varobj debugging is enabled."),
 			     NULL, show_varobjdebug,
 			     &setlist, &showlist);
+
+  observer_attach_traceframe_changed (varobj_traceframe_changed);
 }
 
 /* Invalidate varobj VAR if it is tied to locals and re-create it if it is
-- 
1.7.7.6

[RCF 00/11] Visit varobj available children only in MI

[Yao Qi]

Hi,
This patch series proposes a feature that GDB is able to visit varobj
available children only, by adding a new option
"--available-children-only" to commands -var-create,
-var-info-num-children and -var-list-children.  This patch series was
written by Pedro in one patch.  I read the patch three months ago, split
it, add some minor things, and write test cases.  I wish I didn't
break anything, including the rationale and the implementation :)

In each traceframe, with option --available-children-only, the
children of varobj vary, so it behaves like a dynamic varobj.
Since the key of dynamic varobj is the iterator, which can be used to
visit each child.  We need a new iterator for varobj with
--available-children-only.

Current dynamic varobj is python pretty-printer specific, so the
iterator is python specific too.  In order to add a new type of iterator,
we have to generalize iterator and de-couple it from python.  At
present, iteration is performed against PyObject, but it can
generalized to a name-value pair.  That is what patch #1 ~ #3 do.
Note that patch #2 and #3 can be a single commit, but I split it for
review.  Ideally, dynamic varobj can be a generic stuff, IMO.

After the changes in patch #1 ~ #3, we find some code are not
python-specific, so #if HAVE_PYTHON can be removed.  It is done by
patch #4.

Since we think dynamic varobj is no longer python specific, we'd
better rename predicate varobj_pretty_printed_p to varobj_is_dynamic_p,
and use it more widely (done by patch #5 #6).  It paves a way for
the next patches to add a new type of dynamic varobj.

Patch #7 adds the option --available-children-only in MI code, patch
#8 adds the corresponding iterator, and patch #9 updates varobj children
when traceframe is changed.  These three should be in one commit.
Note that there is at most one iterator can be applied to one varobj.
If pretty-printer is installed and option --available-children-only
is used, GDB has to return one iterator, and we choose iterator
for available-children-only.
 
Patch #10 and #11 are about test cases.  Patch #10 is to teach
testsuite to match dynamic="1", and patch #11 is the test case for
option --available-children-only.

I don't include NEWS and doc patch in this series, because we'd like
to discuss on "whether we can call varobj with --available-children-only
a dynamic varobj".  The result affects the doc, IMO.  Although we
implemented varobj with --available-children-only as a dynamic varobj,
I am not sure we can do the same in doc, which is user visible.

The whole series is tested on x86_64-linux.

*** BLURB HERE ***

Yao Qi (11):
  Use 'struct varobj_item' to represent name and value pair
  Generalize varobj iterator
  Iterate over 'struct varobj_item' instead of PyObject
  Remove #if HAVE_PYTHON
  Rename varobj_pretty_printed_p to varobj_is_dynamic_p
  Use varobj_is_dynamic_p more widely
  MI option --available-children-only
  Iterator varobj_items by their availability
  Delete varobj's children on traceframe is changed.
  Match dynamic="1" in the output of -var-list-children
  Test case

 gdb/Makefile.in                                    |   17 +-
 gdb/mi/mi-cmd-var.c                                |  134 +++++++--
 gdb/python/py-varobj.c                             |  198 ++++++++++++
 gdb/python/python-internal.h                       |    4 +
 gdb/testsuite/gdb.trace/available-children-only.c  |   69 +++++
 gdb/testsuite/gdb.trace/available-children-only.cc |   45 +++
 .../gdb.trace/mi-available-children-only-cxx.exp   |  126 ++++++++
 .../gdb.trace/mi-available-children-only.exp       |  198 ++++++++++++
 gdb/testsuite/lib/mi-support.exp                   |   16 +-
 gdb/varobj-iter-avail.c                            |  160 ++++++++++
 gdb/varobj-iter.h                                  |   74 +++++
 gdb/varobj.c                                       |  321 ++++++++++----------
 gdb/varobj.h                                       |   15 +-
 13 files changed, 1183 insertions(+), 194 deletions(-)
 create mode 100644 gdb/python/py-varobj.c
 create mode 100644 gdb/testsuite/gdb.trace/available-children-only.c
 create mode 100644 gdb/testsuite/gdb.trace/available-children-only.cc
 create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
 create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only.exp
 create mode 100644 gdb/varobj-iter-avail.c
 create mode 100644 gdb/varobj-iter.h

-- 
1.7.7.6

Re: [RCF 00/11] Visit varobj available children only in MI

[Yao Qi]

On 11/24/2013 10:09 AM, Yao Qi wrote:
> Hi,
> This patch series proposes a feature that GDB is able to visit varobj
> available children only, by adding a new option
> "--available-children-only" to commands -var-create,
> -var-info-num-children and -var-list-children.  This patch series was
> written by Pedro in one patch.  I read the patch three months ago, split
> it, add some minor things, and write test cases.  I wish I didn't
> break anything, including the rationale and the implementation :)
>
> In each traceframe, with option --available-children-only, the
> children of varobj vary, so it behaves like a dynamic varobj.
> Since the key of dynamic varobj is the iterator, which can be used to
> visit each child.  We need a new iterator for varobj with
> --available-children-only.
>
> Current dynamic varobj is python pretty-printer specific, so the
> iterator is python specific too.  In order to add a new type of iterator,
> we have to generalize iterator and de-couple it from python.  At
> present, iteration is performed against PyObject, but it can
> generalized to a name-value pair.  That is what patch #1 ~ #3 do.
> Note that patch #2 and #3 can be a single commit, but I split it for
> review.  Ideally, dynamic varobj can be a generic stuff, IMO.
>
> After the changes in patch #1 ~ #3, we find some code are not
> python-specific, so #if HAVE_PYTHON can be removed.  It is done by
> patch #4.
>
> Since we think dynamic varobj is no longer python specific, we'd
> better rename predicate varobj_pretty_printed_p to varobj_is_dynamic_p,
> and use it more widely (done by patch #5 #6).  It paves a way for
> the next patches to add a new type of dynamic varobj.
>
> Patch #7 adds the option --available-children-only in MI code, patch
> #8 adds the corresponding iterator, and patch #9 updates varobj children
> when traceframe is changed.  These three should be in one commit.
> Note that there is at most one iterator can be applied to one varobj.
> If pretty-printer is installed and option --available-children-only
> is used, GDB has to return one iterator, and we choose iterator
> for available-children-only.
>
> Patch #10 and #11 are about test cases.  Patch #10 is to teach
> testsuite to match dynamic="1", and patch #11 is the test case for
> option --available-children-only.
>
> I don't include NEWS and doc patch in this series, because we'd like
> to discuss on "whether we can call varobj with --available-children-only
> a dynamic varobj".  The result affects the doc, IMO.  Although we
> implemented varobj with --available-children-only as a dynamic varobj,
> I am not sure we can do the same in doc, which is user visible.
>
> The whole series is tested on x86_64-linux.

Ping.  https://sourceware.org/ml/gdb-patches/2013-11/msg00739.html

-- 
Yao (齐尧)

Re: [RCF 00/11] Visit varobj available children only in MI

[Yao Qi]

On 11/24/2013 10:09 AM, Yao Qi wrote:
> Hi,
> This patch series proposes a feature that GDB is able to visit varobj
> available children only, by adding a new option
> "--available-children-only" to commands -var-create,
> -var-info-num-children and -var-list-children.  This patch series was
> written by Pedro in one patch.  I read the patch three months ago, split
> it, add some minor things, and write test cases.  I wish I didn't
> break anything, including the rationale and the implementation :)
> 
> In each traceframe, with option --available-children-only, the
> children of varobj vary, so it behaves like a dynamic varobj.
> Since the key of dynamic varobj is the iterator, which can be used to
> visit each child.  We need a new iterator for varobj with
> --available-children-only.
> 
> Current dynamic varobj is python pretty-printer specific, so the
> iterator is python specific too.  In order to add a new type of iterator,
> we have to generalize iterator and de-couple it from python.  At
> present, iteration is performed against PyObject, but it can
> generalized to a name-value pair.  That is what patch #1 ~ #3 do.
> Note that patch #2 and #3 can be a single commit, but I split it for
> review.  Ideally, dynamic varobj can be a generic stuff, IMO.
> 
> After the changes in patch #1 ~ #3, we find some code are not
> python-specific, so #if HAVE_PYTHON can be removed.  It is done by
> patch #4.
> 
> Since we think dynamic varobj is no longer python specific, we'd
> better rename predicate varobj_pretty_printed_p to varobj_is_dynamic_p,
> and use it more widely (done by patch #5 #6).  It paves a way for
> the next patches to add a new type of dynamic varobj.
> 
> Patch #7 adds the option --available-children-only in MI code, patch
> #8 adds the corresponding iterator, and patch #9 updates varobj children
> when traceframe is changed.  These three should be in one commit.
> Note that there is at most one iterator can be applied to one varobj.
> If pretty-printer is installed and option --available-children-only
> is used, GDB has to return one iterator, and we choose iterator
> for available-children-only.
>  
> Patch #10 and #11 are about test cases.  Patch #10 is to teach
> testsuite to match dynamic="1", and patch #11 is the test case for
> option --available-children-only.
> 
> I don't include NEWS and doc patch in this series, because we'd like
> to discuss on "whether we can call varobj with --available-children-only
> a dynamic varobj".  The result affects the doc, IMO.  Although we
> implemented varobj with --available-children-only as a dynamic varobj,
> I am not sure we can do the same in doc, which is user visible.
> 
> The whole series is tested on x86_64-linux.

Ping.  https://sourceware.org/ml/gdb-patches/2013-11/msg00739.html
I am OK to check in after branch 7.7 is created.

-- 
Yao (齐尧)

Re: [RCF 00/11] Visit varobj available children only in MI

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> This patch series proposes a feature that GDB is able to visit varobj
> available children only, by adding a new option
> "--available-children-only" to commands -var-create,
> -var-info-num-children and -var-list-children.  This patch series was
> written by Pedro in one patch.  I read the patch three months ago, split
> it, add some minor things, and write test cases.  I wish I didn't
> break anything, including the rationale and the implementation :)

Hi, Yao,

I apologize for taking so long to get this review underway. I expected 
to have more time during the end-of-year holidays, but things didn't 
work out that way.

I have a few questions about this proposed feature. I understand you've 
inherited quite a bit of the code, and thank you for pursuing this 
submission.

Why was this feature implemented this way? Specifically, if some varobj 
children are not available from trace data, isn't the decision to filter 
this information a function of the user interface? Wouldn't it have 
sufficed to add a flag to the varobj designating the child as not 
collected in the trace experiment or unavailable?

I am definitely /not/ suggesting that this be re-implemented or 
re-designed. I just want to understand why this was implemented this 
way. Perhaps there is a significant speed advantage for large trace 
experiments or some other technical/legitimate reason for this 
implementation?

That aside, one other thing I'd like to ask about: the flag 
"--available-children-only" rather strikes me like a property of the 
varobj. Not altogether unlike the display format. Is there a reason a 
flag was chosen to implement this over, say, a (new) command like 
"-var-set-show-available-children-only" or requiring/allowing 
--available-children-only to be specified on the root varobj creation 
and "saved"/enforced for all subsequent commands on the varobj and its 
children?

The only rationale I can think of is if a UI wanted to query gdb/mi for 
varobjs with and without this option. Is that a common use case? Is 
there perhaps another use case which I have not considered?

Finally, I didn't see any mention of documentation updates. This change 
will require both a manual update and a NEWS entry, documenting the new 
feature.

I believe Joel has committed the MI "features" series; an update to this 
might be desirable [perhaps Joel might be able to offer an opinion].

Keith

Re: [RCF 00/11] Visit varobj available children only in MI

[Joel Brobecker]

> I have a few questions about this proposed feature. I understand
> you've inherited quite a bit of the code, and thank you for pursuing
> this submission.

All really good questions! And a major reason why I spend the time
to discuss new features with the rest of the community ahead of
implementing it :-) (I realize this is always possible, of course).

> I believe Joel has committed the MI "features" series; an update to
> this might be desirable [perhaps Joel might be able to offer an
> opinion].

That's a good suggestion. At the moment, and assuming we maintain
the idea of extending a command, the only option at the moment is
to add an entry to the output of the -list-features command. But,
for static command-line options like this, we could enhance the
-info-gdb-mi-command command to provide feature lists as well.
At the moment, it's unclear that we're in any danger of increasing
the size of the -list-features list, so people will probably want
to KISS and update -list-features instead.

-- 
Joel

Re: [RCF 00/11] Visit varobj available children only in MI

[Yao Qi]

Thanks for the review, Keith.

On 01/08/2014 02:22 AM, Keith Seitz wrote:
> Why was this feature implemented this way? Specifically, if some varobj 
> children are not available from trace data, isn't the decision to filter 
> this information a function of the user interface? Wouldn't it have 
> sufficed to add a flag to the varobj designating the child as not 
> collected in the trace experiment or unavailable?
> 
> I am definitely /not/ suggesting that this be re-implemented or 
> re-designed. I just want to understand why this was implemented this 
> way. Perhaps there is a significant speed advantage for large trace 
> experiments or some other technical/legitimate reason for this 
> implementation?

We don't want frontends learn much new.  If they already support
pretty-print dynamic varobj, they should support available-children-only
varobj.  Frontends have only to pass option "--available-children-only"
to some MI commands, and then frontends can handle the output as
correctly as it handles pretty-printer's output.

> 
> That aside, one other thing I'd like to ask about: the flag 
> "--available-children-only" rather strikes me like a property of the 
> varobj. Not altogether unlike the display format. Is there a reason a 

It (available-children-only) is a property of varobj, we add a new field
available_children_only in struct varobj_dynamic for this purpose (in
patch 07), at least, it is in the code.

> flag was chosen to implement this over, say, a (new) command like 
> "-var-set-show-available-children-only" or requiring/allowing 
> --available-children-only to be specified on the root varobj creation 
> and "saved"/enforced for all subsequent commands on the varobj and its 
> children?

Yeah, we can have a global setting to decide whether to honour
available-children-only or not, and this global setting can be
set by a new MI command.

> 
> The only rationale I can think of is if a UI wanted to query gdb/mi for 
> varobjs with and without this option. Is that a common use case? Is 

I am not sure how common this use case is, but IMO, it is more flexible,
compared with the approach using global setting.

> there perhaps another use case which I have not considered?

> 
> Finally, I didn't see any mention of documentation updates. This change 
> will require both a manual update and a NEWS entry, documenting the new 
> feature.

It was intended and was mentioned in the cover letter of this series.
There should be some changes during the review, which affect the doc
and NEWS.

> 
> I believe Joel has committed the MI "features" series; an update to this 
> might be desirable [perhaps Joel might be able to offer an opinion].

Right, we can add a new feature "mi-available-children-only" in the
reply of -list-features.

-- 
Yao (齐尧)

Re: [PATCH 01/11] Use 'struct varobj_item' to represent name and value pair

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> name and value pair is widely used in varobj.c.  This patch is to add
> a new struct varobj_item to represent them, so that the number of
> function arguments can be reduced.  Finally, the iteration is done on
> 'struct varobj_item' instead of PyObject after this patch series.

I think this approach works. I agree: while we can use the existing 
dynamic varobj infrastructure internally, we may not want to actually 
expose that nomenclature to the user (in the documentation).

As I mentioned in a previous mail, if I could request one "big" change, 
it would be for the --available-only to be persistent (and alterable 
with a new command), e.g., -var-create ... --available-only. But I am 
not going to suggest that is required without the agreement of a maintainer.

I will proceed with review as the patches are written. You've done an 
excellent job of splitting these patches into manageable chunks. Thank 
you so much!

For this first patch (like most of the patches), I really only have 
minor nits.

Keith

> 2013-11-24  Yao Qi  <yao@codesourcery.com>
>
> 	* varobj.c (struct varobj_item): New structure.
> 	(create_child_with_value): Update declaration.
> 	(varobj_add_child): Replace arguments 'name' and 'value' with
> 	'item'.  Callers update.
                          ^^^^^^
Either "Callers updated." or "Update callers."

> 	(install_dynamic_child): Likewise.
> 	(update_dynamic_varobj_children): Likewise.
> 	(varobj_add_child): Likewise.
> 	(create_child_with_value): Likewise.
> ---
>   gdb/varobj.c |   64 +++++++++++++++++++++++++++++++++++----------------------
>   1 files changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index b78969a..4a10617 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -108,6 +108,16 @@ struct varobj_root
>     struct varobj_root *next;
>   };
>
> +/* A node or item of varobj, composed by the name and the value.  */
                                 ^^^^^^^^^^^
"composed of"

> +
> +struct varobj_item
> +{
> +  /* Name of this item.  */
> +  char *name;

Add a newline here.

> +  /* Value of this item.  */
> +  struct value *value;
> +};
> +
>   /* Dynamic part of varobj.  */
>
>   struct varobj_dynamic
> @@ -169,8 +179,8 @@ static void uninstall_variable (struct varobj *);
>   static struct varobj *create_child (struct varobj *, int, char *);
>
>   static struct varobj *
> -create_child_with_value (struct varobj *parent, int index, char *name,
> -			 struct value *value);
> +create_child_with_value (struct varobj *parent, int index,
> +			 struct varobj_item *item);
>
>   /* Utility routines */
>
> @@ -214,8 +224,7 @@ static int is_root_p (struct varobj *var);
>   #if HAVE_PYTHON
>
>   static struct varobj *varobj_add_child (struct varobj *var,
> -					char *name,
> -					struct value *value);
> +					struct varobj_item *item);
>
>   #endif /* HAVE_PYTHON */
>
> @@ -714,13 +723,12 @@ install_dynamic_child (struct varobj *var,
>   		       VEC (varobj_p) **unchanged,
>   		       int *cchanged,
>   		       int index,
> -		       char *name,
> -		       struct value *value)
> +		       struct varobj_item *item)
>   {
>     if (VEC_length (varobj_p, var->children) < index + 1)
>       {
>         /* There's no child yet.  */
> -      struct varobj *child = varobj_add_child (var, name, value);
> +      struct varobj *child = varobj_add_child (var, item);
>
>         if (new)
>   	{
> @@ -731,14 +739,14 @@ install_dynamic_child (struct varobj *var,
>     else
>       {
>         varobj_p existing = VEC_index (varobj_p, var->children, index);
> -      int type_updated = update_type_if_necessary (existing, value);
> +      int type_updated = update_type_if_necessary (existing, item->value);
>
>         if (type_updated)
>   	{
>   	  if (type_changed)
>   	    VEC_safe_push (varobj_p, *type_changed, existing);
>   	}
> -      if (install_new_value (existing, value, 0))
> +      if (install_new_value (existing, item->value, 0))
>   	{
>   	  if (!type_updated && changed)
>   	    VEC_safe_push (varobj_p, *changed, existing);
> @@ -889,7 +897,7 @@ update_dynamic_varobj_children (struct varobj *var,
>   	{
>   	  PyObject *py_v;
>   	  const char *name;
> -	  struct value *v;
> +	  struct varobj_item varobj_item;
>   	  struct cleanup *inner;
>   	  int can_mention = from < 0 || i >= from;
>
> @@ -901,15 +909,17 @@ update_dynamic_varobj_children (struct varobj *var,
>   	      error (_("Invalid item from the child list"));
>   	    }
>
> -	  v = convert_value_from_python (py_v);
> -	  if (v == NULL)
> +	  varobj_item.value = convert_value_from_python (py_v);
> +	  if (varobj_item.value == NULL)
>   	    gdbpy_print_stack ();
> +	  varobj_item.name = xstrdup (name);
> +
>   	  install_dynamic_child (var, can_mention ? changed : NULL,
>   				 can_mention ? type_changed : NULL,
>   				 can_mention ? new : NULL,
>   				 can_mention ? unchanged : NULL,
>   				 can_mention ? cchanged : NULL, i,
> -				 xstrdup (name), v);
> +				 &varobj_item);
>   	  do_cleanups (inner);
>   	}
>         else
> @@ -1028,11 +1038,11 @@ varobj_list_children (struct varobj *var, int *from, int *to)
>   #if HAVE_PYTHON
>
>   static struct varobj *
> -varobj_add_child (struct varobj *var, char *name, struct value *value)
> +varobj_add_child (struct varobj *var, struct varobj_item *item)
>   {
> -  varobj_p v = create_child_with_value (var,
> +  varobj_p v = create_child_with_value (var,
>   					VEC_length (varobj_p, var->children),
> -					name, value);
> +					item);
>
>     VEC_safe_push (varobj_p, var->children, v);
>     return v;
> @@ -2098,13 +2108,17 @@ uninstall_variable (struct varobj *var)
>   static struct varobj *
>   create_child (struct varobj *parent, int index, char *name)
>   {
> -  return create_child_with_value (parent, index, name,
> -				  value_of_child (parent, index));
> +  struct varobj_item item;
> +
> +  item.name = name;
> +  item.value = value_of_child (parent, index);
> +
> +  return create_child_with_value (parent, index, &item);
>   }
>
>   static struct varobj *
> -create_child_with_value (struct varobj *parent, int index, char *name,
> -			 struct value *value)
> +create_child_with_value (struct varobj *parent, int index,
> +			 struct varobj_item *item)
>   {
>     struct varobj *child;
>     char *childs_name;
> @@ -2112,7 +2126,7 @@ create_child_with_value (struct varobj *parent, int index, char *name,
>     child = new_variable ();
>
>     /* NAME is allocated by caller.  */
> -  child->name = name;
> +  child->name = item->name;
>     child->index = index;
>     child->parent = parent;
>     child->root = parent->root;
> @@ -2120,22 +2134,22 @@ create_child_with_value (struct varobj *parent, int index, char *name,
>     if (varobj_is_anonymous_child (child))
>       childs_name = xstrprintf ("%s.%d_anonymous", parent->obj_name, index);
>     else
> -    childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
> +    childs_name = xstrprintf ("%s.%s", parent->obj_name, item->name);
>     child->obj_name = childs_name;
>
>     install_variable (child);
>
>     /* Compute the type of the child.  Must do this before
>        calling install_new_value.  */
> -  if (value != NULL)
> +  if (item->value != NULL)
>       /* If the child had no evaluation errors, var->value
>          will be non-NULL and contain a valid type.  */
> -    child->type = value_actual_type (value, 0, NULL);
> +    child->type = value_actual_type (item->value, 0, NULL);
>     else
>       /* Otherwise, we must compute the type.  */
>       child->type = (*child->root->lang_ops->type_of_child) (child->parent,
>   							   child->index);
> -  install_new_value (child, value, 1);
> +  install_new_value (child, item->value, 1);
>
>     return child;
>   }
>

Re: [PATCH 02/11] Generalize varobj iterator

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> This patch generalizes varobj iterator, in a python-independent way.
> Note varobj_item is still a typedef of PyObject, we can only focus on
> API changes, and leave the data type changes to the next patch.  As a
> result, we include "varobj-iter.h" after the typedef of PyObject in
> varobj.c, but it is an intermediate state.  Finally, varobj-iter.h is
> independent of PyObject.

This looks good to me with the trivial problems mentioned corrected.

> gdb:
>
> 2013-11-24  Pedro Alves  <pedro@codesourcery.com>
> 	    Yao Qi  <yao@codesourcery.com>
>
> 	* Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
> 	(SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
> 	(HFILES_NO_SRCDIR): Add "varobj-iter.h".
> 	(py-varobj.o): New rule.
> 	* python/py-varobj.c: New file.
> 	* python/python-internal.h (py_varobj_get_iterator): Declare.
> 	* varobj-iter.h: New file.
> 	* varobj.c: Include "varobj-iter.h"
                                            ^
Missing '.'

> 	(struct varobj) <child_iter>: Change its type from "PyObject *"
> 	to "struct varobj_iter *".
> 	<saved_item>: Likewise.
> 	[HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
> 	[HAVE_PYTHON] (varobj_get_iterator): New function.
> 	(update_dynamic_varobj_children) [HAVE_PYTHON]: Move
> 	python-specific code to python/py-varobj.c.
> 	(install_visualizer): Call varobj_iter_delete instead of
> 	Py_XDECREF.
> 	* varobj.h (varobj_ensure_python_env): Declare.
> ---
>   gdb/Makefile.in              |   13 +++-
>   gdb/python/py-varobj.c       |  183 ++++++++++++++++++++++++++++++++++++++++++
>   gdb/python/python-internal.h |    4 +
>   gdb/varobj-iter.h            |   62 ++++++++++++++
>   gdb/varobj.c                 |  111 ++++++++------------------
>   gdb/varobj.h                 |    2 +
>   6 files changed, 295 insertions(+), 80 deletions(-)
>   create mode 100644 gdb/python/py-varobj.c
>   create mode 100644 gdb/varobj-iter.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 0591279..7d8a365 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -312,7 +312,8 @@ SUBDIR_PYTHON_OBS = \
>   	py-threadevent.o \
>   	py-type.o \
>   	py-utils.o \
> -	py-value.o
> +	py-value.o \
> +	py-varobj.o
>
>   SUBDIR_PYTHON_SRCS = \
>   	python/python.c \
> @@ -348,7 +349,8 @@ SUBDIR_PYTHON_SRCS = \
>   	python/py-threadevent.c \
>   	python/py-type.c \
>   	python/py-utils.c \
> -	python/py-value.c
> +	python/py-value.c \
> +	python/py-varobj.c
>   SUBDIR_PYTHON_DEPS =
>   SUBDIR_PYTHON_LDFLAGS=
>   SUBDIR_PYTHON_CFLAGS=
> @@ -797,7 +799,8 @@ proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
>   ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
>   exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
>   i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
> -ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \
> +ia64-tdep.h ada-lang.h ada-varobj.h varobj.h varobj-iter.h frv-tdep.h \
> +nto-tdep.h serial.h \
>   c-lang.h d-lang.h go-lang.h frame.h event-loop.h block.h cli/cli-setshow.h \
>   cli/cli-decode.h cli/cli-cmds.h cli/cli-utils.h \
>   cli/cli-script.h macrotab.h symtab.h common/version.h \
> @@ -2285,6 +2288,10 @@ py-value.o: $(srcdir)/python/py-value.c
>   	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-value.c
>   	$(POSTCOMPILE)
>
> +py-varobj.o: $(srcdir)/python/py-varobj.c
> +	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-varobj.c
> +	$(POSTCOMPILE)
> +
>   #
>   # Dependency tracking.  Most of this is conditional on GNU Make being
>   # found by configure; if GNU Make is not found, we fall back to a
> diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
> new file mode 100644
> index 0000000..6588fc2
> --- /dev/null
> +++ b/gdb/python/py-varobj.c
> @@ -0,0 +1,183 @@
> +/* Copyright (C) 2013 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/>.  */
> +
> +#include "defs.h"
> +#include "python-internal.h"
> +#include "varobj.h"
> +#include "varobj-iter.h"
> +
> +/* A dynamic varobj iterator "class" for python pretty-printed
> +   varobjs.  This inherits struct varobj_iter.  */
> +
> +struct py_varobj_iter
> +{
> +  /* The 'base class'.  */
> +  struct varobj_iter base;
> +
> +  /* The python iterator returned by the printer's 'children' method,
> +     or NULL if not available.  */
> +  PyObject *iter;
> +};
> +
> +/* Implementation of the 'dtor' method of pretty-printed varobj
> +   iterators.  */
> +
> +static void
> +py_varobj_iter_dtor (struct varobj_iter *self)
> +{
> +  struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
> +
> +  Py_XDECREF (dis->iter);
> +}
> +
> +/* Implementation of the 'next' method of pretty-pretty varobj
                                              ^^^^^^^^^^^^^

"pretty-printed" :-)

> +   iterators.  */
> +
> +static varobj_item *
> +py_varobj_iter_next (struct varobj_iter *self)
> +{
> +  struct py_varobj_iter *t = (struct py_varobj_iter *) self;
> +  struct cleanup *back_to;
> +  PyObject *item;
> +
> +  back_to = varobj_ensure_python_env (self->var);
> +
> +  item = PyIter_Next (t->iter);
> +
> +  if (item == NULL)
> +    {
> +      /* Normal end of iteration.  */
> +      if (!PyErr_Occurred ())
> +	return NULL;
> +
> +      /* If we got a memory error, just use the text as the item.  */
> +      if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
> +	{
> +	  PyObject *type, *value, *trace;
> +	  char *name_str, *value_str;
> +
> +	  PyErr_Fetch (&type, &value, &trace);
> +	  value_str = gdbpy_exception_to_string (type, value);
> +	  Py_XDECREF (type);
> +	  Py_XDECREF (value);
> +	  Py_XDECREF (trace);
> +	  if (value_str == NULL)
> +	    {
> +	      gdbpy_print_stack ();
> +	      return NULL;
> +	    }
> +
> +	  name_str = xstrprintf ("<error at %d>",
> +				 self->next_raw_index++);
> +	  item = Py_BuildValue ("(ss)", name_str, value_str);
> +	  xfree (name_str);
> +	  xfree (value_str);
> +	  if (item == NULL)
> +	    {
> +	      gdbpy_print_stack ();
> +	      return NULL;
> +	    }
> +	}
> +      else
> +	{
> +	  /* Any other kind of error.  */
> +	  gdbpy_print_stack ();
> +	  return NULL;
> +	}
> +    }
> +
> +  self->next_raw_index++;
> +  do_cleanups (back_to);
> +  return item;
> +}
> +
> +/* The 'vtable' of pretty-printed python varobj iterators.  */
> +
> +static const struct varobj_iter_ops py_varobj_iter_ops =
> +{
> +  py_varobj_iter_dtor,
> +  py_varobj_iter_next
> +};
> +
> +/* Constructor of pretty-printed varobj iterators.  VAR is the varobj
> +   whose children the iterator will be iterating over.  PYITER is the
> +   python iterator actually responsible for the iteration.  */
> +
> +static void
> +py_varobj_iter_ctor (struct py_varobj_iter *self,
> +		      struct varobj *var, PyObject *pyiter)
> +{
> +  self->base.var = var;
> +  self->base.ops = &py_varobj_iter_ops;
> +  self->base.next_raw_index = 0;
> +  self->iter = pyiter;
> +}
> +
> +/* Allocate and construct a pretty-printed varobj iterator.  VAR is
> +   the varobj whose children the iterator will be iterating over.
> +   PYITER is the python iterator actually responsible for the
> +   iteration.  */
> +
> +static struct py_varobj_iter *
> +py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
> +{
> +  struct py_varobj_iter *self;
> +
> +  self = xmalloc (sizeof (struct py_varobj_iter));

You can use "XNEW (struct py_varobj_iter)" as a shorthand for this.

> +  py_varobj_iter_ctor (self, var, pyiter);
> +  return self;
> +}
> +
> +/* Return a new pretty-printed varobj iterator suitable to iterate
> +   over VAR's children.  */
> +
> +struct varobj_iter *
> +py_varobj_get_iterator (struct varobj *var, PyObject *printer)
> +{
> +  PyObject *children;
> +  int i;
> +  PyObject *iter;
> +  struct py_varobj_iter *py_iter;
> +  struct cleanup *back_to = varobj_ensure_python_env (var);
> +
> +  if (!PyObject_HasAttr (printer, gdbpy_children_cst))
> +    {
> +      do_cleanups (back_to);
> +      return NULL;
> +    }
> +
> +  children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
> +					 NULL);
> +  if (children == NULL)
> +    {
> +      gdbpy_print_stack ();
> +      error (_("Null value returned for children"));
> +    }
> +
> +  make_cleanup_py_decref (children);
> +
> +  iter = PyObject_GetIter (children);
> + if (iter == NULL)
> +    {
> +      gdbpy_print_stack ();
> +      error (_("Could not get children iterator"));
> +    }
> +
> +  py_iter = py_varobj_iter_new (var, iter);
> +
> +  do_cleanups (back_to);
> +
> +  return &py_iter->base;
> +}
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index 7d8c4ad..93f9c0a 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -494,4 +494,8 @@ int gdb_pymodule_addobject (PyObject *module, const char *name,
>   			    PyObject *object)
>     CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
>
> +struct varobj_iter;
> +struct varobj;
> +struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
> +					    PyObject *printer);
>   #endif /* GDB_PYTHON_INTERNAL_H */
> diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
> new file mode 100644
> index 0000000..59be278
> --- /dev/null
> +++ b/gdb/varobj-iter.h
> @@ -0,0 +1,62 @@
> +/* Iterator of varobj.
> +   Copyright (C) 2013 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/>.  */
> +
> +struct varobj_iter_ops;
> +
> +typedef PyObject varobj_item;
> +
> +/* A dynamic varobj iterator "class".  */
> +
> +struct varobj_iter
> +{
> +  /* The 'vtable'.  */
> +  const struct varobj_iter_ops *ops;
> +
> +  /* The varobj this iterator is listing children for.  */
> +  struct varobj *var;
> +
> +  /* The next raw index we will try to check is available.  If is

"If it is" or "If this is"

> +     equal to number_of_children, then we've already iterated the
> +     whole set.  */
> +  int next_raw_index;
> +};
> +
> +/* The vtable of the varobj iterator class.  */

Missing newline

> +struct varobj_iter_ops
> +{
> +  /* Destructor.  Releases everything from SELF (but not SELF
> +     itself).  */
> +  void (*dtor) (struct varobj_iter *self);
> +
> +  /* Returns the next object or NULL if it has reached the end.  */
> +  varobj_item *(*next) (struct varobj_iter *self);
> +};
> +
> +/* Returns the next varobj or NULL if it has reached the end.  */
> +
> +#define varobj_iter_next(ITER)	(ITER)->ops->next (ITER)
> +
> +/* Delete a varobj_iter object.  */
> +
> +#define varobj_iter_delete(ITER)	       \
> +  do					       \
> +    {					       \
> +      if (ITER)			       \

I believe we are now requested an explicit check against NULL.

> +	{				       \
> +	  (ITER)->ops->dtor (ITER);	       \
> +	  xfree (ITER);		       \
> +	}				       \
> +    } while (0)
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 4a10617..7a8305b 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -41,6 +41,8 @@
>   typedef int PyObject;
>   #endif
>
> +#include "varobj-iter.h"
> +
>   /* Non-zero if we want to see trace of varobj level stuff.  */
>
>   unsigned int varobjdebug = 0;
> @@ -139,14 +141,14 @@ struct varobj_dynamic
>
>     /* The iterator returned by the printer's 'children' method, or NULL
>        if not available.  */
> -  PyObject *child_iter;
> +  struct varobj_iter *child_iter;
>
>     /* We request one extra item from the iterator, so that we can
>        report to the caller whether there are more items than we have
>        already reported.  However, we don't want to install this value
>        when we read it, because that will mess up future updates.  So,
>        we stash it here instead.  */
> -  PyObject *saved_item;
> +  varobj_item *saved_item;
>   };
>
>   struct cpstack
> @@ -255,7 +257,7 @@ is_root_p (struct varobj *var)
>   #ifdef HAVE_PYTHON
>   /* Helper function to install a Python environment suitable for
>      use during operations on VAR.  */
> -static struct cleanup *
> +struct cleanup *
>   varobj_ensure_python_env (struct varobj *var)
>   {
>     return ensure_python_env (var->root->exp->gdbarch,
> @@ -772,6 +774,19 @@ dynamic_varobj_has_child_method (struct varobj *var)
>     return result;
>   }
>
> +/* Dynamic varobj's iterator factory.  Returns an iterator object
> +   suitable for iterating over VAR's children.  */

This first bit is a tiny bit awkward. How about "A factory for creating 
dynamic varobj iterators." ?

> +
> +static struct varobj_iter *
> +varobj_get_iterator (struct varobj *var)
> +{
> +  if (var->dynamic->pretty_printer)
> +    return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
> +
> +  gdb_assert_not_reached (_("\
> +requested an interator from a non-dynamic varobj"));
                 ^^^^^^^^^
"iterator"

> +}
> +
>   #endif
>
>   static int
> @@ -787,9 +802,7 @@ update_dynamic_varobj_children (struct varobj *var,
>   {
>   #if HAVE_PYTHON
>     struct cleanup *back_to;
> -  PyObject *children;
>     int i;
> -  PyObject *printer = var->dynamic->pretty_printer;
>
>     if (!gdb_python_initialized)
>       return 0;
> @@ -797,37 +810,22 @@ update_dynamic_varobj_children (struct varobj *var,
>     back_to = varobj_ensure_python_env (var);
>
>     *cchanged = 0;
> -  if (!PyObject_HasAttr (printer, gdbpy_children_cst))
> -    {
> -      do_cleanups (back_to);
> -      return 0;
> -    }
>
>     if (update_children || var->dynamic->child_iter == NULL)
>       {
> -      children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
> -					     NULL);
> +      varobj_iter_delete (var->dynamic->child_iter);
> +      var->dynamic->child_iter = varobj_get_iterator (var);
>
> -      if (!children)
> -	{
> -	  gdbpy_print_stack ();
> -	  error (_("Null value returned for children"));
> -	}
> +      Py_XDECREF (var->dynamic->saved_item);
> +      var->dynamic->saved_item = NULL;
>
> -      make_cleanup_py_decref (children);
> +      i = 0;
>
> -      Py_XDECREF (var->dynamic->child_iter);
> -      var->dynamic->child_iter = PyObject_GetIter (children);
>         if (var->dynamic->child_iter == NULL)
>   	{
> -	  gdbpy_print_stack ();
> -	  error (_("Could not get children iterator"));
> +	  do_cleanups (back_to);
> +	  return 0;
>   	}
> -
> -      Py_XDECREF (var->dynamic->saved_item);
> -      var->dynamic->saved_item = NULL;
> -
> -      i = 0;
>       }
>     else
>       i = VEC_length (varobj_p, var->children);
> @@ -837,7 +835,6 @@ update_dynamic_varobj_children (struct varobj *var,
>     for (; to < 0 || i < to + 1; ++i)
>       {
>         PyObject *item;
> -      int force_done = 0;
>
>         /* See if there was a leftover from last time.  */
>         if (var->dynamic->saved_item)
> @@ -846,52 +843,17 @@ update_dynamic_varobj_children (struct varobj *var,
>   	  var->dynamic->saved_item = NULL;
>   	}
>         else
> -	item = PyIter_Next (var->dynamic->child_iter);
> -
> -      if (!item)
>   	{
> -	  /* Normal end of iteration.  */
> -	  if (!PyErr_Occurred ())
> -	    break;
> -
> -	  /* If we got a memory error, just use the text as the
> -	     item.  */
> -	  if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
> -	    {
> -	      PyObject *type, *value, *trace;
> -	      char *name_str, *value_str;
> -
> -	      PyErr_Fetch (&type, &value, &trace);
> -	      value_str = gdbpy_exception_to_string (type, value);
> -	      Py_XDECREF (type);
> -	      Py_XDECREF (value);
> -	      Py_XDECREF (trace);
> -	      if (!value_str)
> -		{
> -		  gdbpy_print_stack ();
> -		  break;
> -		}
> -
> -	      name_str = xstrprintf ("<error at %d>", i);
> -	      item = Py_BuildValue ("(ss)", name_str, value_str);
> -	      xfree (name_str);
> -	      xfree (value_str);
> -	      if (!item)
> -		{
> -		  gdbpy_print_stack ();
> -		  break;
> -		}
> -
> -	      force_done = 1;
> -	    }
> -	  else
> -	    {
> -	      /* Any other kind of error.  */
> -	      gdbpy_print_stack ();
> -	      break;
> -	    }
> +	  item = varobj_iter_next (var->dynamic->child_iter);
>   	}
>
> +      if (item == NULL)
> +	{
> +	  /* Iteration is done.  Remove iterator from VAR.  */
> +	  varobj_iter_delete (var->dynamic->child_iter);
> +	  var->dynamic->child_iter = NULL;
> +	  break;
> +	}
>         /* We don't want to push the extra child on any report list.  */
>         if (to < 0 || i < to)
>   	{
> @@ -931,9 +893,6 @@ update_dynamic_varobj_children (struct varobj *var,
>   	     element.  */
>   	  break;
>   	}
> -
> -      if (force_done)
> -	break;
>       }
>
>     if (i < VEC_length (varobj_p, var->children))
> @@ -952,8 +911,6 @@ update_dynamic_varobj_children (struct varobj *var,
>       *cchanged = 1;
>
>     var->num_children = VEC_length (varobj_p, var->children);
> -
> -  do_cleanups (back_to);

Doesn't this cleanup still need to run?

>
>     return 1;
>   #else
> @@ -1244,7 +1201,7 @@ install_visualizer (struct varobj_dynamic *var, PyObject *constructor,
>     Py_XDECREF (var->pretty_printer);
>     var->pretty_printer = visualizer;
>
> -  Py_XDECREF (var->child_iter);
> +  varobj_iter_delete (var->child_iter);
>     var->child_iter = NULL;
>   }
>
> diff --git a/gdb/varobj.h b/gdb/varobj.h
> index 978d9b9..0f68311 100644
> --- a/gdb/varobj.h
> +++ b/gdb/varobj.h
> @@ -308,6 +308,8 @@ extern int varobj_has_more (struct varobj *var, int to);
>
>   extern int varobj_pretty_printed_p (struct varobj *var);
>
> +extern  struct cleanup *varobj_ensure_python_env (struct varobj *var);
> +

IIRC, we are now adding documentation near declarations, too. Many are 
often as simple as "See description in varobj.c."

>   extern int varobj_default_value_is_changeable_p (struct varobj *var);
>   extern int varobj_value_is_changeable_p (struct varobj *var);
>
>

Re: [PATCH 05/11] Rename varobj_pretty_printed_p to varobj_is_dynamic_p

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> We think varobj with --available-children-only behaves like a dynamic
> varobj, so dyanmic varobj is not pretty-printer specific.  We rename
> varobj_pretty_printed_p to varobj_is_dynamic_p, so that we can handle
> available-children-only checking in varobj_is_dynamic_p in the next
> patch.

One little nit, otherwise, this looks good to me.

Keith

>
> gdb:
>
> 2013-11-24  Yao Qi  <yao@codesourcery.com>
>
> 	* varobj.c (varobj_pretty_printed_p): Rename to ...
> 	(varobj_is_dynamic_p): ... this.  New function.
> 	* varobj.h (varobj_pretty_printed_p): Remove declaration.
> 	(varobj_is_dynamic_p): Declare.
> 	* mi/mi-cmd-var.c (print_varobj): Caller update.

"Caller updated" or "Update caller"

> 	(mi_print_value_p, varobj_update_one): Likewise.
> ---
>   gdb/mi/mi-cmd-var.c |    6 +++---
>   gdb/varobj.c        |    4 +++-
>   gdb/varobj.h        |    2 +-
>   3 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
> index 84cdc15..2201630 100644
> --- a/gdb/mi/mi-cmd-var.c
> +++ b/gdb/mi/mi-cmd-var.c
> @@ -88,7 +88,7 @@ print_varobj (struct varobj *var, enum print_values print_values,
>         xfree (display_hint);
>       }
>
> -  if (varobj_pretty_printed_p (var))
> +  if (varobj_is_dynamic_p (var))
>       ui_out_field_int (uiout, "dynamic", 1);
>   }
>
> @@ -352,7 +352,7 @@ mi_print_value_p (struct varobj *var, enum print_values print_values)
>     if (print_values == PRINT_ALL_VALUES)
>       return 1;
>
> -  if (varobj_pretty_printed_p (var))
> +  if (varobj_is_dynamic_p (var))
>       return 1;
>
>     type = varobj_get_gdb_type (var);
> @@ -777,7 +777,7 @@ varobj_update_one (struct varobj *var, enum print_values print_values,
>   	  xfree (display_hint);
>   	}
>
> -      if (varobj_pretty_printed_p (r->varobj))
> +      if (varobj_is_dynamic_p (r->varobj))
>   	ui_out_field_int (uiout, "dynamic", 1);
>
>         varobj_get_child_range (r->varobj, &from, &to);
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index e362229..948f80c 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -1066,8 +1066,10 @@ varobj_get_attributes (struct varobj *var)
>     return attributes;
>   }
>
> +/* Return true if VAR is a dynamic varobj.  */
> +
>   int
> -varobj_pretty_printed_p (struct varobj *var)
> +varobj_is_dynamic_p (struct varobj *var)
>   {
>     return var->dynamic->pretty_printer != NULL;
>   }
> diff --git a/gdb/varobj.h b/gdb/varobj.h
> index 0f68311..60ace6f 100644
> --- a/gdb/varobj.h
> +++ b/gdb/varobj.h
> @@ -306,7 +306,7 @@ extern void varobj_enable_pretty_printing (void);
>
>   extern int varobj_has_more (struct varobj *var, int to);
>
> -extern int varobj_pretty_printed_p (struct varobj *var);
> +extern int varobj_is_dynamic_p (struct varobj *var);
>
>   extern  struct cleanup *varobj_ensure_python_env (struct varobj *var);
>
>

Re: [PATCH 06/11] Use varobj_is_dynamic_p more widely

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> Use varobj_is_dynamic_p more widely so that the callers of
> varobj_is_dynamic_p are unchanged when we add available-children-only
> stuff in varobj_is_dynamic_p.

This also looks good to me.

Keith

>
> gdb:
>
> 2013-11-24  Yao Qi  <yao@codesourcery.com>
>
> 	* varobj.c (varobj_get_num_children): Call
> 	varobj_is_dynamic_p.
> 	(varobj_list_children): Likewise.
> 	(varobj_update): Likewise.  Update comments.
> ---
>   gdb/varobj.c |   11 +++++------
>   1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 948f80c..0e0be6c 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -895,7 +895,7 @@ varobj_get_num_children (struct varobj *var)
>   {
>     if (var->num_children == -1)
>       {
> -      if (var->dynamic->pretty_printer != NULL)
> +      if (varobj_is_dynamic_p (var))
>   	{
>   	  int dummy;
>
> @@ -922,7 +922,7 @@ varobj_list_children (struct varobj *var, int *from, int *to)
>
>     var->dynamic->children_requested = 1;
>
> -  if (var->dynamic->pretty_printer != NULL)
> +  if (varobj_is_dynamic_p (var))
>       {
>         /* This, in theory, can result in the number of children changing without
>   	 frontend noticing.  But well, calling -var-list-children on the same
> @@ -1715,10 +1715,9 @@ varobj_update (struct varobj **varp, int explicit)
>   	    }
>   	}
>
> -      /* We probably should not get children of a varobj that has a
> -	 pretty-printer, but for which -var-list-children was never
> -	 invoked.  */
> -      if (v->dynamic->pretty_printer != NULL)
> +      /* We probably should not get children of a dynamic varobj, but
> +	 for which -var-list-children was never invoked.  */
> +      if (varobj_is_dynamic_p (v))
>   	{
>   	  VEC (varobj_p) *changed = 0, *type_changed = 0, *unchanged = 0;
>   	  VEC (varobj_p) *new = 0;
>

Re: [PATCH 03/11] Iterate over 'struct varobj_item' instead of PyObject

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> gdb:
>
> 2013-11-24  Pedro Alves  <pedro@codesourcery.com>
> 	    Yao Qi  <yao@codesourcery.com>
>
> 	* python/py-varobj.c (py_varobj_iter_next): Move some code
> 	from varobj.c.
> 	(py_varobj_get_iterator):
          ^^^^^^^^^^^^^^^^^^^^^^
Superfluous? I don't see any changes to that function.

> 	* varobj-iter.h (struct varobj_item): New.  Moved from
> 	varobj.c.

I would remove "New." That threw me a little while reading the patch. It 
looked familiar! Simply saying that you moved it is sufficient.

> 	* varobj.c: Move "varobj-iter.h" inclusion earlier.
> 	(struct varobj_item): Remove.

I think it is more concise to say "Moved to varobj-iter.h" instead of 
"Remove".

> 	(varobj_clear_saved_item): New function.
> 	(update_dynamic_varobj_children): Move python-related code to
> 	py-varobj.c.
> 	(free_variable): Call varobj_clear_saved_item.
> ---
>   gdb/python/py-varobj.c |   17 ++++++++++-
>   gdb/varobj-iter.h      |   12 ++++++-
>   gdb/varobj.c           |   73 ++++++++++++++++-------------------------------
>   3 files changed, 51 insertions(+), 51 deletions(-)
>
> diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
> index 6588fc2..506e865 100644
> --- a/gdb/python/py-varobj.c
> +++ b/gdb/python/py-varobj.c
> @@ -51,6 +51,9 @@ py_varobj_iter_next (struct varobj_iter *self)
>     struct py_varobj_iter *t = (struct py_varobj_iter *) self;
>     struct cleanup *back_to;
>     PyObject *item;
> +  PyObject *py_v;
> +  varobj_item *vitem;
> +  const char *name = NULL;
>
>     back_to = varobj_ensure_python_env (self->var);
>
> @@ -98,9 +101,21 @@ py_varobj_iter_next (struct varobj_iter *self)
>   	}
>       }
>
> +  if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
> +    {
> +      gdbpy_print_stack ();
> +      error (_("Invalid item from the child list"));
> +    }
> +
> +  vitem = xmalloc (sizeof *vitem);

You can use "XNEW (struct varobj_item)" here, too, although there isn't 
any real overriding reason to do so (other than personal preference). 
Feel free to ignore me, though. There is no hard rule for this in 
gdb-land as far as I know.

> +  vitem->value = convert_value_from_python (py_v);
> +  if (vitem->value == NULL)
> +    gdbpy_print_stack ();
> +  vitem->name = xstrdup (name);
> +
>     self->next_raw_index++;
>     do_cleanups (back_to);
> -  return item;
> +  return vitem;
>   }
>
>   /* The 'vtable' of pretty-printed python varobj iterators.  */
> diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
> index 59be278..b4bda82 100644
> --- a/gdb/varobj-iter.h
> +++ b/gdb/varobj-iter.h
> @@ -14,9 +14,17 @@
>      You should have received a copy of the GNU General Public License
>      along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>
> -struct varobj_iter_ops;
> +/* A node or item of varobj, composed by the name and the value.  */
> +
> +typedef struct varobj_item
> +{
> +  /* Name of this item.  */
> +  char *name;
> +  /* Value of this item.  */
> +  struct value *value;
> +} varobj_item;
>

[Changes mentioned in last patch for above hunk]

> -typedef PyObject varobj_item;
> +struct varobj_iter_ops;
>
>   /* A dynamic varobj iterator "class".  */
>
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 7a8305b..28e934c 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -33,6 +33,7 @@
>   #include "vec.h"
>   #include "gdbthread.h"
>   #include "inferior.h"
> +#include "varobj-iter.h"
>
>   #if HAVE_PYTHON
>   #include "python/python.h"
> @@ -41,8 +42,6 @@
>   typedef int PyObject;
>   #endif
>
> -#include "varobj-iter.h"
> -
>   /* Non-zero if we want to see trace of varobj level stuff.  */
>
>   unsigned int varobjdebug = 0;
> @@ -110,16 +109,6 @@ struct varobj_root
>     struct varobj_root *next;
>   };
>
> -/* A node or item of varobj, composed by the name and the value.  */
> -
> -struct varobj_item
> -{
> -  /* Name of this item.  */
> -  char *name;
> -  /* Value of this item.  */
> -  struct value *value;
> -};
> -
>   /* Dynamic part of varobj.  */
>
>   struct varobj_dynamic
> @@ -787,6 +776,18 @@ varobj_get_iterator (struct varobj *var)
>   requested an interator from a non-dynamic varobj"));
>   }
>
> +/* Release and clear VAR's saved item, if any.  */
> +
> +static void
> +varobj_clear_saved_item (struct varobj_dynamic *var)
> +{
> +  if (var->saved_item != NULL)
> +    {
> +      value_free (var->saved_item->value);
> +      xfree (var->saved_item);
> +      var->saved_item = NULL;
> +    }
> +}
>   #endif
>
>   static int
> @@ -801,14 +802,8 @@ update_dynamic_varobj_children (struct varobj *var,
>   				int to)
>   {
>   #if HAVE_PYTHON
> -  struct cleanup *back_to;
>     int i;
>
> -  if (!gdb_python_initialized)
> -    return 0;
> -
> -  back_to = varobj_ensure_python_env (var);
> -
>     *cchanged = 0;
>
>     if (update_children || var->dynamic->child_iter == NULL)
> @@ -816,16 +811,12 @@ update_dynamic_varobj_children (struct varobj *var,
>         varobj_iter_delete (var->dynamic->child_iter);
>         var->dynamic->child_iter = varobj_get_iterator (var);
>
> -      Py_XDECREF (var->dynamic->saved_item);
> -      var->dynamic->saved_item = NULL;
> +      varobj_clear_saved_item (var->dynamic);
>
>         i = 0;
>
>         if (var->dynamic->child_iter == NULL)
> -	{
> -	  do_cleanups (back_to);
> -	  return 0;
> -	}
> +	return 0;
>       }
>     else
>       i = VEC_length (varobj_p, var->children);
> @@ -834,10 +825,10 @@ update_dynamic_varobj_children (struct varobj *var,
>        are more children.  */
>     for (; to < 0 || i < to + 1; ++i)
>       {
> -      PyObject *item;
> +      varobj_item *item;
>
>         /* See if there was a leftover from last time.  */
> -      if (var->dynamic->saved_item)
> +      if (var->dynamic->saved_item != NULL)
>   	{
>   	  item = var->dynamic->saved_item;
>   	  var->dynamic->saved_item = NULL;
> @@ -845,6 +836,10 @@ update_dynamic_varobj_children (struct varobj *var,
>         else
>   	{
>   	  item = varobj_iter_next (var->dynamic->child_iter);
> +	  /* Release vitem->value so its lifetime is not bound to the
> +	     execution of a command.  */
> +	  if (item != NULL && item->value != NULL)
> +	    release_value_or_incref (item->value);
>   	}
>
>         if (item == NULL)
> @@ -857,36 +852,19 @@ update_dynamic_varobj_children (struct varobj *var,
>         /* We don't want to push the extra child on any report list.  */
>         if (to < 0 || i < to)
>   	{
> -	  PyObject *py_v;
> -	  const char *name;
> -	  struct varobj_item varobj_item;
> -	  struct cleanup *inner;
>   	  int can_mention = from < 0 || i >= from;
>
> -	  inner = make_cleanup_py_decref (item);
> -
> -	  if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
> -	    {
> -	      gdbpy_print_stack ();
> -	      error (_("Invalid item from the child list"));
> -	    }
> -
> -	  varobj_item.value = convert_value_from_python (py_v);
> -	  if (varobj_item.value == NULL)
> -	    gdbpy_print_stack ();
> -	  varobj_item.name = xstrdup (name);
> -
>   	  install_dynamic_child (var, can_mention ? changed : NULL,
>   				 can_mention ? type_changed : NULL,
>   				 can_mention ? new : NULL,
>   				 can_mention ? unchanged : NULL,
>   				 can_mention ? cchanged : NULL, i,
> -				 &varobj_item);
> -	  do_cleanups (inner);
> +				 item);
> +
> +	  xfree (item);
>   	}
>         else
>   	{
> -	  Py_XDECREF (var->dynamic->saved_item);
>   	  var->dynamic->saved_item = item;
>
>   	  /* We want to truncate the child list just before this
> @@ -2180,12 +2158,11 @@ free_variable (struct varobj *var)
>
>         Py_XDECREF (var->dynamic->constructor);
>         Py_XDECREF (var->dynamic->pretty_printer);
> -      Py_XDECREF (var->dynamic->child_iter);

If I'm reading this right, this statement was moved to the python 
iterator dtor function. What happens if we do not iterate over all 
children and then delete the varobj? I don't see where 
varobj_delete_iter is called in that case, i.e., isn't this leaked in 
this scenario?

> -      Py_XDECREF (var->dynamic->saved_item);
>         do_cleanups (cleanup);
>       }
>   #endif
>
> +  varobj_clear_saved_item (var->dynamic);
>     value_free (var->value);
>
>     /* Free the expression if this is a root variable.  */
>

Re: [PATCH 04/11] Remove #if HAVE_PYTHON

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> This patch removes some unnecessary "#if HAVE_PYTHON" so that more
> code is generalized.

Yippie! :-)

This all looks good to me.

Keith

> 2013-11-24  Pedro Alves  <pedro@codesourcery.com>
> 	    Yao Qi  <yao@codesourcery.com>
>
> 	* varobj.c: Remove "#if HAVE_PYTHON" and "#endif".
> 	(varobj_get_iterator): Wrap up code for pretty-printer by
> 	"#if HAVE_PYTHON" and "#endif".
> 	(update_dynamic_varobj_children): Likewise.
> ---
>   gdb/varobj.c |   20 +++++---------------
>   1 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 28e934c..e362229 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -212,13 +212,9 @@ static char *my_value_of_variable (struct varobj *var,
>
>   static int is_root_p (struct varobj *var);
>
> -#if HAVE_PYTHON
> -
>   static struct varobj *varobj_add_child (struct varobj *var,
>   					struct varobj_item *item);
>
> -#endif /* HAVE_PYTHON */
> -
>   /* Private data */
>
>   /* Mappings of varobj_display_formats enums to gdb's format codes.  */
> @@ -701,8 +697,6 @@ varobj_restrict_range (VEC (varobj_p) *children, int *from, int *to)
>       }
>   }
>
> -#if HAVE_PYTHON
> -
>   /* A helper for update_dynamic_varobj_children that installs a new
>      child when needed.  */
>
> @@ -747,6 +741,8 @@ install_dynamic_child (struct varobj *var,
>       }
>   }
>
> +#if HAVE_PYTHON
> +
>   static int
>   dynamic_varobj_has_child_method (struct varobj *var)
>   {
> @@ -762,6 +758,7 @@ dynamic_varobj_has_child_method (struct varobj *var)
>     do_cleanups (back_to);
>     return result;
>   }
> +#endif
>
>   /* Dynamic varobj's iterator factory.  Returns an iterator object
>      suitable for iterating over VAR's children.  */
> @@ -769,8 +766,10 @@ dynamic_varobj_has_child_method (struct varobj *var)
>   static struct varobj_iter *
>   varobj_get_iterator (struct varobj *var)
>   {
> +#if HAVE_PYTHON
>     if (var->dynamic->pretty_printer)
>       return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
> +#endif
>
>     gdb_assert_not_reached (_("\
>   requested an interator from a non-dynamic varobj"));
> @@ -788,7 +787,6 @@ varobj_clear_saved_item (struct varobj_dynamic *var)
>         var->saved_item = NULL;
>       }
>   }
> -#endif
>
>   static int
>   update_dynamic_varobj_children (struct varobj *var,
> @@ -801,7 +799,6 @@ update_dynamic_varobj_children (struct varobj *var,
>   				int from,
>   				int to)
>   {
> -#if HAVE_PYTHON
>     int i;
>
>     *cchanged = 0;
> @@ -891,9 +888,6 @@ update_dynamic_varobj_children (struct varobj *var,
>     var->num_children = VEC_length (varobj_p, var->children);
>
>     return 1;
> -#else
> -  gdb_assert_not_reached ("should never be called if Python is not enabled");
> -#endif
>   }
>
>   int
> @@ -970,8 +964,6 @@ varobj_list_children (struct varobj *var, int *from, int *to)
>     return var->children;
>   }
>
> -#if HAVE_PYTHON
> -
>   static struct varobj *
>   varobj_add_child (struct varobj *var, struct varobj_item *item)
>   {
> @@ -983,8 +975,6 @@ varobj_add_child (struct varobj *var, struct varobj_item *item)
>     return v;
>   }
>
> -#endif /* HAVE_PYTHON */
> -
>   /* Obtain the type of an object Variable as a string similar to the one gdb
>      prints on the console.  */
>
>

Re: [PATCH 07/11] MI option --available-children-only

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> This patch adds option --available-children-only to three MI commands,
> parse it, and set corresponding field of 'struct varobj_dynamic'.

I've already mentioned my preference for allowing 
--available-children-only for -var-create and adding a new command to 
flip it on/off, so unless another maintainer agrees, we'll agree to 
leave this alone.

Two minor nits below.

Keith

> gdb:
>
> 2013-11-24  Pedro Alves  <pedro@codesourcery.com>
> 	    Yao Qi  <yao@codesourcery.com>
>
> 	* mi/mi-cmd-var.c (mi_cmd_var_create): Use mi_getopt to parse
> 	options.  Handle "--available-children-only".
> 	(mi_cmd_var_info_num_children): Likewise.
> 	(mi_cmd_var_list_children): Likewise.
> 	* varobj.c (struct varobj_dynamic) <available_children_only>:
> 	New.
> 	(new_variable, new_root_variable): Update declaration.
> 	(varobj_is_dynamic_p): Return true if available-children-only
> 	is true.
> 	(varobj_create): Add new argument "available_children_only".
> 	Callers update.
> 	(varobj_get_num_children): Handle "available_children_only"
> 	(varobj_set_available_children_only): New function.
> 	(varobj_list_children): Handle "available_children_only".
> 	(install_new_value): Likewise.
> 	(varobj_update): Likewise.
> 	(new_variable): Add new argument "available_children_only".
> 	(new_root_variable): Likewise.
> 	(varobj_invalidate_iter): Likewise.
> 	* varobj.h (varobj_create): Update declaration.
> 	(varobj_set_available_children_only): Declare.
> ---
>   gdb/mi/mi-cmd-var.c |  128 ++++++++++++++++++++++++++++++++++++++++++++-------
>   gdb/varobj.c        |   51 +++++++++++++++-----
>   gdb/varobj.h        |   11 ++++-
>   3 files changed, 160 insertions(+), 30 deletions(-)
>
> diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
> index 2201630..552a527 100644
> --- a/gdb/mi/mi-cmd-var.c
> +++ b/gdb/mi/mi-cmd-var.c
> @@ -105,19 +105,50 @@ mi_cmd_var_create (char *command, char **argv, int argc)
>     char *expr;
>     struct cleanup *old_cleanups;
>     enum varobj_type var_type;
> +  int available_children_only = 0;
> +  int oind = 0;
> +  enum opt
> +    {
> +      AVAILABLE_CHILDREN_ONLY,
> +    };
> +  static const struct mi_opt opts[] =
> +    {
> +      {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
> +      { 0, 0, 0 }
> +    };
>
> -  if (argc != 3)
> -    error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
> +  /* Parse arguments.  In this instance we are just looking for
> +     --available_children_only.  */
> +  while (1)
> +    {
> +      char *oarg;
> +      int opt = mi_getopt ("-var-create", argc, argv,
> +			   opts, &oind, &oarg);
> +      if (opt < 0)
> +	break;
> +      switch ((enum opt) opt)
> +	{
> +	case AVAILABLE_CHILDREN_ONLY:
> +	  available_children_only = 1;
> +	  break;
> +	}
> +    }
>
> -  name = xstrdup (argv[0]);
> +
> +  if (argc - oind != 3)
> +    error (_("Usage: -var-create [--available-children-only] "
> +	     "NAME FRAME EXPRESSION"));
> +
> +
> +  name = xstrdup (argv[oind]);
>     /* Add cleanup for name. Must be free_current_contents as name can
>        be reallocated.  */
>     old_cleanups = make_cleanup (free_current_contents, &name);
>
> -  frame = xstrdup (argv[1]);
> +  frame = xstrdup (argv[oind + 1]);
>     make_cleanup (xfree, frame);
>
> -  expr = xstrdup (argv[2]);
> +  expr = xstrdup (argv[oind + 2]);
>     make_cleanup (xfree, expr);
>
>     if (strcmp (name, "-") == 0)
> @@ -143,7 +174,8 @@ mi_cmd_var_create (char *command, char **argv, int argc)
>   		    "Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
>   			name, frame, hex_string (frameaddr), expr);
>
> -  var = varobj_create (name, expr, frameaddr, var_type);
> +  var = varobj_create (name, expr, frameaddr, var_type,
> +		       available_children_only);
>
>     if (var == NULL)
>       error (_("-var-create: unable to create variable object"));
> @@ -328,12 +360,43 @@ mi_cmd_var_info_num_children (char *command, char **argv, int argc)
>   {
>     struct ui_out *uiout = current_uiout;
>     struct varobj *var;
> +  int available_children_only = 0;
> +  int oind = 0;
> +  enum opt
> +    {
> +      AVAILABLE_CHILDREN_ONLY,
> +    };
> +  static const struct mi_opt opts[] =
> +    {
> +      {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
> +      { 0, 0, 0 }
> +    };
>
> -  if (argc != 1)
> -    error (_("-var-info-num-children: Usage: NAME."));
> +  /* Parse arguments.  In this instance we are just looking for
> +     --available_children_only.  */
> +  while (1)
> +    {
> +      char *oarg;
> +      int opt = mi_getopt ("-var-info-num-children", argc, argv,
> +			   opts, &oind, &oarg);
> +      if (opt < 0)
> +	break;
> +      switch ((enum opt) opt)
> +	{
> +	case AVAILABLE_CHILDREN_ONLY:
> +	  available_children_only = 1;
> +	  break;
> +	}
> +    }
> +
> +  if (argc - oind != 1)
> +    error (_("-var-info-num-children: Usage: "
> +	     "[--available-children-only] NAME."));

This statement spans multiple lines and should be enclosed in braces ("{}").

>
>     /* Get varobj handle, if a valid var obj name was specified.  */
> -  var = varobj_get_handle (argv[0]);
> +  var = varobj_get_handle (argv[oind]);
> +
> +  varobj_set_available_children_only (var, available_children_only);
>
>     ui_out_field_int (uiout, "numchild", varobj_get_num_children (var));
>   }
> @@ -381,18 +444,47 @@ mi_cmd_var_list_children (char *command, char **argv, int argc)
>     int ix;
>     int from, to;
>     char *display_hint;
> +  int available_children_only = 0;
> +  int oind = 0;
> +  enum opt
> +    {
> +      AVAILABLE_CHILDREN_ONLY,
> +    };
> +  static const struct mi_opt opts[] =
> +    {
> +      {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
> +      { 0, 0, 0 }
> +    };
> +
> +  /* Parse arguments.  In this instance we are just looking for
> +     --available_children_only.  */
> +  while (1)
> +    {
> +      char *oarg;
> +      int opt = mi_getopt_allow_unknown ("-var-list-children", argc, argv,
> +					 opts, &oind, &oarg);
> +
> +      if (opt < 0)
> +	break;
> +      switch ((enum opt) opt)
> +	{
> +	case AVAILABLE_CHILDREN_ONLY:
> +	  available_children_only = 1;
> +	  break;
> +	}
> +    }
>
> -  if (argc < 1 || argc > 4)
> -    error (_("-var-list-children: Usage: "
> +  if (argc - oind < 1 || argc - oind > 4)
> +    error (_("-var-list-children: Usage: [--available-children-only] "
>   	     "[PRINT_VALUES] NAME [FROM TO]"));
>
>     /* Get varobj handle, if a valid var obj name was specified.  */
> -  if (argc == 1 || argc == 3)
> -    var = varobj_get_handle (argv[0]);
> +  if (argc - oind == 1 || argc - oind == 3)
> +    var = varobj_get_handle (argv[oind]);
>     else
> -    var = varobj_get_handle (argv[1]);
> +    var = varobj_get_handle (argv[oind + 1]);
>
> -  if (argc > 2)
> +  if (argc - oind > 2)
>       {
>         from = atoi (argv[argc - 2]);
>         to = atoi (argv[argc - 1]);
> @@ -403,10 +495,12 @@ mi_cmd_var_list_children (char *command, char **argv, int argc)
>         to = -1;
>       }
>
> +  varobj_set_available_children_only (var, available_children_only);
> +
>     children = varobj_list_children (var, &from, &to);
>     ui_out_field_int (uiout, "numchild", to - from);
> -  if (argc == 2 || argc == 4)
> -    print_values = mi_parse_print_values (argv[0]);
> +  if (argc - oind == 2 || argc - oind == 4)
> +    print_values = mi_parse_print_values (argv[oind]);
>     else
>       print_values = PRINT_NO_VALUES;
>
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 0e0be6c..1fd9fd2 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -119,6 +119,10 @@ struct varobj_dynamic
>        can avoid that.  */
>     int children_requested;
>
> +  /* True if this varobj only includes available/collected objects in
> +     its list of children.  */
> +  int available_children_only;
> +
>     /* The pretty-printer constructor.  If NULL, then the default
>        pretty-printer will be looked up.  If None, then no
>        pretty-printer will be installed.  */
> @@ -175,9 +179,9 @@ create_child_with_value (struct varobj *parent, int index,
>
>   /* Utility routines */
>
> -static struct varobj *new_variable (void);
> +static struct varobj *new_variable (int available_children_only);
>
> -static struct varobj *new_root_variable (void);
> +static struct varobj *new_root_variable (int available_children_only);
>
>   static void free_variable (struct varobj *var);
>
> @@ -285,14 +289,14 @@ find_frame_addr_in_frame_chain (CORE_ADDR frame_addr)
>   }
>
>   struct varobj *
> -varobj_create (char *objname,
> -	       char *expression, CORE_ADDR frame, enum varobj_type type)
> +varobj_create (char *objname, char *expression, CORE_ADDR frame,
> +	       enum varobj_type type, int available_children_only)
>   {
>     struct varobj *var;
>     struct cleanup *old_chain;
>
>     /* Fill out a varobj structure for the (root) variable being constructed.  */
> -  var = new_root_variable ();
> +  var = new_root_variable (available_children_only);
>     old_chain = make_cleanup_free_variable (var);
>
>     if (expression != NULL)
> @@ -911,6 +915,23 @@ varobj_get_num_children (struct varobj *var)
>     return var->num_children >= 0 ? var->num_children : 0;
>   }
>

Missing comment to describe below function. ["See description in varobj.h."]

> +void
> +varobj_set_available_children_only (struct varobj *var, int available_only)
> +{
> +  if (var->dynamic->available_children_only != available_only)
> +    {
> +      /* If there are any children now, wipe them.  */
> +      varobj_delete (var, NULL, /* children only */ 1);
> +      var->num_children = -1;
> +      var->dynamic->available_children_only = available_only;
> +
> +      /* We're starting over, so get rid of any iterator.  */
> +      varobj_iter_delete (var->dynamic->child_iter);
> +      var->dynamic->child_iter = NULL;
> +      varobj_clear_saved_item (var->dynamic);
> +    }
> +}
> +
>   /* Creates a list of the immediate children of a variable object;
>      the return code is the number of such children or -1 on error.  */
>
> @@ -1071,7 +1092,8 @@ varobj_get_attributes (struct varobj *var)
>   int
>   varobj_is_dynamic_p (struct varobj *var)
>   {
> -  return var->dynamic->pretty_printer != NULL;
> +  return (var->dynamic->pretty_printer != NULL
> +	  || var->dynamic->available_children_only);
>   }
>
>   char *
> @@ -2049,7 +2071,7 @@ create_child_with_value (struct varobj *parent, int index,
>     struct varobj *child;
>     char *childs_name;
>
> -  child = new_variable ();
> +  child = new_variable (parent->dynamic->available_children_only);
>
>     /* NAME is allocated by caller.  */
>     child->name = item->name;
> @@ -2086,8 +2108,9 @@ create_child_with_value (struct varobj *parent, int index,
>    */
>
>   /* Allocate memory and initialize a new variable.  */
> +
>   static struct varobj *
> -new_variable (void)
> +new_variable (int available_children_only)
>   {
>     struct varobj *var;
>
> @@ -2116,15 +2139,17 @@ new_variable (void)
>     var->dynamic->pretty_printer = 0;
>     var->dynamic->child_iter = 0;
>     var->dynamic->saved_item = 0;
> +  var->dynamic->available_children_only = available_children_only;
>
>     return var;
>   }
>
>   /* Allocate memory and initialize a new root variable.  */
> +
>   static struct varobj *
> -new_root_variable (void)
> +new_root_variable (int available_children_only)
>   {
> -  struct varobj *var = new_variable ();
> +  struct varobj *var = new_variable (available_children_only);
>
>     var->root = (struct varobj_root *) xmalloc (sizeof (struct varobj_root));
>     var->root->lang_ops = NULL;
> @@ -2396,7 +2421,8 @@ value_of_root (struct varobj **var_handle, int *type_changed)
>         char *old_type, *new_type;
>
>         tmp_var = varobj_create (NULL, var->name, (CORE_ADDR) 0,
> -			       USE_SELECTED_FRAME);
> +			       USE_SELECTED_FRAME,
> +			       var->dynamic->available_children_only);
>         if (tmp_var == NULL)
>   	{
>   	  return NULL;
> @@ -2756,7 +2782,8 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
>         /* Try to create a varobj with same expression.  If we succeed
>   	 replace the old varobj, otherwise invalidate it.  */
>         tmp_var = varobj_create (NULL, var->name, (CORE_ADDR) 0,
> -			       USE_CURRENT_FRAME);
> +			       USE_CURRENT_FRAME,
> +			       var->dynamic->available_children_only);
>         if (tmp_var != NULL)
>   	{
>   	  tmp_var->obj_name = xstrdup (var->obj_name);
> diff --git a/gdb/varobj.h b/gdb/varobj.h
> index 60ace6f..9beec79 100644
> --- a/gdb/varobj.h
> +++ b/gdb/varobj.h
> @@ -225,7 +225,8 @@ const struct lang_varobj_ops ada_varobj_ops;
>
>   extern struct varobj *varobj_create (char *objname,
>   				     char *expression, CORE_ADDR frame,
> -				     enum varobj_type type);
> +				     enum varobj_type type,
> +				     int available_children_only);
>
>   extern char *varobj_gen_name (void);
>
> @@ -310,6 +311,14 @@ extern int varobj_is_dynamic_p (struct varobj *var);
>
>   extern  struct cleanup *varobj_ensure_python_env (struct varobj *var);
>
> +/* Marks the varobj VAR as listing available children only or not,
> +   depending on the boolean AVAILABLE_ONLY.  If the
> +   available-only-ness of the varobj changes compared to its present
> +   state with this call, the varobj's current list of children is
> +   deleted.  */
> +extern void varobj_set_available_children_only (struct varobj *var,
> +						int available_only);
> +
>   extern int varobj_default_value_is_changeable_p (struct varobj *var);
>   extern int varobj_value_is_changeable_p (struct varobj *var);
>
>

Re: [PATCH 08/11] Iterator varobj_items by their availability

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> This patch adds a new implementation of varobj iterator, which is
> based on the availability of children.
>
> gdb:
>
> 2013-10-24  Pedro Alves  <pedro@codesourcery.com>
> 	    Yao Qi  <yao@codesourcery.com>
>
> 	* Makefile.in (SFILES): Add varobj-iter-avail.c.
> 	(COMMON_OBS): Add varobj-iter-avail.o.
> 	* varobj-iter-avail.c: New file.
> 	* varobj-iter.h (avail_varobj_get_iterator): Declare.
> 	* varobj.c (dynamic_varobj_has_child_method):

No changes to dynamic_varobj_has_child_method that I can see.
> 	(varobj_get_iterator): Add one more argument 'lang_ops' and
> 	Caller update.
> 	Return iterator for available children.
> ---
>   gdb/Makefile.in         |    4 +-
>   gdb/varobj-iter-avail.c |  160 +++++++++++++++++++++++++++++++++++++++++++++++
>   gdb/varobj-iter.h       |    4 +
>   gdb/varobj.c            |    8 ++-
>   4 files changed, 172 insertions(+), 4 deletions(-)
>   create mode 100644 gdb/varobj-iter-avail.c
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 7d8a365..6d84def 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -772,7 +772,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
>   	ui-out.c utils.c ui-file.h ui-file.c \
>   	user-regs.c \
>   	valarith.c valops.c valprint.c value.c varobj.c common/vec.c \
> -	xml-tdesc.c xml-support.c \
> +	varobj-iter-avail.c xml-tdesc.c xml-support.c \
>   	inferior.c gdb_usleep.c \
>   	record.c record-full.c gcore.c \
>   	jit.c \
> @@ -933,7 +933,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
>   	ada-lang.o c-lang.o d-lang.o f-lang.o objc-lang.o \
>   	ada-tasks.o ada-varobj.o c-varobj.o \
>   	ui-out.o cli-out.o \
> -	varobj.o vec.o \
> +	varobj.o varobj-iter-avail.o vec.o \
>   	go-lang.o go-valprint.o go-typeprint.o \
>   	jv-lang.o jv-valprint.o jv-typeprint.o jv-varobj.o \
>   	m2-lang.o opencl-lang.o p-lang.o p-typeprint.o p-valprint.o \
> diff --git a/gdb/varobj-iter-avail.c b/gdb/varobj-iter-avail.c
> new file mode 100644
> index 0000000..0cd2e23
> --- /dev/null
> +++ b/gdb/varobj-iter-avail.c
> @@ -0,0 +1,160 @@
> +/* Copyright (C) 2013 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/>.  */
> +
> +#include "defs.h"
> +#include "gdb_assert.h"
> +#include "value.h"
> +#include "valprint.h"
> +#include "varobj.h"
> +#include "varobj-iter.h"
> +
> +/* A dynamic varobj iterator "class" for available-children-only
> +   varobjs.  This inherits struct varobj_iter.  */
> +
> +struct avail_varobj_iter
> +{
> +  /* The 'base class'.  */
> +  struct varobj_iter base;
> +
> +  const struct lang_varobj_ops *lang_ops;

This field should also have a (trivial) comment.

> +};
> +
> +/* Returns true if VAL is not interesting for an
> +   available-children-only varobj.  */
> +
> +static int
> +varobj_value_unavailable (struct value *val)
> +{
> +  volatile struct gdb_exception e;
> +  int unavail = 0;
> +
> +  gdb_assert (val != NULL);
> +
> +  /* value_entirely_unavailable may need to try reading the value,
> +     which may throw for example, a memory error, e.g., when not
> +     inspecting a traceframe, we try to read the pointee of a dangling
> +     or NULL pointer.  */
> +  TRY_CATCH (e, RETURN_MASK_ERROR)
> +    {
> +      struct type *type = value_type (val);
> +
> +      unavail = (value_entirely_unavailable (val)
> +		 /* A scalar object that does not have all bits available
> +		    is also considered unavailable, because all bits
> +		    contribute to its representation.  */
> +		 || (val_print_scalar_type_p (type)
> +		     && !value_bytes_available (val,
> +						value_embedded_offset (val),
> +						TYPE_LENGTH (type))));
> +    }
> +  if (e.reason < 0)
> +    return 1;
> +  else
> +    return unavail;
> +}
> +
> +/* Implementation of the `next' method for available-children-only
> +   varobj iterators.  */
> +
> +static varobj_item *
> +avail_varobj_iter_next (struct varobj_iter *self)
> +{
> +  struct avail_varobj_iter *dis = (struct avail_varobj_iter *) self;
> +  int num_children = dis->lang_ops->number_of_children (self->var);
> +  int raw_index = self->next_raw_index;
> +  varobj_item *item = NULL;
> +
> +  for (; raw_index < num_children; raw_index++)
> +    {
> +      struct value *child_value
> +	= dis->lang_ops->value_of_child (self->var, raw_index);
> +
> +      /* The "fake" children will have NULL values.  */
> +      if (child_value == NULL || !varobj_value_unavailable (child_value))
> +	{
> +	  item = xmalloc (sizeof *item);
> +
> +	  item->name
> +	    = dis->lang_ops->name_of_child (self->var, raw_index);
> +	  item->value = child_value;
> +
> +	  raw_index++;
> +	  self->next_raw_index = raw_index;
> +	  break;
> +	}
> +    }
> +  return item;
> +}
> +
> +/* Implementation of the `dtor' method of available-children-only
> +   varobj iterators.  */
> +
> +static void
> +avail_varobj_iter_dtor (struct varobj_iter *self)
> +{
> +  /* nothing to do */
> +}
> +
> +/* The 'vtable' of available-children-only varobj iterators.  */
> +
> +static const struct varobj_iter_ops avail_varobj_iter_ops =
> +{
> +  avail_varobj_iter_dtor,
> +  avail_varobj_iter_next
> +};
> +
> +/* Constructor of available-children-only varobj iterators.  VAR is
> +   the varobj whose children the iterator will be iterating over.  */
> +
> +static void
> +avail_varobj_iter_ctor (struct avail_varobj_iter *self, struct varobj *var,
> +			const struct lang_varobj_ops *lang_ops)
> +{
> +  self->base.var = var;
> +  self->base.ops = &avail_varobj_iter_ops;
> +  self->base.next_raw_index = 0;
> +  self->lang_ops = lang_ops;
> +}

Missing a newline here.

> +/* Allocate and construct an available-children-only varobj iterator.
> +   VAR is the varobj whose children the iterator will be iterating
> +   over.  */
> +
> +static struct avail_varobj_iter *
> +avail_varobj_iter_new (struct varobj *var, const struct lang_varobj_ops *lang_ops)
> +{
> +  struct avail_varobj_iter *self;
> +
> +  self = xmalloc (sizeof (struct avail_varobj_iter));

Please use XNEW.

> +  avail_varobj_iter_ctor (self, var, lang_ops);
> +  return self;
> +}
> +
> +/* Return a new available-children-only varobj iterator suitable to
> +   iterate over VAR's children.  */
> +
> +struct varobj_iter *
> +avail_varobj_get_iterator (struct varobj *var,
> +			   const struct lang_varobj_ops *lang_ops)
> +{
> +  struct avail_varobj_iter *iter;
> +
> +  /* Avoid the needless allocation/dealocation of the iterator if
                                       ^^^^^^^^^^^

typo

> +     there are no raw children to iterator over anyway.  */
                                      ^^^^^^^^
"iterate"

> +  if (lang_ops->number_of_children (var) == 0)
> +    return NULL;
> +
> +  iter = avail_varobj_iter_new (var, lang_ops);
> +  return &iter->base;
> +}
> diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
> index b4bda82..c466225 100644
> --- a/gdb/varobj-iter.h
> +++ b/gdb/varobj-iter.h
> @@ -68,3 +68,7 @@ struct varobj_iter_ops
>   	  xfree (ITER);		       \
>   	}				       \
>       } while (0)
> +
> +struct varobj_iter *
> +  avail_varobj_get_iterator (struct varobj *var,
> +			     const struct lang_varobj_ops *lang_ops);

Needs a comment.

> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 1fd9fd2..ba93eb5 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -768,8 +768,11 @@ dynamic_varobj_has_child_method (struct varobj *var)
>      suitable for iterating over VAR's children.  */
>
>   static struct varobj_iter *
> -varobj_get_iterator (struct varobj *var)
> +varobj_get_iterator (struct varobj *var, const struct lang_varobj_ops *lang_ops)
>   {
> +  if (var->dynamic->available_children_only)
> +    return avail_varobj_get_iterator (var, lang_ops);
> +
>   #if HAVE_PYTHON
>     if (var->dynamic->pretty_printer)
>       return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
> @@ -810,7 +813,8 @@ update_dynamic_varobj_children (struct varobj *var,
>     if (update_children || var->dynamic->child_iter == NULL)
>       {
>         varobj_iter_delete (var->dynamic->child_iter);
> -      var->dynamic->child_iter = varobj_get_iterator (var);
> +      var->dynamic->child_iter = varobj_get_iterator (var,
> +						      var->root->lang_ops);
>
>         varobj_clear_saved_item (var->dynamic);
>
>

Re: [PATCH 10/11] Match dynamic="1" in the output of -var-list-children

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> I am not satisfied with the regexp construction for each child in
> mi_child_regexp.

Agreed.

> mi_list_varobj_children "struct_declarations" {
>      {struct_declarations.integer integer 0 int}
> } "test"
>
> will be rewritten to:
>
> mi_list_varobj_children "struct_declarations" {
>      {struct_declarations.integer integer 0 {type int}}
> } "test"
>
> if we want to match "dynamic" attribute, we can write:
>
> mi_list_varobj_children "struct_declarations" {
>      {struct_declarations.integer integer 0 {type int dynamic 1}}
> } "test"
>

That sounds reasonable. A slightly easier syntax to parse would use a 
list of lists for this optional argument. That would then generically 
allow flags to be used, further simplifying it to "{{type int} dynamic}".

But then just about anything is better than what we have now, which 
relies on the /length/ of the list to determine what to add to the 
regexp. Yuck!

> Since mi_list_varobj_children has been widely used in test suite, I'd
> like to defer the change after this patch series.

Agreed.

Just one little thing below. Otherwise, this looks okay to me.

Keith

> 2013-11-24  Yao Qi  <yao@codesourcery.com>
>
> 	* lib/mi-support.exp (mi_child_regexp): Append 'dynamic="1"' to
> 	children_exp.
> ---
>   gdb/testsuite/lib/mi-support.exp |   16 ++++++++++------
>   1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index 0c3cdbe..99ff9a5 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -1410,21 +1410,25 @@ proc mi_child_regexp {children add_child} {
>   	set name [lindex $item 0]
>   	set exp [lindex $item  1]
>   	set numchild [lindex $item 2]
> +
> +	set line "$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\""
> +
>   	if {[llength $item] == 5} {
>   	    set type [lindex $item 3]
>   	    set value [lindex $item 4]
>
> -	    lappend children_exp\
> -		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\",value=\"$value\",type=\"$type\"(,thread-id=\"\[0-9\]+\")?}"
> +	    append line ",value=\"$value\",type=\"$type\""
>   	} elseif {[llength $item] == 4} {
>   	    set type [lindex $item 3]
>
> -	    lappend children_exp\
> -		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\",type=\"$type\"(,thread-id=\"\[0-9\]+\")?}"
> +	    append line ",type=\"$type\""
>   	} else {
> -	    lappend children_exp\
> -		"$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\"(,thread-id=\"\[0-9\]+\")?}"
>   	}

This 'else' block is now empty and can be removed.

> +
> +	append line \
> +	    "(,thread-id=\"\[0-9\]+\")?(,dynamic=\"1\")?}"
> +
> +	lappend children_exp $line
>       }
>       return [join $children_exp ","]
>   }
>

Re: [PATCH 09/11] Delete varobj's children on traceframe is changed.

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> In this patch, we install a traceframe_changed observer to clear
> varobjs marked as 'available_children_only'.
>

Once again, only some minor comments.

Keith

> gdb:
>
> 2013-11-24  Yao Qi  <yao@codesourcery.com>
>
> 	* varobj.c: Include "observer.h".
> 	(varobj_delete_if_available_children_only): New function.
> 	(varobj_traceframe_changed): New function.
> 	(_initialize_varobj): Install varobj_traceframe_changed to
> 	traceframe_changed observer.
> ---
>   gdb/varobj.c |   29 +++++++++++++++++++++++++++++
>   1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index ba93eb5..4b201df 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -24,6 +24,7 @@
>   #include "gdbcmd.h"
>   #include "block.h"
>   #include "valprint.h"
> +#include "observer.h"
>
>   #include "gdb_assert.h"
>   #include <string.h>
> @@ -2749,6 +2750,32 @@ all_root_varobjs (void (*func) (struct varobj *var, void *data), void *data)
>         (*func) (var_root->rootvar, data);
>       }
>   }
> +
> +/* Delete VAR's children if it is marked as 'available_children_only'.  */
> +
> +static void
> +varobj_delete_if_available_children_only (struct varobj *var, void *data)
> +{
> +  if (var->dynamic->available_children_only)
> +    {
> +      varobj_delete (var, NULL, /* children only */ 1);
> +      var->num_children = -1;
> +
> +      /* We're starting over, so get rid of any iterator.  */

This comment doesn't really make sense here. Best to just get rid of it.
[cut-n-paste-o?]

> +      varobj_iter_delete (var->dynamic->child_iter);
> +      var->dynamic->child_iter = NULL;
> +      varobj_clear_saved_item (var->dynamic);
> +    }
> +}
> +

The above code looks like it is duplicated in 
varobj_set_available_children_only. That suggests that a common function 
could be introduced to do this.

> +/* Installed on traceframe_changed observer.  */

"The callback installed for traceframe_changed events."

> +
> +static void
> +varobj_traceframe_changed (int tfnum, int tpnum)
> +{
> +  all_root_varobjs (varobj_delete_if_available_children_only , NULL);
> +}
> +
>   \f
>   extern void _initialize_varobj (void);
>   void
> @@ -2766,6 +2793,8 @@ _initialize_varobj (void)
>   			     _("When non-zero, varobj debugging is enabled."),
>   			     NULL, show_varobjdebug,
>   			     &setlist, &showlist);
> +
> +  observer_attach_traceframe_changed (varobj_traceframe_changed);
>   }
>
>   /* Invalidate varobj VAR if it is tied to locals and re-create it if it is
>

Re: [PATCH 11/11] Test case

[Keith Seitz]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> This patch adds two test cases for the new option.
> mi-available-children-only-cxx.exp focuses on the C++ fake children.
>
> gdb/testsuite:
>
> 2013-11-24  Yao Qi  <yao@codesourcery.com>
>
> 	* gdb.trace/available-children-only.c: New.
> 	* gdb.trace/mi-available-children-only.exp: New.
> 	* gdb.trace/mi-available-children-only-cxx.exp: New.
> 	* gdb.trace/available-children-only.cc: New.

I believe the two .exp files collide on 8.3 systems and might require an 
entry in config/djgpp/fnchange.lst, but I know just enough to get myself 
into trouble. Perhaps someone more familiar will offer an opinion.

Once again, this looks okay to me; just some small nits again. Most of 
these nits are me trying to exert some sort of `control' over the style 
of the test suite's Tcl code, so consider them especially minor. :-)

Keith

> ---
>   gdb/testsuite/gdb.trace/available-children-only.c  |   69 +++++++
>   gdb/testsuite/gdb.trace/available-children-only.cc |   45 +++++
>   .../gdb.trace/mi-available-children-only-cxx.exp   |  126 +++++++++++++
>   .../gdb.trace/mi-available-children-only.exp       |  198 ++++++++++++++++++++
>   4 files changed, 438 insertions(+), 0 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.trace/available-children-only.c
>   create mode 100644 gdb/testsuite/gdb.trace/available-children-only.cc
>   create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
>   create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only.exp
>
> diff --git a/gdb/testsuite/gdb.trace/available-children-only.c b/gdb/testsuite/gdb.trace/available-children-only.c
> new file mode 100644
> index 0000000..1697a8b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/available-children-only.c
> @@ -0,0 +1,69 @@
> +/* Copyright 2013 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/>.  */
> +
> +/* Two trace actions are set, and they collect different fields to
> +   reproduce the case that children appear in the different orders.  */
> +
> +struct simple
> +{
> +  int a; /* Collected by both.  */
> +  int b; /* Collected by action 2.  */
> +  struct
> +  {
> +    struct
> +    {
> +      int g; /* Collected by action 1.  */
> +      int h; /* Collected by action 2.  */
> +    } s3;
> +    int d; /* Collected by action 1.  */
> +  } s1;
> +
> +  struct
> +  {
> +    int e;
> +    int f; /* Collected by action 1.  */
> +  } s2;
> +};
> +
> +struct simple s;
> +
> +static void
> +marker1 (void)
> +{
> +}
> +
> +static void
> +marker2 (void)
> +{
> +}
> +
> +static void
> +end (void)
> +{
> +}
> +
> +int
> +main (void)
> +{
> +  marker1 ();
> +
> +  marker2 ();
> +
> +  end ();
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.trace/available-children-only.cc b/gdb/testsuite/gdb.trace/available-children-only.cc
> new file mode 100644
> index 0000000..2ff4a4c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/available-children-only.cc
> @@ -0,0 +1,45 @@
> +/* Copyright 2013 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/>.  */
> +
> +class Fake
> +{
> + private:
> +  int pri1; /* Collected.  */
> +
> + public:
> +  int pub1;
> +
> +};
> +
> +Fake fake;
> +
> +static void
> +marker (void)
> +{}
> +
> +static void
> +end (void)
> +{}
> +
> +int
> +main (void)
> +{
> +  marker ();
> +
> +  end ();
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp b/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
> new file mode 100644
> index 0000000..f72a13f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
> @@ -0,0 +1,126 @@
> +# Copyright 2013 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/>.
> +
> +# Test for C++ fake children.
> +
> +load_lib mi-support.exp
> +load_lib trace-support.exp
> +set MIFLAGS "-i=mi"
> +
> +standard_testfile available-children-only.cc
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +	   executable {debug c++}] != "" } {
> +    untested "Couldn't compile ${srcfile}"

Just FYI: none of the braces here are not needed. I see this often in 
the test suite. Tcl is not sh! :-P

> +    return -1
> +}
> +
> +# Test target supports tracepoints or not.
> +
> +clean_restart $testfile
> +
> +if ![runto_main] {

Please add braces around the condition to 'if' statements. I know this 
is something that is missing all over the test suite, and Tcl doesn't 
complain, but it does make life much easier for the interpreter.

> +    fail "Can't run to main to check for trace support"
> +    return -1
> +}
> +
> +if ![gdb_target_supports_trace] {

Likewise.

> +    unsupported "Current target does not support trace"
> +    return -1
> +}
> +gdb_exit
> +
> +if [mi_gdb_start] {
> +    continue

Likewise.

> +}
> +
> +mi_run_to_main
> +
> +mi_gdb_test "-break-insert -a marker" "\\^done.*" \
> +    "trace marker"

Can mi_create_breakpoint be used here?

> +
> +mi_gdb_test "-break-commands 2 \"collect fake.pri1\" \"end\" " \
> +    {\^done} "set action on marker"

Is there a way to not hard code the breakpoint number? I think $bpnum is 
still accessible (or number is mentioned in ^done,bkpt={number="X",...)

> +
> +mi_gdb_test "-trace-start" ".*\\^done.*" "trace start"
> +mi_continue_to end
> +mi_gdb_test "-trace-stop" "\\^done.*" "trace stop"
> +# Save trace frames to trace file.
> +set tracefile [standard_output_file ${testfile}]
> +mi_gdb_test "-trace-save ${tracefile}.tfile" \
> +    ".*" \
> +    "save tfile trace"
> +
> +# In live target, '--available-children-only' shouldn't have any
> +# effects.
> +
> +mi_gdb_test "-trace-find frame-number 0" \
> +    ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
> +    "-trace-find frame-number 0"
> +
> +mi_gdb_test "-var-create --available-children-only fake @ fake" \
> +    {\^done,name="fake",numchild="0",value=".*",type="Fake",dynamic="1",has_more="1"} \
> +    "-var-create --available-children-only fake"
> +
> +mi_list_varobj_children { fake --available-children-only } {
> +    { fake.public public 0 }
> +    { fake.private private 0 }
> +} "-var-list-children --available-children-only fake"
> +
> +mi_gdb_test "-var-info-num-children --available-children-only fake" \
> +    "\\^done,numchild=\"2\"" \
> +    "-var-info-num-children --available-children-only fake"
> +
> +mi_gdb_test "-var-delete fake" {\^done,ndeleted="3"} "-var-delete fake"
> +
> +# Select a traceframe, and '--available-children-only' have some
> +# effects.
> +
> +proc check_with_traceframe { } {
> +    global decimal
> +
> +    with_test_prefix "w/ setting traceframe" {
> +	mi_gdb_test "-trace-find frame-number 0" \
> +	    ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
> +	    "-trace-find frame-number 0"
> +
> +	mi_gdb_test "-var-create --available-children-only fake @ fake" \
> +	    {\^done,name="fake",numchild="0",value=".*",type="Fake",dynamic="1",has_more="1"} \
> +	    "-var-create --available-children-only fake"
> +
> +	mi_list_varobj_children "fake" {
> +	    { fake.public public 1 }
> +	    { fake.private private 1 }
> +	} "-var-list-children fake"
> +
> +	mi_gdb_test "-var-info-num-children fake" \
> +	    "\\^done,numchild=\"2\"" "-var-info-num-children fake"
> +
> +	mi_list_varobj_children {"fake" "--available-children-only" } {
> +	    { fake.public public 0 }
> +	    { fake.private private 0 }
> +	} "-var-list-children --available-children-only fake"
> +
> +	mi_gdb_test "-var-info-num-children --available-children-only fake" \
> +	    "\\^done,numchild=\"2\"" \
> +	    "-var-info-num-children --available-children-only fake"
> +
> +	mi_gdb_test "-var-delete fake" {\^done,ndeleted="3"} "-var-delete fake"
> +    }
> +    mi_gdb_test "-trace-find none" ".*\\^done,found=\"0\".*" \
> +	"-trace-find none"
> +}
> +
> +check_with_traceframe
> diff --git a/gdb/testsuite/gdb.trace/mi-available-children-only.exp b/gdb/testsuite/gdb.trace/mi-available-children-only.exp
> new file mode 100644
> index 0000000..73f85d7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/mi-available-children-only.exp
> @@ -0,0 +1,198 @@
> +# Copyright 2013 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/>.
> +
> +load_lib mi-support.exp
> +load_lib trace-support.exp
> +set MIFLAGS "-i=mi"
> +
> +standard_testfile available-children-only.c
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +    untested mi-available-children-only.exp
> +    return -1
> +}
> +
> +# Test target supports tracepoints or not.
> +
> +clean_restart $testfile
> +
> +if ![runto_main] {

and here

> +    fail "Can't run to main to check for trace support"
> +    return -1
> +}
> +
> +if ![gdb_target_supports_trace] {

and here

> +    unsupported "Current target does not support trace"
> +    return -1
> +}
> +gdb_exit
> +
> +if [mi_gdb_start] {

and here

> +    continue
> +}
> +
> +mi_run_to_main
> +mi_gdb_test "-break-insert -a marker1" "\\^done.*" \
> +    "trace marker1"

Can mi_create_breakpoint be used here?

> +
> +mi_gdb_test "-break-commands 2 \"collect s.a\" \"collect s.s1.d\" \"collect s.s1.s3.g\" \"collect s.s2.f\" \"end\" " \
> +    {\^done} "set action on marker1"

Hard-coded breakpoint number again.

> +
> +mi_gdb_test "-break-insert -a marker2" "\\^done.*" \
> +    "trace marker2"
> +
> +mi_gdb_test "-break-commands 3 \"collect s.a\" \"collect s.b\" \"collect s.s1.s3.h\" \"end\" " \
> +    {\^done} "set action on marker2"

Likewise

> +
> +mi_gdb_test "-trace-start" ".*\\^done.*" "trace start"
> +mi_continue_to end
> +mi_gdb_test "-trace-stop" "\\^done.*" "trace stop"
> +
> +# Save trace frames to trace file.
> +set tracefile [standard_output_file ${testfile}]
> +mi_gdb_test "-trace-save ${tracefile}.tfile" \
> +    ".*" \
> +    "save tfile trace"

This file should be deleted when done with it.

> +
> +# In live target, '--available-children-only' shouldn't have any
> +# effects.
> +mi_gdb_test "-var-create --available-children-only s1 @ s" \
> +    {\^done,name="s1",numchild="0",value=".*",type="struct simple",dynamic="1",has_more="1"} \
> +    "-var-create --available-children-only s1"
> +
> +mi_list_varobj_children  { "s1" "--available-children-only" } {
> +    { s1.a a 0 int }
> +    { s1.b b 0 int }
> +    { s1.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
> +    { s1.s2 s2 0 "struct \\{\\.\\.\\.\\}" }
> +} "-var-list-children --available-children-only s1"
> +
> +mi_gdb_test "-var-info-num-children --available-children-only s1" \
> +    "\\^done,numchild=\"4\"" \
> +    "-var-info-num-children --available-children-only s1"
> +
> +mi_gdb_test "-var-delete s1" {\^done,ndeleted="5"} "-var-delete s1"
> +
> +# Select a traceframe, and '--available-children-only' have some
> +# effects.
> +
> +proc check_with_traceframe { } {
> +    global decimal
> +
> +    mi_gdb_test "-trace-find frame-number 0" \
> +	".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
> +	"-trace-find frame-number 0"
> +
> +    with_test_prefix "traceframe 0" {
> +	mi_gdb_test "-var-create --available-children-only s2 @ s" \
> +	    {\^done,name="s2",numchild="0",value=".*",type="struct simple",dynamic="1",has_more="1"} \
> +	    "-var-create --available-children-only s2"
> +
> +	mi_gdb_test "-var-list-children s2" \
> +	    "\\^done,numchild=\"4\",.*,has_more=\"0\"" \
> +	    "-var-list-children s2"
> +
> +	mi_gdb_test "-var-info-num-children s2" \
> +	    "\\^done,numchild=\"4\"" \
> +	    "-var-info-num-children s2"
> +
> +	# "s2" should have children "a", "s1" and "s2".
> +	mi_list_varobj_children  { "s2" "--available-children-only" } {
> +	    { s2.a a 0 int }
> +	    { s2.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
> +	    { s2.s2 s2 0 "struct \\{\\.\\.\\.\\}" }
> +	} "-var-list-children --available-children-only s2"
> +
> +	mi_gdb_test "-var-info-num-children --available-children-only s2" \
> +	    "\\^done,numchild=\"3\"" \
> +	    "-var-info-num-children --available-children-only s2"
> +
> +	mi_list_varobj_children  { "s2.s1" "--available-children-only" } {
> +	    { s2.s1.s3 s3 0 "struct \\{\\.\\.\\.\\}" }
> +	    { s2.s1.d d 0 int }
> +	} "-var-list-children --available-children-only s2.s1"
> +
> +    }
> +
> +    mi_gdb_test "-trace-find frame-number 1" \
> +	".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"1\",frame=\{.*" \
> +	"-trace-find frame-number 1"
> +
> +    with_test_prefix "traceframe 1" {
> +	mi_list_varobj_children  { "s2" "--available-children-only" } {
> +	    { s2.a a 0 int }
> +	    { s2.b b 0 int }
> +	    { s2.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
> +	} "-var-list-children --available-children-only s2"
> +
> +	mi_list_varobj_children  { "s2.s1" "--available-children-only" } {
> +	    { s2.s1.s3 s3 0 "struct \\{\\.\\.\\.\\}" }
> +	} "-var-list-children --available-children-only s2.s1"
> +
> +	mi_list_varobj_children  { "s2.s1.s3" "--available-children-only" } {
> +	    { s2.s1.s3.h h 0 int }
> +	} "-var-list-children --available-children-only s2.s1.s3"
> +
> +	mi_gdb_test "-var-info-num-children --available-children-only s2" \
> +	    "\\^done,numchild=\"3\"" \
> +	    "-var-info-num-children --available-children-only s2"
> +
> +	mi_gdb_test "-var-delete s2" {\^done,ndeleted="6"} "-var-delete s2"
> +    }
> +
> +    mi_gdb_test "-trace-find none" ".*\\^done,found=\"0\".*" \
> +	"-trace-find none"
> +}
> +
> +check_with_traceframe
> +
> +# Test when changing target from remote to ${target}.
> +
> +proc test_from_remote { target } {
> +    global mi_gdb_prompt decimal
> +    global tracefile
> +
> +    with_test_prefix "remote to ${target}" {
> +	# Change target to ${target}.
> +	mi_gdb_test "-target-select ${target} ${tracefile}.${target}" ".*\\^connected.*" \
> +	    "change target to ${target}"
> +
> +	with_test_prefix "w/ setting traceframe" {
> +	    check_with_traceframe
> +	}
> +    }
> +}
> +
> +test_from_remote "tfile"
> +
> +proc test_from_exec { target } {
> +    global binfile
> +    global tracefile
> +
> +    mi_gdb_exit
> +    if [mi_gdb_start] {
> +	return
> +    }
> +    mi_gdb_load ${binfile}
> +
> +    with_test_prefix "exec to ${target}" {
> +	mi_gdb_test "-target-select ${target} ${tracefile}.${target}" ".*\\^connected.*" \
> +	    "change target to ${target}"
> +
> +	check_with_traceframe
> +    }
> +}
> +
> +test_from_exec "tfile"
>

Re: [PATCH 01/11] Use 'struct varobj_item' to represent name and value pair

[Doug Evans]

On Tue, Jan 21, 2014 at 12:43 PM, Keith Seitz <keiths@redhat.com> wrote:
>> 2013-11-24  Yao Qi  <yao@codesourcery.com>
>>
>>         * varobj.c (struct varobj_item): New structure.
>>         (create_child_with_value): Update declaration.
>>         (varobj_add_child): Replace arguments 'name' and 'value' with
>>         'item'.  Callers update.
>
>                          ^^^^^^
> Either "Callers updated." or "Update callers."

"All callers updated".

ref: grep -i "caller.*up" ChangeLog* | wc ; grep -i "all callers
updated" ChangeLog* | wc

Re: [PATCH 02/11] Generalize varobj iterator

[Doug Evans]

On Tue, Jan 21, 2014 at 12:43 PM, Keith Seitz <keiths@redhat.com> wrote:
> [...]
>> diff --git a/gdb/varobj.h b/gdb/varobj.h
>> index 978d9b9..0f68311 100644
>> --- a/gdb/varobj.h
>> +++ b/gdb/varobj.h
>> @@ -308,6 +308,8 @@ extern int varobj_has_more (struct varobj *var, int
>> to);
>>
>>   extern int varobj_pretty_printed_p (struct varobj *var);
>>
>> +extern  struct cleanup *varobj_ensure_python_env (struct varobj *var);
>> +
>
>
> IIRC, we are now adding documentation near declarations, too. Many are often
> as simple as "See description in varobj.c."

I could be wrong of course, but the rules are either:

1) Document at point of definition, and nothing at point of declaration.

2) Document at point of declaration, and add a comment at point of
definition saying something along the lines of "See foo.h".
ref: grep -F '/* See' *.c

As for which to pick, I can argue either point, and don't impose a
particular choice, as long as the choice isn't imposed on me either.
doxygen can make this moot so I'm not worrying about it too much at the moment.

Re: [PATCH 01/11] Use 'struct varobj_item' to represent name and value pair

[Yao Qi]

On 01/22/2014 09:00 AM, Doug Evans wrote:
> "All callers updated".

"All" is recommended in GNU coding standards too.

http://www.gnu.org/prep/standards/standards.html#Change-Logs

-- 
Yao (齐尧)

Re: [PATCH 01/11] Use 'struct varobj_item' to represent name and value pair

[Doug Evans]

On Wed, Jan 22, 2014 at 8:06 PM, Yao Qi <yao@codesourcery.com> wrote:
> On 01/22/2014 09:00 AM, Doug Evans wrote:
>> "All callers updated".
>
> "All" is recommended in GNU coding standards too.
>
> http://www.gnu.org/prep/standards/standards.html#Change-Logs

Yeah, I would have mentioned it except, I like opportunities to show
monkey-see-monkey-do hacking when it comes to following coding
standards often Just Works.
:-)