Re: MI testsuite failures

[Vladimir Prus <ghost at cs dot msu dot su>]

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