MI testsuite failures

MI testsuite failures

[Nick Roberts]

I notice now that my last change introduced several testsuite failures:

  FAIL: gdb.mi/mi2-var-cmd.exp: update all vars: func and lpsimple changed
  FAIL: gdb.mi/mi2-var-cmd.exp: update all vars: linteger changed after assign

These are because GDB notices that string contents have been changed and
just need an extra output field, which I guess is a good thing.


  FAIL: gdb.mi/mi-var-cmd.exp: assign same value to func (update)

This is more subtle and is caused by having two variable objects for
one variable, var1 and var2 say.  Then if they have a common value 10
say and you do:

-var-assign var1 11
-var-assign var2 12

var->updated is set to 1 for each and they are both reported as changed
with -var-update.

However doing -var-update again gives var1 in the chagelist because the
real value is 12 but var->print_value is "11".

I think this is a bug in -var-assign; it should set the variable value
but not interfere with the variable object.  Note that generally if you
have two variable objects for one value and set the value of one with
-var-assign, one will be reported as changed because var->updated is 1,
and the other because the value has changed.  I think the value held
by the variable object shouldn't change until -var-update is issued,
just as is the case if the real value changes during execution.  This is
a patch to do that.

OK to commit?

After this I plan to fix the testsuite failures so I can then
add the field for -var-update.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


2007-01-08  Nick Roberts  <nickrob@snap.net.nz>

	* varobj.h: Declare varobj_set_value as char*.

	* varobj.c (varobj_set_value): Make type char*. Return new value but
	don't install it.

	* mi/mi-cmd-var.c (mi_cmd_var_assign): Don't use varobj_get_value.


Index: varobj.h
===================================================================
RCS file: /cvs/src/src/gdb/varobj.h,v
retrieving revision 1.6
diff -c -p -r1.6 varobj.h
*** varobj.h	17 Dec 2005 22:34:03 -0000	1.6
--- varobj.h	7 Jan 2007 23:27:50 -0000
*************** extern int varobj_get_attributes (struct
*** 93,99 ****
  
  extern char *varobj_get_value (struct varobj *var);
  
! extern int varobj_set_value (struct varobj *var, char *expression);
  
  extern int varobj_list (struct varobj ***rootlist);
  
--- 93,99 ----
  
  extern char *varobj_get_value (struct varobj *var);
  
! extern char* varobj_set_value (struct varobj *var, char *expression);
  
  extern int varobj_list (struct varobj ***rootlist);
  
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.76
diff -c -p -r1.76 varobj.c
*** varobj.c	5 Jan 2007 21:58:48 -0000	1.76
--- varobj.c	7 Jan 2007 23:27:54 -0000
*************** varobj_get_value (struct varobj *var)
*** 797,808 ****
     value of the given expression */
  /* Note: Invokes functions that can call error() */
  
! int
  varobj_set_value (struct varobj *var, char *expression)
  {
    struct value *val;
!   int offset = 0;
!   int error = 0;
  
    /* The argument "expression" contains the variable's new value.
       We need to first construct a legal expression for this -- ugh! */
--- 797,807 ----
     value of the given expression */
  /* Note: Invokes functions that can call error() */
  
! char*
  varobj_set_value (struct varobj *var, char *expression)
  {
    struct value *val;
!   char* print_value;
  
    /* The argument "expression" contains the variable's new value.
       We need to first construct a legal expression for this -- ugh! */
*************** varobj_set_value (struct varobj *var, ch
*** 822,864 ****
  	{
  	  /* We cannot proceed without a valid expression. */
  	  xfree (exp);
! 	  return 0;
  	}
  
-       /* All types that are editable must also be changeable.  */
-       gdb_assert (varobj_value_is_changeable_p (var));
- 
-       /* The value of a changeable variable object must not be lazy.  */
-       gdb_assert (!value_lazy (var->value));
- 
-       /* Need to coerce the input.  We want to check if the
- 	 value of the variable object will be different
- 	 after assignment, and the first thing value_assign
- 	 does is coerce the input.
- 	 For example, if we are assigning an array to a pointer variable we
- 	 should compare the pointer with the the array's address, not with the
- 	 array's content.  */
-       value = coerce_array (value);
- 
-       /* The new value may be lazy.  gdb_value_assign, or 
- 	 rather value_contents, will take care of this.
- 	 If fetching of the new value will fail, gdb_value_assign
- 	 with catch the exception.  */
        if (!gdb_value_assign (var->value, value, &val))
  	return 0;
!      
!       /* If the value has changed, record it, so that next -var-update can
! 	 report this change.  If a variable had a value of '1', we've set it
! 	 to '333' and then set again to '1', when -var-update will report this
! 	 variable as changed -- because the first assignment has set the
! 	 'updated' flag.  There's no need to optimize that, because return value
! 	 of -var-update should be considered an approximation.  */
!       var->updated = install_new_value (var, val, 0 /* Compare values. */);
        input_radix = saved_input_radix;
!       return 1;
      }
  
!   return 0;
  }
  
  /* Returns a malloc'ed list with all root variable objects */
--- 821,839 ----
  	{
  	  /* We cannot proceed without a valid expression. */
  	  xfree (exp);
! 	  return NULL;
  	}
  
        if (!gdb_value_assign (var->value, value, &val))
  	return 0;
! 
!       print_value = value_get_print_value (val, var->format);
        input_radix = saved_input_radix;
! 
!       return print_value;
      }
  
!   return NULL;
  }
  
  /* Returns a malloc'ed list with all root variable objects */
Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.27
diff -c -p -r1.27 mi-cmd-var.c
*** mi/mi-cmd-var.c	8 Dec 2006 04:09:53 -0000	1.27
--- mi/mi-cmd-var.c	7 Jan 2007 23:27:55 -0000
*************** enum mi_cmd_result
*** 447,453 ****
  mi_cmd_var_assign (char *command, char **argv, int argc)
  {
    struct varobj *var;
!   char *expression;
  
    if (argc != 2)
      error (_("mi_cmd_var_assign: Usage: NAME EXPRESSION."));
--- 447,453 ----
  mi_cmd_var_assign (char *command, char **argv, int argc)
  {
    struct varobj *var;
!   char *expression, *print_value;
  
    if (argc != 2)
      error (_("mi_cmd_var_assign: Usage: NAME EXPRESSION."));
*************** mi_cmd_var_assign (char *command, char *
*** 463,472 ****
  
    expression = xstrdup (argv[1]);
  
!   if (!varobj_set_value (var, expression))
      error (_("mi_cmd_var_assign: Could not assign expression to varible object"));
  
!   ui_out_field_string (uiout, "value", varobj_get_value (var));
    return MI_CMD_DONE;
  }
  
--- 463,474 ----
  
    expression = xstrdup (argv[1]);
  
!   print_value = varobj_set_value (var, expression);
! 
!   if (!print_value)
      error (_("mi_cmd_var_assign: Could not assign expression to varible object"));
  
!   ui_out_field_string (uiout, "value", print_value);
    return MI_CMD_DONE;
  }
  

Re: MI testsuite failures [PATCH]

[Nick Roberts]

 > After this I plan to fix the testsuite failures so I can then
 > add the field for -var-update.

Here's a patch with fixes for the testsuite.  I've made a further change to
varobj.c to fix a failure in mi-var-child.exp.  I'm not sure what
saved_input_radix does because currently GDB can return from varobj_set_value
without restoring input_radix.

I've fixed mi-var-cmd.exp and mi-var-display.exp but nor their mi2
counterparts.  I can easily do this but I'd rather just remove the mi2-*.exp
files completely especially with the changes needed for the proposed change to
-var-update.  Is this an option?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

2007-01-08  Nick Roberts  <nickrob@snap.net.nz>

	* varobj.h: Declare varobj_set_value as char*.

	* varobj.c (varobj_set_value): Make type char*. Return new value but
	don't install it.

	* mi/mi-cmd-var.c (mi_cmd_var_assign): Don't use varobj_get_value.

2007-01-08  Nick Roberts  <nickrob@snap.net.nz>

	* gdb.mi/mi-var-cmd.exp: Add fields to changelists for string
	contents changes. After -var-assign, do -var-update before
	-var-evaluate-expression.

	* gdb.mi/mi-var-display.exp: After -var-assign, do -var-update
	before -var-evaluate-expression.


Index: varobj.h
===================================================================
RCS file: /cvs/src/src/gdb/varobj.h,v
retrieving revision 1.6
diff -c -p -r1.6 varobj.h
*** varobj.h	17 Dec 2005 22:34:03 -0000	1.6
--- varobj.h	8 Jan 2007 05:37:44 -0000
*************** extern int varobj_get_attributes (struct
*** 93,99 ****
  
  extern char *varobj_get_value (struct varobj *var);
  
! extern int varobj_set_value (struct varobj *var, char *expression);
  
  extern int varobj_list (struct varobj ***rootlist);
  
--- 93,99 ----
  
  extern char *varobj_get_value (struct varobj *var);
  
! extern char* varobj_set_value (struct varobj *var, char *expression);
  
  extern int varobj_list (struct varobj ***rootlist);
  
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.76
diff -c -p -r1.76 varobj.c
*** varobj.c	5 Jan 2007 21:58:48 -0000	1.76
--- varobj.c	8 Jan 2007 05:37:47 -0000
*************** struct varobj
*** 130,138 ****
    /* The format of the output for this object */
    enum varobj_display_formats format;
  
-   /* Was this variable updated via a varobj_set_value operation */
-   int updated;
- 
    /* Last print value.  */
    char *print_value;
  };
--- 130,135 ----
*************** varobj_get_value (struct varobj *var)
*** 797,808 ****
     value of the given expression */
  /* Note: Invokes functions that can call error() */
  
! int
  varobj_set_value (struct varobj *var, char *expression)
  {
    struct value *val;
!   int offset = 0;
!   int error = 0;
  
    /* The argument "expression" contains the variable's new value.
       We need to first construct a legal expression for this -- ugh! */
--- 794,804 ----
     value of the given expression */
  /* Note: Invokes functions that can call error() */
  
! char*
  varobj_set_value (struct varobj *var, char *expression)
  {
    struct value *val;
!   char* print_value;
  
    /* The argument "expression" contains the variable's new value.
       We need to first construct a legal expression for this -- ugh! */
*************** varobj_set_value (struct varobj *var, ch
*** 822,864 ****
  	{
  	  /* We cannot proceed without a valid expression. */
  	  xfree (exp);
! 	  return 0;
  	}
  
-       /* All types that are editable must also be changeable.  */
-       gdb_assert (varobj_value_is_changeable_p (var));
- 
-       /* The value of a changeable variable object must not be lazy.  */
-       gdb_assert (!value_lazy (var->value));
- 
-       /* Need to coerce the input.  We want to check if the
- 	 value of the variable object will be different
- 	 after assignment, and the first thing value_assign
- 	 does is coerce the input.
- 	 For example, if we are assigning an array to a pointer variable we
- 	 should compare the pointer with the the array's address, not with the
- 	 array's content.  */
-       value = coerce_array (value);
- 
-       /* The new value may be lazy.  gdb_value_assign, or 
- 	 rather value_contents, will take care of this.
- 	 If fetching of the new value will fail, gdb_value_assign
- 	 with catch the exception.  */
        if (!gdb_value_assign (var->value, value, &val))
  	return 0;
!      
!       /* If the value has changed, record it, so that next -var-update can
! 	 report this change.  If a variable had a value of '1', we've set it
! 	 to '333' and then set again to '1', when -var-update will report this
! 	 variable as changed -- because the first assignment has set the
! 	 'updated' flag.  There's no need to optimize that, because return value
! 	 of -var-update should be considered an approximation.  */
!       var->updated = install_new_value (var, val, 0 /* Compare values. */);
        input_radix = saved_input_radix;
!       return 1;
      }
  
!   return 0;
  }
  
  /* Returns a malloc'ed list with all root variable objects */
--- 818,836 ----
  	{
  	  /* We cannot proceed without a valid expression. */
  	  xfree (exp);
! 	  return NULL;
  	}
  
        if (!gdb_value_assign (var->value, value, &val))
  	return 0;
! 
!       print_value = value_get_print_value (val, var->format);
        input_radix = saved_input_radix;
! 
!       return print_value;
      }
  
!   return NULL;
  }
  
  /* Returns a malloc'ed list with all root variable objects */
*************** install_new_value (struct varobj *var, s
*** 963,1001 ****
      var->print_value = value_get_print_value (value, var->format);
    else if (changeable)
      {
!       /* If the value of the varobj was changed by -var-set-value, then the 
! 	 value in the varobj and in the target is the same.  However, that value
! 	 is different from the value that the varobj had after the previous
! 	 -var-update. So need to the varobj as changed.  */	 
!       if (var->updated)
  	changed = 1;
!       else 
  	{
! 	  /* Try to compare the values.  That requires that both
! 	     values are non-lazy.  */
! 	  
! 	  /* Quick comparison of NULL values.  */
! 	  if (var->value == NULL && value == NULL)
! 	    /* Equal. */
! 	    ;
! 	  else if (var->value == NULL || value == NULL)
! 	    changed = 1;
! 	  else
! 	    {
! 	      char *print_value;
! 	      gdb_assert (!value_lazy (var->value));
! 	      gdb_assert (!value_lazy (value));
! 	      print_value = value_get_print_value (value, var->format);
  
! 	      if (strcmp (var->print_value, print_value) != 0)
! 		{
! 		  xfree (var->print_value);
! 		  var->print_value = print_value;
! 		  changed = 1;
! 		}
! 	      else
! 		xfree (print_value);
  	    }
  	}
      }
  
--- 935,963 ----
      var->print_value = value_get_print_value (value, var->format);
    else if (changeable)
      {
!       /* Try to compare the values.  That requires that both
! 	 values are non-lazy.  */
! 	  
!       /* Quick comparison of NULL values.  */
!       if (var->value == NULL && value == NULL)
! 	/* Equal. */
! 	;
!       else if (value == NULL)
  	changed = 1;
!       else
  	{
! 	  char *print_value;
! 	  gdb_assert (!value_lazy (value));
! 	  print_value = value_get_print_value (value, var->format);
  
! 	  if (strcmp (var->print_value, print_value) != 0)
! 	    {
! 	      xfree (var->print_value);
! 	      var->print_value = print_value;
! 	      changed = 1;
  	    }
+ 	  else
+ 	    xfree (print_value);
  	}
      }
  
*************** install_new_value (struct varobj *var, s
*** 1003,1009 ****
    if (var->value != NULL)
      value_free (var->value);
    var->value = value;
-   var->updated = 0;
  
    gdb_assert (!var->value || value_type (var->value));
  
--- 965,970 ----
*************** varobj_update (struct varobj **varp, str
*** 1116,1122 ****
  	    {
  	      /* Note that it's changed */
  	      VEC_safe_push (varobj_p, result, v);
- 	      v->updated = 0;
  	    }
  	}
      }
--- 1077,1082 ----
*************** new_variable (void)
*** 1390,1396 ****
    var->children = NULL;
    var->format = 0;
    var->root = NULL;
-   var->updated = 0;
    var->print_value = NULL;
  
    return var;
--- 1350,1355 ----
Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.27
diff -c -p -r1.27 mi-cmd-var.c
*** mi/mi-cmd-var.c	8 Dec 2006 04:09:53 -0000	1.27
--- mi/mi-cmd-var.c	8 Jan 2007 05:37:49 -0000
*************** enum mi_cmd_result
*** 447,453 ****
  mi_cmd_var_assign (char *command, char **argv, int argc)
  {
    struct varobj *var;
!   char *expression;
  
    if (argc != 2)
      error (_("mi_cmd_var_assign: Usage: NAME EXPRESSION."));
--- 447,453 ----
  mi_cmd_var_assign (char *command, char **argv, int argc)
  {
    struct varobj *var;
!   char *expression, *print_value;
  
    if (argc != 2)
      error (_("mi_cmd_var_assign: Usage: NAME EXPRESSION."));
*************** mi_cmd_var_assign (char *command, char *
*** 463,472 ****
  
    expression = xstrdup (argv[1]);
  
!   if (!varobj_set_value (var, expression))
      error (_("mi_cmd_var_assign: Could not assign expression to varible object"));
  
!   ui_out_field_string (uiout, "value", varobj_get_value (var));
    return MI_CMD_DONE;
  }
  
--- 463,474 ----
  
    expression = xstrdup (argv[1]);
  
!   print_value = varobj_set_value (var, expression);
! 
!   if (!print_value)
      error (_("mi_cmd_var_assign: Could not assign expression to varible object"));
  
!   ui_out_field_string (uiout, "value", print_value);
    return MI_CMD_DONE;
  }
  
Index: testsuite/gdb.mi/mi-var-cmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-cmd.exp,v
retrieving revision 1.22
diff -c -p -r1.22 mi-var-cmd.exp
*** testsuite/gdb.mi/mi-var-cmd.exp	4 Jan 2007 21:59:10 -0000	1.22
--- testsuite/gdb.mi/mi-var-cmd.exp	8 Jan 2007 05:37:50 -0000
*************** mi_execute_to "exec-step 8" "end-steppin
*** 261,267 ****
  # Note: this test also checks that lpsimple->integer and lsimple.integer have
  #       changed (they are the same)
  mi_gdb_test "-var-update *" \
! 	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
  	"update all vars: func and lpsimple changed"
  
  
--- 261,267 ----
  # Note: this test also checks that lpsimple->integer and lsimple.integer have
  #       changed (they are the same)
  mi_gdb_test "-var-update *" \
! 	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
  	"update all vars: func and lpsimple changed"
  
  
*************** mi_gdb_test "-var-assign linteger 3333" 
*** 279,285 ****
  	"assign to linteger"
  
  mi_gdb_test "-var-update *" \
! 	"\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
  	"update all vars: linteger changed after assign"
  
  mi_gdb_test "-var-assign linteger 3333" \
--- 279,285 ----
  	"assign to linteger"
  
  mi_gdb_test "-var-update *" \
! 	"\\^done,changelist=\\\[\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
  	"update all vars: linteger changed after assign"
  
  mi_gdb_test "-var-assign linteger 3333" \
*************** mi_gdb_test "-var-assign lcharacter 'z'"
*** 324,329 ****
--- 324,332 ----
  	"\\^done,value=\"122 'z'\"" \
  	"assign to lcharacter"
  
+ mi_gdb_test "-var-update *" \
+ 	"\\^done,changelist=.*" \
+ 	"var update"
  mi_gdb_test "-var-evaluate-expression lcharacter" \
  	"\\^done,value=\"122 'z'\"" \
  	"eval lcharacter"
*************** mi_gdb_test "-var-evaluate-expression lc
*** 331,336 ****
--- 334,342 ----
  mi_gdb_test "-var-assign llong 1313L" \
  	"\\^done,value=\"1313\"" \
  	"assign to llong"
+ mi_gdb_test "-var-update *" \
+ 	"\\^done,changelist=.*" \
+ 	"var update"
  mi_gdb_test "-var-evaluate-expression llong" \
  	"\\^done,value=\"1313\"" \
  	"eval llong"
*************** mi_gdb_test "-var-assign lplong &llong" 
*** 351,356 ****
--- 357,365 ----
  mi_gdb_test "-var-assign lfloat 3.4567" \
  	"\\^done,value=\"3.45.*\"" \
  	"assign to lfloat"
+ mi_gdb_test "-var-update *" \
+ 	"\\^done,changelist=.*" \
+ 	"var update"
  mi_gdb_test "-var-evaluate-expression lfloat" \
  	"\\^done,value=\"3.45.*\"" \
  	"eval lfloat"
Index: testsuite/gdb.mi/mi-var-display.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-display.exp,v
retrieving revision 1.15
diff -c -p -r1.15 mi-var-display.exp
*** testsuite/gdb.mi/mi-var-display.exp	10 Aug 2006 05:27:21 -0000	1.15
--- testsuite/gdb.mi/mi-var-display.exp	8 Jan 2007 05:37:51 -0000
*************** mi_gdb_test "-var-assign bar 3" \
*** 108,114 ****
  mi_gdb_test "-var-set-format bar decimal" \
  	"\\^done,format=\"decimal\"" \
  	"set format variable bar"
! 
  mi_gdb_test "-var-evaluate-expression bar" \
  	"\\^done,value=\"3\"" \
  	"eval variable bar with new value"
--- 108,116 ----
  mi_gdb_test "-var-set-format bar decimal" \
  	"\\^done,format=\"decimal\"" \
  	"set format variable bar"
! mi_gdb_test "-var-update *" \
! 	"\\^done,changelist=.*" \
! 	"var update"
  mi_gdb_test "-var-evaluate-expression bar" \
  	"\\^done,value=\"3\"" \
  	"eval variable bar with new value"
*************** mi_gdb_test "-var-set-format foo decimal
*** 169,174 ****
--- 171,179 ----
  
  # Test: c_variable-6.18
  # Desc: check new value of foo
+ mi_gdb_test "-var-update *" \
+ 	"\\^done,changelist=.*" \
+ 	"var update"
  mi_gdb_test "-var-evaluate-expression foo" \
  	"\\^done,value=\"3\"" \
  	"eval variable foo"

Re: MI testsuite failures

[Vladimir Prus]

Nick Roberts wrote:

> 
> I notice now that my last change introduced several testsuite failures:

FWIW, it would be best if you run MI testsuite before *sending* a patch ;-)

> 
>   FAIL: gdb.mi/mi2-var-cmd.exp: update all vars: func and lpsimple changed
>   FAIL: gdb.mi/mi2-var-cmd.exp: update all vars: linteger changed after
>   assign
> 
> These are because GDB notices that string contents have been changed and
> just need an extra output field, which I guess is a good thing.
> 
> 
>   FAIL: gdb.mi/mi-var-cmd.exp: assign same value to func (update)
> 
> This is more subtle and is caused by having two variable objects for
> one variable, var1 and var2 say.  Then if they have a common value 10
> say and you do:
> 
> -var-assign var1 11
> -var-assign var2 12
> 
> var->updated is set to 1 for each and they are both reported as changed
> with -var-update.
> 
> However doing -var-update again gives var1 in the chagelist because the
> real value is 12 but var->print_value is "11".

That's because -var-assign should update var->print_value.

> I think this is a bug in -var-assign; it should set the variable value
> but not interfere with the variable object.  Note that generally if you
> have two variable objects for one value and set the value of one with
> -var-assign, one will be reported as changed because var->updated is 1,
> and the other because the value has changed.  I think the value held
> by the variable object shouldn't change until -var-update is issued,
> just as is the case if the real value changes during execution.  This is
> a patch to do that.

I don't think that if you assign a value to varobj and then 
-var-evaluate-expression returns something else than the value you've assigned,
it would be rather confusing interface.

> --- varobj.h    7 Jan 2007 23:27:50 -0000
> *************** extern int varobj_get_attributes (struct
> *** 93,99 ****
>   
>   extern char *varobj_get_value (struct varobj *var);
>   
> ! extern int varobj_set_value (struct varobj *var, char *expression);
>   
>   extern int varobj_list (struct varobj ***rootlist);
>   
> --- 93,99 ----
>   
>   extern char *varobj_get_value (struct varobj *var);
>   
> ! extern char* varobj_set_value (struct varobj *var, char *expression);
>   
>   extern int varobj_list (struct varobj ***rootlist);
>   
> Index: varobj.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/varobj.c,v
> retrieving revision 1.76
> diff -c -p -r1.76 varobj.c
> *** varobj.c    5 Jan 2007 21:58:48 -0000       1.76
> --- varobj.c    7 Jan 2007 23:27:54 -0000
> *************** varobj_get_value (struct varobj *var)
> *** 797,808 ****
>      value of the given expression */
>   /* Note: Invokes functions that can call error() */
>   
> ! int
>   varobj_set_value (struct varobj *var, char *expression)
>   {
>     struct value *val;
> !   int offset = 0;
> !   int error = 0;
>   
>     /* The argument "expression" contains the variable's new value.
>        We need to first construct a legal expression for this -- ugh! */
> --- 797,807 ----
>      value of the given expression */
>   /* Note: Invokes functions that can call error() */
>   
> ! char*
>   varobj_set_value (struct varobj *var, char *expression)
>   {
>     struct value *val;
> !   char* print_value;
>   
>     /* The argument "expression" contains the variable's new value.
>        We need to first construct a legal expression for this -- ugh! */
> *************** varobj_set_value (struct varobj *var, ch
> *** 822,864 ****
>         {
>           /* We cannot proceed without a valid expression. */
>           xfree (exp);
> !         return 0;
>         }
>   
> -       /* All types that are editable must also be changeable.  */
> -       gdb_assert (varobj_value_is_changeable_p (var));
> - 
> -       /* The value of a changeable variable object must not be lazy.  */
> -       gdb_assert (!value_lazy (var->value));

You're removing a bunch of asserts again, and I see no reason why.

> - 
> -       /* Need to coerce the input.  We want to check if the
> -        value of the variable object will be different
> -        after assignment, and the first thing value_assign
> -        does is coerce the input.
> -        For example, if we are assigning an array to a pointer variable we
> -        should compare the pointer with the the array's address, not with the
> -        array's content.  */
> -       value = coerce_array (value);

What's the reason for removing this block?

> - 
> -       /* The new value may be lazy.  gdb_value_assign, or 
> -        rather value_contents, will take care of this.
> -        If fetching of the new value will fail, gdb_value_assign
> -        with catch the exception.  */
>         if (!gdb_value_assign (var->value, value, &val))
>         return 0;
> !      
> !       /* If the value has changed, record it, so that next -var-update can
> !        report this change.  If a variable had a value of '1', we've set it
> !        to '333' and then set again to '1', when -var-update will report this
> !        variable as changed -- because the first assignment has set the
> !        'updated' flag.  There's no need to optimize that, because return value
> !        of -var-update should be considered an approximation.  */
> !       var->updated = install_new_value (var, val, 0 /* Compare values. */);
>         input_radix = saved_input_radix;
> !       return 1;
>       }
>   
> !   return 0;
>   }
>   
>   /* Returns a malloc'ed list with all root variable objects */
> --- 821,839 ----
>         {
>           /* We cannot proceed without a valid expression. */
>           xfree (exp);
> !         return NULL;
>         }
>   
>         if (!gdb_value_assign (var->value, value, &val))
>         return 0;
> ! 
> !       print_value = value_get_print_value (val, var->format);
>         input_radix = saved_input_radix;
> ! 
> !       return print_value;

As I say above, I'm not comfortable with -var-assign not changing
the value of varobj itself.

- Volodya

Re: MI testsuite failures

[Nick Roberts]

 > >   FAIL: gdb.mi/mi-var-cmd.exp: assign same value to func (update)
 > > 
 > > This is more subtle and is caused by having two variable objects for
 > > one variable, var1 and var2 say.  Then if they have a common value 10
 > > say and you do:
 > > 
 > > -var-assign var1 11
 > > -var-assign var2 12
 > > 
 > > var->updated is set to 1 for each and they are both reported as changed
 > > with -var-update.
 > > 
 > > However doing -var-update again gives var1 in the chagelist because the
 > > real value is 12 but var->print_value is "11".
 > 
 > That's because -var-assign should update var->print_value.

"-var-assign var2 12" will update var->print_value for var2 but not var1.  If
you can see a way to do update both please show me.

 > > I think this is a bug in -var-assign; it should set the variable value
 > > but not interfere with the variable object.  Note that generally if you
 > > have two variable objects for one value and set the value of one with
 > > -var-assign, one will be reported as changed because var->updated is 1,
 > > and the other because the value has changed.  I think the value held
 > > by the variable object shouldn't change until -var-update is issued,
 > > just as is the case if the real value changes during execution.  This is
 > > a patch to do that.
 > 
 > I don't think that if you assign a value to varobj and then
 > -var-evaluate-expression returns something else than the value you've
 > assigned, it would be rather confusing interface.

I recall that when I submitted my first patches (start of 2003?) Daniel J was
confused by the fact that -var-evaluate-expression didn't always return the
current variable value, and Jim Ingham explained that variable objects were
designed to work like that.  It only appears confusing because you're used to
the current behaviour.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI testsuite failures

[Vladimir Prus]

On Monday 08 January 2007 11:15, Nick Roberts wrote:
>  > >   FAIL: gdb.mi/mi-var-cmd.exp: assign same value to func (update)
>  > > 
>  > > This is more subtle and is caused by having two variable objects for
>  > > one variable, var1 and var2 say.  Then if they have a common value 10
>  > > say and you do:
>  > > 
>  > > -var-assign var1 11
>  > > -var-assign var2 12
>  > > 
>  > > var->updated is set to 1 for each and they are both reported as changed
>  > > with -var-update.
>  > > 
>  > > However doing -var-update again gives var1 in the chagelist because the
>  > > real value is 12 but var->print_value is "11".
>  > 
>  > That's because -var-assign should update var->print_value.
> 
> "-var-assign var2 12" will update var->print_value for var2 but not var1.  

Ah, indeed, -var-assign var2 will change var->print_value.

> If 
> you can see a way to do update both please show me.

Hmm, I don't understand why this test worked before and does not work
now. After all, we never detected the case where two varobjs
refer to the same memory location. Why it passed before and why it fails now?

We're talking about this failure:

	FAIL: gdb.mi/mi-var-cmd.exp: assign same value to func (update)

right? The assignement is to the 'func' varobj. What is the other varobj
that's involved here?

>  > > I think this is a bug in -var-assign; it should set the variable value
>  > > but not interfere with the variable object.  Note that generally if you
>  > > have two variable objects for one value and set the value of one with
>  > > -var-assign, one will be reported as changed because var->updated is 1,
>  > > and the other because the value has changed.  I think the value held
>  > > by the variable object shouldn't change until -var-update is issued,
>  > > just as is the case if the real value changes during execution.  This is
>  > > a patch to do that.
>  > 
>  > I don't think that if you assign a value to varobj and then
>  > -var-evaluate-expression returns something else than the value you've
>  > assigned, it would be rather confusing interface.
> 
> I recall that when I submitted my first patches (start of 2003?) Daniel J was
> confused by the fact that -var-evaluate-expression didn't always return the
> current variable value, and Jim Ingham explained that variable objects were
> designed to work like that.  It only appears confusing because you're used to
> the current behaviour.

The fact that -var-evaluate-expression is not necessary the value in memory
is OK. The fact that -var-assign does not change the value of varobj is independent
from that, and still confusing.

- Volodya

Re: MI testsuite failures [PATCH]

[Daniel Jacobowitz]

On Mon, Jan 08, 2007 at 06:53:11PM +1300, Nick Roberts wrote:
> Here's a patch with fixes for the testsuite.  I've made a further change to
> varobj.c to fix a failure in mi-var-child.exp.

Was this:

FAIL: gdb.mi/mi-var-child.exp: update all vars int_ptr_ptr and children changed
FAIL: gdb.mi/mi-var-child.exp: update all vars struct_declarations.long_array.0 changed

?

The patch below fixes those, and this one too:

FAIL: gdb.mi/mi-var-cmd.exp: assign same value to func (update)

I can't quite tell what your patch does, but I do see why these
failures happen.  Because the var->updated and the var->value
previously NULL cases were not updating print_value, you sometimes had
to -var-update twice to get a value to leave the list.

This is the same reason you had to add spurious -var-update's to the
testsuite.  We should only need this testsuite change:

> 	* gdb.mi/mi-var-cmd.exp: Add fields to changelists for string
> 	contents changes.

The first time we do that it's a good thing, i.e. we want lpcharacter
to be listed now.  The second time it's kind of dodgy: lpcharacter
is not a NULL terminated string and it just so happens that linteger
is right after lcharacter in memory, so -var-assign'ing to linteger
"changes" the string pointer to by lpcharacter.

-- 
Daniel Jacobowitz
CodeSourcery

2007-01-08  Daniel Jacobowitz  <dan@codesourcery.com>

	* varobj.c (install_new_value): Always update print_value.
	(value_get_print_value): Immediately return NULL for missing
	values.

2007-01-08  Nick Roberts  <nickrob@snap.net.nz>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.mi/mi-var-cmd.exp: Expect lpcharacter to update when
	lcharacter or linteger change.  Correct duplicated test name.
	* gdb.mi/mi2-var-cmd.exp: Likewise.

Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.76
diff -u -p -r1.76 varobj.c
--- varobj.c	5 Jan 2007 21:58:48 -0000	1.76
+++ varobj.c	8 Jan 2007 17:08:01 -0000
@@ -966,9 +966,13 @@ install_new_value (struct varobj *var, s
       /* If the value of the varobj was changed by -var-set-value, then the 
 	 value in the varobj and in the target is the same.  However, that value
 	 is different from the value that the varobj had after the previous
-	 -var-update. So need to the varobj as changed.  */	 
+	 -var-update. So need to the varobj as changed.  */
       if (var->updated)
-	changed = 1;
+	{
+	  xfree (var->print_value);
+	  var->print_value = value_get_print_value (value, var->format);
+	  changed = 1;
+	}
       else 
 	{
 	  /* Try to compare the values.  That requires that both
@@ -979,7 +983,11 @@ install_new_value (struct varobj *var, s
 	    /* Equal. */
 	    ;
 	  else if (var->value == NULL || value == NULL)
-	    changed = 1;
+	    {
+	      xfree (var->print_value);
+	      var->print_value = value_get_print_value (value, var->format);
+	      changed = 1;
+	    }
 	  else
 	    {
 	      char *print_value;
@@ -987,6 +995,7 @@ install_new_value (struct varobj *var, s
 	      gdb_assert (!value_lazy (value));
 	      print_value = value_get_print_value (value, var->format);
 
+	      gdb_assert (var->print_value != NULL && print_value != NULL);
 	      if (strcmp (var->print_value, print_value) != 0)
 		{
 		  xfree (var->print_value);
@@ -1687,12 +1696,19 @@ static char *
 value_get_print_value (struct value *value, enum varobj_display_formats format)
 {
   long dummy;
-  struct ui_file *stb = mem_fileopen ();
-  struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
+  struct ui_file *stb;
+  struct cleanup *old_chain;
   char *thevalue;
-	    
+
+  if (value == NULL)
+    return NULL;
+
+  stb = mem_fileopen ();
+  old_chain = make_cleanup_ui_file_delete (stb);
+
   common_val_print (value, stb, format_code[(int) format], 1, 0, 0);
   thevalue = ui_file_xstrdup (stb, &dummy);
+
   do_cleanups (old_chain);
   return thevalue;
 }
Index: testsuite/gdb.mi/mi-var-cmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-cmd.exp,v
retrieving revision 1.22
diff -u -p -r1.22 mi-var-cmd.exp
--- testsuite/gdb.mi/mi-var-cmd.exp	4 Jan 2007 21:59:10 -0000	1.22
+++ testsuite/gdb.mi/mi-var-cmd.exp	8 Jan 2007 17:08:01 -0000
@@ -261,8 +261,8 @@ mi_execute_to "exec-step 8" "end-steppin
 # Note: this test also checks that lpsimple->integer and lsimple.integer have
 #       changed (they are the same)
 mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
-	"update all vars: func and lpsimple changed"
+	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"update all vars: lsimple and others changed"
 
 
 ### 
@@ -278,8 +278,13 @@ mi_gdb_test "-var-assign linteger 3333" 
 	"\\^done,value=\"3333\"" \
 	"assign to linteger"
 
+# Allow lpcharacter to update, optionally.  Because it points to a
+# char variable instead of a zero-terminated string, if linteger is
+# directly after it in memory the printed characters may appear to
+# change.
+set lpchar_update "\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},"
 mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"\\^done,changelist=\\\[($lpchar_update)?\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
 	"update all vars: linteger changed after assign"
 
 mi_gdb_test "-var-assign linteger 3333" \
Index: testsuite/gdb.mi/mi2-var-cmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-var-cmd.exp,v
retrieving revision 1.6
diff -u -p -r1.6 mi2-var-cmd.exp
--- testsuite/gdb.mi/mi2-var-cmd.exp	4 Jan 2007 18:58:03 -0000	1.6
+++ testsuite/gdb.mi/mi2-var-cmd.exp	8 Jan 2007 17:08:01 -0000
@@ -261,8 +261,8 @@ mi_execute_to "exec-step 8" "end-steppin
 # Note: this test also checks that lpsimple->integer and lsimple.integer have
 #       changed (they are the same)
 mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
-	"update all vars: func and lpsimple changed"
+	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"update all vars: lsimple and others changed"
 
 
 ### 
@@ -278,8 +278,13 @@ mi_gdb_test "-var-assign linteger 3333" 
 	"\\^done,value=\"3333\"" \
 	"assign to linteger"
 
+# Allow lpcharacter to update, optionally.  Because it points to a
+# char variable instead of a zero-terminated string, if linteger is
+# directly after it in memory the printed characters may appear to
+# change.
+set lpchar_update "\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},"
 mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"\\^done,changelist=\\\[($lpchar_update)?\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
 	"update all vars: linteger changed after assign"
 
 mi_gdb_test "-var-assign linteger 3333" \

Re: MI testsuite failures [PATCH]

[Nick Roberts]

 > > Here's a patch with fixes for the testsuite.  I've made a further change to
 > > varobj.c to fix a failure in mi-var-child.exp.
 > 
 > Was this:
 > 
 > FAIL: gdb.mi/mi-var-child.exp: update all vars int_ptr_ptr and children changed
 > FAIL: gdb.mi/mi-var-child.exp: update all vars struct_declarations.long_array.0 changed
 > 
 > ?

Certainly the second, which was to do with value == NULL and
var->value != NULL.

 > The patch below fixes those, and this one too:
 > 
 > FAIL: gdb.mi/mi-var-cmd.exp: assign same value to func (update)

Yes, this looks right and is much simpler, but I just couldn't see how to do
it.

 > I can't quite tell what your patch does, but I do see why these
 > failures happen.  Because the var->updated and the var->value
 > previously NULL cases were not updating print_value, you sometimes had
 > to -var-update twice to get a value to leave the list.

My patch defers the updating of the variable object until -var-update is
issued.

 > This is the same reason you had to add spurious -var-update's to the
 > testsuite.  

I don't know that I'd call them spurious.  The command -var-assign seems a bit
anomalous to me.  It could just set the value like "set var i=10".  I think the
updating of variable objects should be left to -var-update, but this change
might be too radical at this stage.

 > The first time we do that it's a good thing, i.e. we want lpcharacter
 > to be listed now.  The second time it's kind of dodgy: lpcharacter
 > is not a NULL terminated string and it just so happens that linteger
 > is right after lcharacter in memory, so -var-assign'ing to linteger
 > "changes" the string pointer to by lpcharacter.

I think that's the best we can do; the `string' will be displayed differently
in the watch window.

 > 2007-01-08  Daniel Jacobowitz  <dan@codesourcery.com>
 > 
 > 	* varobj.c (install_new_value): Always update print_value.
 > 	(value_get_print_value): Immediately return NULL for missing
 > 	values.
 > 
 > 2007-01-08  Nick Roberts  <nickrob@snap.net.nz>
 > 	    Daniel Jacobowitz  <dan@codesourcery.com>
 > 
 > 	* gdb.mi/mi-var-cmd.exp: Expect lpcharacter to update when
 > 	lcharacter or linteger change.  Correct duplicated test name.
 > 	* gdb.mi/mi2-var-cmd.exp: Likewise.

Yes, I'd be happy to see these changes installed.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI testsuite failures [PATCH]

[Daniel Jacobowitz]

On Tue, Jan 09, 2007 at 11:33:21AM +1300, Nick Roberts wrote:
>  > This is the same reason you had to add spurious -var-update's to the
>  > testsuite.  
> 
> I don't know that I'd call them spurious.  The command -var-assign seems a bit
> anomalous to me.  It could just set the value like "set var i=10".  I think the
> updating of variable objects should be left to -var-update, but this change
> might be too radical at this stage.

I think it's right the way it is; if you want that behavior, you could
use -data-evaluate-expression to assign instead of -var-assign (well,
you'd need resolution on -var-info-path-expression first).  -var-assign
is documented to assign to the variable object, not just the variable.

Anyway, easily adjusted later, if we want.

> I think that's the best we can do; the `string' will be displayed differently
> in the watch window.

I agree.

>  > 2007-01-08  Daniel Jacobowitz  <dan@codesourcery.com>
>  > 
>  > 	* varobj.c (install_new_value): Always update print_value.
>  > 	(value_get_print_value): Immediately return NULL for missing
>  > 	values.
>  > 
>  > 2007-01-08  Nick Roberts  <nickrob@snap.net.nz>
>  > 	    Daniel Jacobowitz  <dan@codesourcery.com>
>  > 
>  > 	* gdb.mi/mi-var-cmd.exp: Expect lpcharacter to update when
>  > 	lcharacter or linteger change.  Correct duplicated test name.
>  > 	* gdb.mi/mi2-var-cmd.exp: Likewise.
> 
> Yes, I'd be happy to see these changes installed.

Thanks for looking at it.  I'll check them in shortly.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI testsuite failures [PATCH]

[Nick Roberts]

 > > Yes, I'd be happy to see these changes installed.
 > 
 > Thanks for looking at it.  I'll check them in shortly.

Great.  Now how about getting rid of the mi2-*.exp tests?  This will halve the
work to add a field to -var-update.  The current level *is* mi2 and when we
move to mi3 we can call the mi-*.exp tests mi2-*.exp and create new tests for
mi3.  I think Andrew possibly jumped the gun by `adding' mi3.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI testsuite failures [PATCH]

[Daniel Jacobowitz]

On Tue, Jan 09, 2007 at 12:27:38PM +1300, Nick Roberts wrote:
>  > > Yes, I'd be happy to see these changes installed.
>  > 
>  > Thanks for looking at it.  I'll check them in shortly.
> 
> Great.  Now how about getting rid of the mi2-*.exp tests?  This will halve the
> work to add a field to -var-update.  The current level *is* mi2 and when we
> move to mi3 we can call the mi-*.exp tests mi2-*.exp and create new tests for
> mi3.  I think Andrew possibly jumped the gun by `adding' mi3.

Haven't we discussed this on the list before, repeatedly?  So far we've
left them in place.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI testsuite failures [PATCH]

[Nick Roberts]

 > > Great.  Now how about getting rid of the mi2-*.exp tests?  This will halve
 > > the work to add a field to -var-update.  The current level *is* mi2 and
 > > when we move to mi3 we can call the mi-*.exp tests mi2-*.exp and create
 > > new tests for mi3.  I think Andrew possibly jumped the gun by `adding'
 > > mi3.
 > 
 > Haven't we discussed this on the list before, repeatedly?

I can only recall discussing it once before, in June 2005, a year and a half
ago.

 >                                                            So far we've
 > left them in place.

That's why I've raised it again.  There are still no features unique to mi3 and
I thought perhaps that enough time has elapsed to reconsider it.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI testsuite failures [PATCH]

[Daniel Jacobowitz]

On Wed, Jan 10, 2007 at 11:18:12AM +1300, Nick Roberts wrote:
> That's why I've raised it again.  There are still no features unique to mi3 and
> I thought perhaps that enough time has elapsed to reconsider it.

*shrug*

I am still planning to add at least one incompatible change for mi3,
once I finally get around to it.  I'd just have to add all the files
back again.  So they're useful to me; if they're too much trouble,
I'll offer to update them myself.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI testsuite failures [PATCH]

[Nick Roberts]

 > > That's why I've raised it again.  There are still no features unique to
 > > mi3 and I thought perhaps that enough time has elapsed to reconsider it.
 > 
 > *shrug*
 > 
 > I am still planning to add at least one incompatible change for mi3,
 > once I finally get around to it.  I'd just have to add all the files
 > back again.  So they're useful to me; if they're too much trouble,
 > I'll offer to update them myself.

This doesn't seem a sensible way to do it as eventually we'll have massive
duplication (multiplication?).  Note that the manual claims that GDB supports
mi1 but there don't appear to be any tests for it.  I think:

1) mi-*exp should be tests that work for all mi`N' where N > 1, and could be
   run for each MI interpreter.

2) mi2-*exp tests that work for mi2 but fail for mi3, currently none.

3) mi3-*exp tests that work for mi3 but fail for mi2, currently none.

Also if mi3 becomes the new mi, we presumably should advise frontend
developers to specify mi2, otherwise existing frontends might get a nasty
surprise when the new features of mi3 appear.

Hmm, what is this incompatible change anyway?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI testsuite failures [PATCH]

[Daniel Jacobowitz]

On Wed, Jan 10, 2007 at 08:09:38PM +1300, Nick Roberts wrote:
> This doesn't seem a sensible way to do it as eventually we'll have massive
> duplication (multiplication?).  Note that the manual claims that GDB supports
> mi1 but there don't appear to be any tests for it.  I think:
> 
> 1) mi-*exp should be tests that work for all mi`N' where N > 1, and could be
>    run for each MI interpreter.
> 
> 2) mi2-*exp tests that work for mi2 but fail for mi3, currently none.
> 
> 3) mi3-*exp tests that work for mi3 but fail for mi2, currently none.

Running a single .exp file for multiple MI interpreters would be a bit
tricky, but we could probably do it.  You'd have to wrap most of them
in a function and call it twice, I guess.

I don't feel strongly about this if you want to change it.

> Also if mi3 becomes the new mi, we presumably should advise frontend
> developers to specify mi2, otherwise existing frontends might get a nasty
> surprise when the new features of mi3 appear.

I haven't thought about it much.  They'll appear at least one release
before we change -i=mi though.

> Hmm, what is this incompatible change anyway?

Quoting.  I posted an analysis of command line quoting issues to the
gdb list around the middle of last year; I intend to make every single
MI command handle quoting the way the manual says MI ought to, but this
will change the behavior of various commands (the directory and file
related ones, mainly, but -gdb-set will probably be affected too).

-- 
Daniel Jacobowitz
CodeSourcery