This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Variable objects laziness
- From: Nick Roberts <nickrob at snap dot net dot nz>
- To: Vladimir Prus <vladimir at codesourcery dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Wed, 15 Nov 2006 15:41:25 +1300
- Subject: Re: Variable objects laziness
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