Re: RFC: how to handle mutable types in varobj?

[Joel Brobecker <brobecker at adacore dot com>]

> I'd like to discuss how mutable types should be handled in varobj.

It turns out to be a little simpler than I thought, assuming my
understanding of the GDB/MI varobj interface is right:

If a varobj mutated, whether a root varobj or not, it should be
listed in the change list as a "type_changed".  However, all its
children varobjs are simply destroyed without further ado, and
they will not be listed in the change list. This means that we
expect the front-end to understand that children varobjs of
a varobj that has mutated are invalid, and thus automatically
destroyed.

With the attached prototype, this is what I get.

  (1) First, creating the varobj and listing its children:

  | -var-create v * v
  | ^done,name="v",numchild="4",value="{...}",type="pck.variant",has_more="0"
  | (gdb)
  | -var-list-children 1 v
  | ^done,numchild="4",children=[child={name="v.disc",exp="disc",numchild="0",value="true",type="boolean"},[snip]],has_more="0"
  | (gdb)

  (2) Second, doing an update after some changes in the value, but
      no mutation (only field "a" in our variant record changed):

  | -var-update v
  | ^done,changelist=[{name="v.a",in_scope="true",type_changed="false",has_more="0"}]
  | (gdb)

  (3) Lastly, doing an update after the value mutated. This time,
      we see that varobj simply reports that the value has mutated
      (type_changed="true"), and just provides the number number of
      children:

  |  -var-update v
  |  ^done,changelist=[{name="v",in_scope="true",type_changed="true",new_type="pck.variant",new_num_children="3",has_more="0"}]
  |  (gdb)

The prototype cannot be applied as is, as it needs to be applied
on top of some other changes that are work-in-progress.  But it
should give a good idea of how I proceeded.

The good thing is that this is fairly localized. I'm also wondering
whether this might be done without a language-dependent hook (see
the first FIXME of ada_varobj_value_has_mutated's description).
Perhaps create_child_with_value could solve my practical problem?
I haven't gone that far yet. I also tried testing the case where
it's a child rather than the root that mutates, but for some reason
the child is not properly described (in a sense similar to what
c_describe_child does), and thus I miss the mutation. So that
feature is untested, but it's just because of a bug I will chase
ASAP.

The unfortunate part, though, is that this is done somewhat in
parallel of the work done to handle pretty-printers. I think it
kind of makes sense that pretty-printers should take precedence
over this treatment. So, I'm somewhat thinking that it's better
that way. I haven't had time to verify that precedence is respected,
however. I just wanted to send the patch for comment...

The careful reader will also notice that I'm having trouble with
figuring out some invariants regarding the `from' and `to' fields
with respect to the `num_children' field... More stuff to be figured
out later...

Thanks,
-- 
Joel

commit d75d746f839bf5a6e022cc8b7bfe1725345036e5
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Dec 29 11:39:06 2011 +0400

    Prototype of Ada value type-mutation.

diff --git a/gdb/varobj.c b/gdb/varobj.c
index 88f356c..b26e109 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1759,6 +1759,87 @@ varobj_set_visualizer (struct varobj *var, const char *visualizer)
 #endif
 }
 
+/* FIXME: Should be a language_specific attribute?  For now, it's just
+   used as a prototype.  It cannot be made language-independent, because
+   it needs to get the number of children and their names solely from
+   the new varobj *value*.  Perhaps the problem could be solved by
+   creating a temporary anonymous varobj for the varobj's new value?
+   But then, there is the question whether mutation is a concept that
+   remains the same for all languages.  It seems a shame to be going
+   through this for languages such as C, where there are clearly no
+   type mutations the way they occur in Ada.
+
+   This function should be able to assume that:
+     - The varobj has a value;
+     - The varobj's number of children is set (not < 0);
+     - NEW_VALUE is not NULL.
+   */
+
+static int
+ada_varobj_value_has_mutated (struct varobj *var, struct value *new_val)
+{
+  int i, from, to;
+
+  /* If the number of fields have changed, then for sure the type
+     has mutated.  */
+  if (ada_varobj_get_number_of_children (new_val) != var->num_children)
+    return 1;
+
+  /* If the number of fields have remained the same, then we need
+     to check the name of each field.  If they remain the same,
+     then chances are the type hasn't mutated.  This is technically
+     an incomplete test, as the child's type might have changed
+     despite the fact that the name remains the same.  But we'll
+     handle this situation by saying that the child has mutated,
+     not this value.
+
+     If only part (or none!) of the children have been fetched,
+     then only check the ones we fetched.  It does not matter
+     to the frontend whether a child that it has not fetched yet
+     has mutated or not. So just assume it hasn't.  */
+
+  restrict_range (var->children, &from, &to);
+  for (i = from; i < to; i++)
+    if (strcmp (ada_varobj_get_name_of_child (var->value, var->name, i),
+		VEC_index (varobj_p, var->children, i)->name) != 0)
+      return 1;
+
+  return 0;
+}
+
+/* If NEW_VALUE is the new value of the given varobj (var), return
+   non-zero if var has mutated.  In other words, if the type of
+   the new value is different from the type of the varobj's old
+   value.
+
+   NEW_VALUE may be NULL, if the varobj is now out of scope.  */
+
+static int
+varobj_value_has_mutated (struct varobj *var, struct value *new_value)
+{
+  /* If there was no previous value, it probably means that our varobj
+     was out of scope.  This is a scoping-change issue, handled
+     elsewhere, which makes the detection of mutations unnecessary.
+     Same if NEW_VALUE is NULL.  */
+  if (var->value == NULL || new_value == NULL)
+    return 0;
+
+  /* If we haven't previously computed the number of children in var,
+     it does not matter from the front-end's perspective whether
+     the type has mutated or not.  For all intents and purposes,
+     it has not mutated.
+
+     FIXME: Do we need to check var->from & var->to?  I cannot seem
+     to convince myself that this is independent of num_children.  */
+  if (var->num_children < 0)
+    return 0;
+
+  if (var->root->lang->language == vlang_ada)
+    return ada_varobj_value_has_mutated (var, new_value);
+  else
+    return 0;
+}
+
 /* Update the values for a variable and its children.  This is a
    two-pronged attack.  First, re-parse the value for the root's
    expression to see if it's changed.  Then go all the way
@@ -1853,9 +1934,24 @@ varobj_update (struct varobj **varp, int explicit)
       /* Update this variable, unless it's a root, which is already
 	 updated.  */
       if (!r.value_installed)
-	{	  
+	{
+	  int has_mutated;
+
 	  new = value_of_child (v->parent, v->index);
-	  if (install_new_value (v, new, 0 /* type not changed */))
+	  has_mutated = varobj_value_has_mutated (v, new);
+
+	  if (has_mutated)
+	    {
+	      /* The children are no longer valid; delete them now.
+	         Report the fact that its type changed as well.  */
+	      varobj_delete (v, NULL, 1 /* only_children */);
+	      v->num_children = -1;
+	      v->to = -1; /* FIXME: Is that necessary?  */
+	      v->from = -1;
+	      r.type_changed = 1;
+	    }
+
+	  if (install_new_value (v, new, r.type_changed))
 	    {
 	      r.changed = 1;
 	      v->updated = 0;
@@ -2543,7 +2639,23 @@ value_of_root (struct varobj **var_handle, int *type_changed)
       *type_changed = 0;
     }
 
-  return (*var->root->lang->value_of_root) (var_handle);
+  {
+    struct value *value;
+
+    value = (*var->root->lang->value_of_root) (var_handle);
+    if (varobj_value_has_mutated (var, value))
+      {
+	/* The type has mutated, so the children are no longer valid.
+	   Just delete them, and tell our caller that the type has
+	   changed.  */
+	varobj_delete (var, NULL, 1 /* only_children */);
+	var->num_children = -1;
+	var->to = -1; /* FIXME: Is that necessary?  */
+	var->from = -1;
+	*type_changed = 1;
+      }
+    return value;
+  }
 }
 
 /* What is the ``struct value *'' for the INDEX'th child of PARENT?  */