Re: Variable objects laziness

Vladimir Prus
2006-11-29T07:25:35Z

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

Nick Roberts wrote:

>
> With the new changes to varobj.c, -var-assign doesn't work for references.

This is embarrassing. However, it also validates my claim that we should a
single invariant-preserving function to assign new value. This crash
happens, for all appearances, because varobj_set_value directly sets new
value.

I've checked in the attached, that fixes the crash, and causes no
regressions.

I did not add no testcase for this. I've run into some grave
reference-related bug reported by KDevelop user (not related to my patch)
and is going to fix that, and then add a new test for C++ features.

> Also -var-update only reports a change when the address changes and not
> the
> value. Variable objects were broken before but in a different way
> (-var-assign worked but -var-update always reported that the reference
> value had changed).

Apparently, not reporting a change is worse

>
> I think a further call to coerce_array is needed

No, please no! Calls to coerce_array is exactly the reason for the other bug
I'm fixing. This function has a nice property of silently coercing_refs,
but that property is not documented, not obvious from function name and
therefore should be considered a bug.

Here, call to coerce_ref would work.

> and the output to
> -var-update should look like:
>
>
> -var-update --all-values var1
> ^done,changelist=[{name="var1",value="4",in_scope="true",type_chan
> ged="false"}]
>
> i.e the address shouldn't appear in the value field.

Attached (references.diff) is the patch that makes gdb sense the changes in
reference values, and eliminates the address from the output. Any opinions?

- Volodya

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ref_assign.diff --]
[-- Type: text/x-diff; name="ref_assign.diff", Size: 1988 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.7995
diff -u -p -r1.7995 ChangeLog
--- ChangeLog 28 Nov 2006 22:21:23 -0000 1.7995
+++ ChangeLog 29 Nov 2006 06:40:51 -0000
@@ -1,3 +1,9 @@
+2006-11-29 Vladimir Prus <vladimir@codesourcery.com>
+
+ * varobj.c (varobj_set_value): Don't compare the old
+ and the new value here. Don't assign new value here.
+ Instead, call install_new_value.
+
2006-11-28 Daniel Jacobowitz <dan@codesourcery.com>

* regformats/reg-mips64.dat: New file.
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.61
diff -u -p -r1.61 varobj.c
--- varobj.c 28 Nov 2006 17:23:09 -0000 1.61
+++ varobj.c 29 Nov 2006 06:40:52 -0000
@@ -841,18 +841,22 @@ varobj_set_value (struct varobj *var, ch
array's content. */
value = coerce_array (value);

- if (!value_contents_equal (var->value, value))
- var->updated = 1;
-
/* 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;
- value_free (var->value);
+
release_value (val);
- var->value = val;
+
+ /* 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;
}