Re: Variable objects laziness

Nick Roberts
2006-11-15T02:45:34Z

I'm having trouble to understand this patch. If it does more than one thing
perhaps you can break it into two patches.

* varobj.c (struct varobj): Clarify comment.
(my_value_equal): Remove.
(install_new_value): New.

New function presumably.

(type_of_child): Remove.
(varobj_create): Use install_new_value.
(varobj_set_value): Use value_contents_equal, not
my_value_equal.

Previously someone (Mark Kettenis?) has gone to a lot of trouble to replace
value_contents_equal with my_value_equal why do you think it's not needed?

(varobj_update): Use install_new_value.
(create_child): Likewise. Inline type_of_child here.
(value_of_child): Don't fetch the value.
(c_value_of_root): Likewise.
(c_value_of_variable): Likewise.

Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.60
diff -u -p -r1.60 varobj.c
--- varobj.c 3 May 2006 22:59:38 -0000 1.60
+++ varobj.c 14 Nov 2006 13:38:35 -0000
@@ -101,7 +101,9 @@ struct varobj
/* The type of this variable. This may NEVER be NULL. */
struct type *type;

- /* The value of this expression or subexpression. This may be NULL. */
+ /* The value of this expression or subexpression. This may be NULL.
+ Invariant: if type_changeable (this) is non-zero, the value is either
+ NULL, or not lazy. */

I don't understand the replacement comment

...

+/** Assign new value to a variable object. If INITIAL is non-zero,
+ this is first assignement after the variable object was just
+ created, or changed type. In that case, just assign the value
+ and return 0.
+ Otherwise, assign the value and if type_changeable returns non-zero,
+ find if the new value is different from the current value.
+ Return 1 if so, and 0 is the values are equal. */
+static int
+install_new_value (struct varobj *var, struct value *value, int initial)
+{

I don't understand this comment either. INITIAL can be type_changed
i.e not really initial. Can you give it more structure and not refer to
internals like type_changeable?

...

+ if (CPLUS_FAKE_CHILD (var))
+ changeable = 0;
+ else
+ changeable = type_changeable (var);

type_changeable returns 0 if CPLUS_FAKE_CHILD (var) is true anyway so do you
need this clause?

As a whole the patch seems to lack clarity (although that might partly be a
reflection on my abilities!)

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