[python] another round of fixlets

[python] another round of fixlets

[Tom Tromey]

I'm checking this in on the python branch.

This fixes a few more oddities pointed out by Vladimir, either in his
note here earlier today, or in the thread on the gdb list.

In particular:

* We are better about computing the "numchild" result
* We emit "{...}" as the value of a varobj that has children
* We emit "has_more" for -var-create; this can be used to discover
  whether the varobj has dynamic children

Vladimir, I made up this last one to allow discoverability of dynamic
children by the MI user.  Maybe it isn't the best way to go ... let me
know what you think, we can certainly do something else here.

Tom

2009-08-19  Tom Tromey  <tromey@redhat.com>

	* mi/mi-cmd-var.c (mi_cmd_var_create): Emit 'has_more' field.
	(mi_cmd_var_list_children): Properly compute 'numchild' field.
	* varobj.c (varobj_get_num_children): Call
	update_dynamic_varobj_children.
	(value_get_print_value): Likewise.  Return "{...}" if varobj has
	children.

2009-08-19  Tom Tromey  <tromey@redhat.com>

	* gdb.mi/mi-var-cmd.exp: Update.
	* gdb.mi/mi-break.exp (test_error): Update.

diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 73c6482..6f488b8 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -142,6 +142,8 @@ mi_cmd_var_create (char *command, char **argv, int argc)
 
   print_varobj (var, PRINT_ALL_VALUES, 0 /* don't print expression */);
 
+  ui_out_field_int (uiout, "has_more", varobj_has_more (var, 0));
+
   do_cleanups (old_cleanups);
 }
 
@@ -408,7 +410,7 @@ mi_cmd_var_list_children (char *command, char **argv, int argc)
     }
 
   children = varobj_list_children (var, &from, &to);
-  ui_out_field_int (uiout, "numchild", VEC_length (varobj_p, children));
+  ui_out_field_int (uiout, "numchild", to - from);
   if (argc == 2 || argc == 4)
     print_values = mi_parse_values_option (argv[0]);
   else
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 619727d..8d06c0e 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -175,7 +175,7 @@ proc test_error {} {
     # containing function call, the internal breakpoint created to handle
     # function call would be reported, messing up MI output.
     mi_gdb_test "-var-create V * return_1()" \
-        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\"" \
+        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
         "create varobj for function call"
 
     mi_gdb_test "-var-update *" \
diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
index ad2e55c..31238c2 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
@@ -377,7 +377,7 @@ mi_gdb_test "-var-update *" \
 	"assign same value to func (update)"
 
 mi_gdb_test "-var-create array_ptr * array_ptr" \
-	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex\",type=\"int \\*\"" \
+	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex\",type=\"int \\*\",has_more=\"0\"" \
 	"create global variable array_ptr"
 
 mi_gdb_test "-var-assign array_ptr array2" \
@@ -596,7 +596,7 @@ mi_check_varobj_value F 7 "check F inside callee"
 # A varobj we fail to read during -var-update should be considered
 # out of scope.
 mi_gdb_test "-var-create null_ptr * **0" \
-    {\^done,name="null_ptr",numchild="0",value=".*",type="int"} \
+    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
     "create null_ptr"
 
 # Allow this to succeed, if address zero is readable, although it
diff --git a/gdb/varobj.c b/gdb/varobj.c
index f0ad1ac..357c990 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1052,23 +1052,21 @@ update_dynamic_varobj_children (struct varobj *var,
 int
 varobj_get_num_children (struct varobj *var)
 {
-  int result = var->num_children;
-
   if (var->num_children == -1)
     {
       if (var->pretty_printer)
 	{
-	  /* If we have a dynamic varobj, don't report -1 children.  */
-	  result = 0;
+	  int dummy;
+
+	  /* If we have a dynamic varobj, don't report -1 children.
+	     So, try to fetch some children first.  */
+	  update_dynamic_varobj_children (var, NULL, NULL, &dummy, 0, 0);
 	}
       else
-	{
-	  var->num_children = number_of_children (var);
-	  result = var->num_children;
-	}
+	var->num_children = number_of_children (var);
     }
 
-  return var->num_children;
+  return var->num_children >= 0 ? var->num_children : 0;
 }
 
 /* Creates a list of the immediate children of a variable object;
@@ -2383,43 +2381,56 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
     struct cleanup *back_to = varobj_ensure_python_env (var);
     PyObject *value_formatter = var->pretty_printer;
 
-    if (value_formatter && PyObject_HasAttr (value_formatter,
-					     gdbpy_to_string_cst))
+    if (value_formatter)
       {
-	char *hint;
-	struct value *replacement;
-	int string_print = 0;
-	PyObject *output = NULL;
-
-	hint = gdbpy_get_display_hint (value_formatter);
-	if (hint)
+	/* First check to see if we have any children at all.  If so,
+	   we simply return {...}.  */
+	if (var->num_children == -1)
 	  {
-	    if (!strcmp (hint, "string"))
-	      string_print = 1;
-	    xfree (hint);
+	    int dummy;
+	    update_dynamic_varobj_children (var, NULL, NULL, &dummy, 0, 0);
 	  }
+	if (var->num_children > 0 || var->saved_item)
+	  return xstrdup ("{...}");
 
-	output = apply_varobj_pretty_printer (value_formatter,
-					      &replacement);
- 	if (output)
-  	  {
- 	    PyObject *py_str = python_string_to_target_python_string (output);
- 	    if (py_str)
- 	      {
- 		char *s = PyString_AsString (py_str);
- 		len = PyString_Size (py_str);
-		thevalue = xmemdup (s, len + 1, len + 1);
- 		Py_DECREF (py_str);
-	      }
- 	    Py_DECREF (output);
-	  }
-	if (thevalue && !string_print)
+	if (PyObject_HasAttr (value_formatter, gdbpy_to_string_cst))
 	  {
-	    do_cleanups (back_to);
-	    return thevalue;
+	    char *hint;
+	    struct value *replacement;
+	    int string_print = 0;
+	    PyObject *output = NULL;
+
+	    hint = gdbpy_get_display_hint (value_formatter);
+	    if (hint)
+	      {
+		if (!strcmp (hint, "string"))
+		  string_print = 1;
+		xfree (hint);
+	      }
+
+	    output = apply_varobj_pretty_printer (value_formatter,
+						  &replacement);
+	    if (output)
+	      {
+		PyObject *py_str
+		  = python_string_to_target_python_string (output);
+		if (py_str)
+		  {
+		    char *s = PyString_AsString (py_str);
+		    len = PyString_Size (py_str);
+		    thevalue = xmemdup (s, len + 1, len + 1);
+		    Py_DECREF (py_str);
+		  }
+		Py_DECREF (output);
+	      }
+	    if (thevalue && !string_print)
+	      {
+		do_cleanups (back_to);
+		return thevalue;
+	      }
+	    if (replacement)
+	      value = replacement;
 	  }
-	if (replacement)
-	  value = replacement;
       }
     do_cleanups (back_to);
   }

Re: [python] another round of fixlets

[Vladimir Prus]

On Thursday 20 August 2009 Tom Tromey wrote:

> I'm checking this in on the python branch.
> 
> This fixes a few more oddities pointed out by Vladimir, either in his
> note here earlier today, or in the thread on the gdb list.
> 
> In particular:
> 
> * We are better about computing the "numchild" result
> * We emit "{...}" as the value of a varobj that has children

I am sorry to be PITA, but something is still wrong. If I create
varobj from a vector that is presently empty, GDB does not notice
when things are pushed into it.

	(gdb) -var-create var0 @ v
	^done,name="var0",numchild="0",value="std::vector of length 0, capacity 0",type="std::vector<int, std::allocator<int> >",thread-id="1",has_more="0"
	(gdb) -exec-next 

This steps over push_back, but:

	(gdb) -var-update --all-values *
	^done,changelist=[]

If I create varobj from a non-empty vector then push_back is handled
fine, as well as pop_back.

Also, notice the string value for the varobj -- it's against "std::vector of length..." :-)
Initially, I was surprised -- I did not expect the string value for std::vector to 
differ depending on the number of children it has *now*. We probbly could use the 
presence of 'children' method in the pretty-printer, or some such, as the criteria?
But on the other hand, showing {...} as string value and no children at all might
be equally confusing... I am not sure what's best here.

> * We emit "has_more" for -var-create; this can be used to discover
>   whether the varobj has dynamic children

> Vladimir, I made up this last one to allow discoverability of dynamic
> children by the MI user.  Maybe it isn't the best way to go ... let me
> know what you think, we can certainly do something else here.

This worked fine for me -- thanks!

- Volodya

Re: [python] another round of fixlets

[Tom Tromey]

>>>>> "Volodya" == Vladimir Prus <vladimir@codesourcery.com> writes:

Volodya> I am sorry to be PITA, but something is still wrong.

Don't worry about it.

I think the chaos is my fault, because I have been lazy about writing
tests.  I will fix these problems but this time I'll start with test
cases in the test suite.

Tom

Re: [python] another round of fixlets

[Tom Tromey]

>>>>> "Volodya" == Vladimir Prus <vladimir@codesourcery.com> writes:

Volodya> 	(gdb) -var-create var0 @ v
Volodya> 	^done,name="var0",numchild="0",value="std::vector of length 0, capacity 0",type="std::vector<int, std::allocator<int> >",thread-id="1",has_more="0"
Volodya> 	(gdb) -exec-next 
Volodya> This steps over push_back, but:
Volodya> 	(gdb) -var-update --all-values *
Volodya> 	^done,changelist=[]

This seems to be intentional:

      /* We probably should not get children of a varobj that has a
	 pretty-printer, but for which -var-list-children was never
	 invoked.  Presumably, such varobj is not yet expanded in the
	 UI, so we need not bother getting it.  */

(I think this comment comes from your original patch from early last
year.)

If I insert a -var-list-children in there, it does work.

I can change this, of course, just let me know if you want that.

Volodya> Also, notice the string value for the varobj -- it's against
Volodya> "std::vector of length..." :-) Initially, I was surprised -- I
Volodya> did not expect the string value for std::vector to differ
Volodya> depending on the number of children it has *now*. We probbly
Volodya> could use the presence of 'children' method in the
Volodya> pretty-printer, or some such, as the criteria?

Yes, I think that is what we must do.

Volodya> But on the other hand, showing {...} as string value and no
Volodya> children at all might be equally confusing... I am not sure
Volodya> what's best here.

We'll have 0 children and has_more==0 -- so I suppose the front end can
display "empty" or something like that.

Tom