MI: frozen variable objects

[Vladimir Prus <vladimir at codesourcery dot com>]

This patch introduces so called "frozen" variable objects -- variable objects 
that are not implicitly updated by the "-var-update *" command or by 
"-var-update" on parent variable objects.

Those are needed in two cases:

    - GUI might want to capture a value of some variable in the program
    and don't update it automatically. For example, so that at a later point
    you can look at the current values, and compare them with previous values.
    Now, MI does not provide any mechanism to do it.

    - Some values might be "read-sensitive" -- it might be some memory-mapped
    hardware register and reading from it will extact data from FIFO, or
    acknowledge an interrupt, so this is not something that can be done
    implicitly.

Frozen variable object is a mechanism to handle those use cases. The most 
important design aspects are:

   - A variable object can be made frozen either by explicit request from the
    user, or automatically by gdb, if gdb detect that the value is
    read-sensitive. The present patch does not implement such auto-detection
    but I'm prototyped such functionality too, and it will be coming later.

   - If a variable object is frozen, then the value of that object, or any
     children of it, will be fetched from the memory (via value_fetch_lazy)
     only by explicit -var-update for that variable object or a child. No
     other operations, importantly -var-update * and -var-update of parents
     of frozen variable object, and -var-list-children, will fetch the value.

   - It is possible that a frozen variable has no value fetched. In that 
    case -var-evaluate-expression will return empty string -- it won't
    implicitly fetch the value.

The patch is not fully finished -- it does not include testcase changes, and 
it does not include docs. Both will take quite some time, so I'd like to pass 
the code patch itself for review first.

Thanks,
Volodya

	* varobj.h (varobj_set_frozen): New
	(varobj_get_frozen): New.
	(varobj_update): New parameter explicit.
	* varobj.c (struct varobj): New fields frozen
	and not_fetched.
	(varobj_set_frozen, varobj_get_frozen): New.
	(install_new_value): Don't fetch values for 
	frozen variable object, or children thereof.  Allow
	a frozen variable object to have non-fetched value.
	(varobj_update): Allow updating child variables.
	Don't traverse frozen children.
	(new_variable): Initialize the frozen field.
	(c_value_of_variable): Return NULL for frozen
	variable without any value yet.
	* mi/mi-cmd-var.c (varobj_update_one): New parameter
	'explicit'.
	(mi_cmd_var_create): Output the 'frozen' field,
	as soon as testsuite is adjusted to expect that field.
	(mi_cmd_var_set_frozen): New.
	(mi_cmd_var_list_children): : Output the 'frozen' field,
	as soon as testsuite is adjusted to expect that field.
	(mi_cmd_var_update): Pass the 'explicit' parameter to
	varobj_update_one.
	* mi/mi-cmds.c (mi_cmds): Register '-var-set-frozen'.
	* mi/mi-cmds.h (mi_cmd_var_set_frozen): Declare.

=== gdb/mi/mi-cmds.h
==================================================================
--- gdb/mi/mi-cmds.h	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/mi/mi-cmds.h	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -112,6 +112,7 @@
 extern mi_cmd_argv_ftype mi_cmd_var_info_type;
 extern mi_cmd_argv_ftype mi_cmd_var_list_children;
 extern mi_cmd_argv_ftype mi_cmd_var_set_format;
+extern mi_cmd_argv_ftype mi_cmd_var_set_frozen;
 extern mi_cmd_argv_ftype mi_cmd_var_show_attributes;
 extern mi_cmd_argv_ftype mi_cmd_var_show_format;
 extern mi_cmd_argv_ftype mi_cmd_var_update;
=== gdb/mi/mi-cmds.c
==================================================================
--- gdb/mi/mi-cmds.c	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/mi/mi-cmds.c	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -160,6 +160,7 @@
   { "var-info-type", { NULL, 0 }, 0, mi_cmd_var_info_type},
   { "var-list-children", { NULL, 0 }, 0, mi_cmd_var_list_children},
   { "var-set-format", { NULL, 0 }, 0, mi_cmd_var_set_format},
+  { "var-set-frozen", { NULL, 0 }, 0, mi_cmd_var_set_frozen},
   { "var-show-attributes", { NULL, 0 }, 0, mi_cmd_var_show_attributes},
   { "var-show-format", { NULL, 0 }, 0, mi_cmd_var_show_format},
   { "var-update", { NULL, 0 }, 0, mi_cmd_var_update},
=== gdb/mi/mi-cmd-var.c
==================================================================
--- gdb/mi/mi-cmd-var.c	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/mi/mi-cmd-var.c	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -37,7 +37,8 @@
 extern int varobjdebug;		/* defined in varobj.c */
 
 static int varobj_update_one (struct varobj *var,
-			      enum print_values print_values);
+			      enum print_values print_values,
+			      int explicit);
 
 /* VAROBJ operations */
 
@@ -108,6 +109,7 @@
       ui_out_field_string (uiout, "type", type);
       xfree (type);
     }
+  ui_out_field_int (uiout, "frozen", varobj_get_frozen (var)); 
 
   do_cleanups (old_cleanups);
   return MI_CMD_DONE;
@@ -215,6 +217,35 @@
 }
 
 enum mi_cmd_result
+mi_cmd_var_set_frozen (char *command, char **argv, int argc)
+{
+  struct varobj *var;
+  int frozen;
+
+  if (argc != 2)
+    error (_("mi_cmd_var_set_format: Usage: NAME FROZEN_FLAG."));
+
+  var = varobj_get_handle (argv[0]);
+  if (var == NULL)
+    error (_("Variable object not found"));
+
+  if (strcmp (argv[1], "0") == 0)
+    frozen = 0;
+  else if (strcmp (argv[1], "1") == 0)
+    frozen = 1;
+  else
+    error (_("Invalid flag value"));
+
+  varobj_set_frozen (var, frozen);
+
+  /* We don't automatically return the new value, or what varobjs got new
+     values during unfreezing.  If this information is required, client
+     should call -var-update explicitly.  */
+  return MI_CMD_DONE;
+}
+
+
+enum mi_cmd_result
 mi_cmd_var_show_format (char *command, char **argv, int argc)
 {
   enum varobj_display_formats format;
@@ -345,6 +376,7 @@
       /* C++ pseudo-variables (public, private, protected) do not have a type */
       if (type)
 	ui_out_field_string (uiout, "type", type);
+      ui_out_field_int (uiout, "frozen", varobj_get_frozen (*cc));
       do_cleanups (cleanup_child);
       cc++;
     }
@@ -503,7 +535,7 @@
       cr = rootlist;
       while (*cr != NULL)
 	{
-	  varobj_update_one (*cr, print_values);
+	  varobj_update_one (*cr, print_values, 0 /* implicit */);
 	  cr++;
 	}
       xfree (rootlist);
@@ -520,7 +552,7 @@
         cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
       else
         cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
-      varobj_update_one (var, print_values);
+      varobj_update_one (var, print_values, 1 /* explicit */);
       do_cleanups (cleanup);
     }
     return MI_CMD_DONE;
@@ -531,14 +563,15 @@
    scope), and 1 if it succeeds. */
 
 static int
-varobj_update_one (struct varobj *var, enum print_values print_values)
+varobj_update_one (struct varobj *var, enum print_values print_values,
+		   int explicit)
 {
   struct varobj **changelist;
   struct varobj **cc;
   struct cleanup *cleanup = NULL;
   int nc;
 
-  nc = varobj_update (&var, &changelist);
+  nc = varobj_update (&var, &changelist, explicit);
 
   /* nc == 0 means that nothing has changed.
      nc == -1 means that an error occured in updating the variable.
=== gdb/varobj.c
==================================================================
--- gdb/varobj.c	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/varobj.c	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -103,7 +103,7 @@
 
   /* 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.  */
+     NULL, or not lazy, or not_fetched is true.  */
   struct value *value;
 
   /* Did an error occur evaluating the expression or getting its value? */
@@ -126,6 +126,16 @@
 
   /* Was this variable updated via a varobj_set_value operation */
   int updated;
+
+  /* Is this variable frozen. Frozen variables are never implicitly
+     updated by -var-update * or -var-update <direct-or-indirect-parent>.
+  */
+  int frozen;
+
+  /* Is the value of this variable intentionally not fetched?  It is
+     not fetched if either the variable is frozen, or any parents is
+     frozen.  */
+  int not_fetched;
 };
 
 /* Every variable keeps a linked list of its children, described
@@ -682,7 +692,27 @@
   return var->format;
 }
 
+void
+varobj_set_frozen (struct varobj *var, int frozen)
+{
+  /* When a variable is unfrozen, we don't fetch its value.
+     The 'not_fetched' flag remains set, so next -var-update
+     won't complain.
+
+     We don't fetch the value, because for structures the client
+     should do -var-update anyway.  It would be bad to have different
+     client-size logic for structure and other types.  */
+  var->frozen = frozen;
+}
+
 int
+varobj_get_frozen (struct varobj *var)
+{
+  return var->frozen;
+}
+
+
+int
 varobj_get_num_children (struct varobj *var)
 {
   if (var->num_children == -1)
@@ -896,6 +926,7 @@
 { 
   int changeable;
   int changed = 0;
+  int intentionally_not_fetched = 0;
 
   var->error = 0;
   /* We need to know the varobj's type to decide if the value should
@@ -907,11 +938,24 @@
   /* The new value might be lazy.  If the type is changeable,
      that is we'll be comparing values of this type, fetch the
      value now.  Otherwise, on the next update the old value
-     will be lazy, which means we've lost that old value.  */
+     will be lazy, which means we've lost that old value.  */     
   if (changeable && value && value_lazy (value))
     {
-      if (!gdb_value_fetch_lazy (value))
+      struct varobj *parent = var->parent;
+      int frozen = var->frozen;
+      for (; !frozen && parent; parent = parent->parent)
+	frozen |= parent->frozen;
+
+      if (frozen && initial)
 	{
+	  /* For variables that are frozen, or are children of frozen
+	     variables, we don't do fetch on initial assignment.
+	     For non-initial assignemnt we do the fetch, since it means we're
+	     explicitly asked to compare the new value with the old one.  */
+	  intentionally_not_fetched = 1;
+	}
+      else if (!gdb_value_fetch_lazy (value))
+	{
 	  var->error = 1;
 	  /* Set the value to NULL, so that for the next -var-update,
 	     we don't try to compare the new value with this value,
@@ -937,28 +981,43 @@
 	{
 	  /* Try to compare the values.  That requires that both
 	     values are non-lazy.  */
-	  
-	  /* Quick comparison of NULL values.  */
-	  if (var->value == NULL && value == NULL)
-	    /* Equal. */
-	    ;
-	  else if (var->value == NULL || value == NULL)
-	    changed = 1;
+	  if (var->not_fetched && value_lazy (var->value))
+	    {
+	      /* This is a frozen varobj and the value was never read.
+		 Presumably, UI shows some "never read" indicator.
+		 Now that we've fetched the real value, we need to report
+		 this varobj as changed so that UI can show the real
+		 value.  */
+	      changed = 1;
+	    }
 	  else
-	    {
-	      gdb_assert (!value_lazy (var->value));
-	      gdb_assert (!value_lazy (value));
-	      
-	      if (!value_contents_equal (var->value, value))
+	    {	  
+	      /* Quick comparison of NULL values.  */
+	      if (var->value == NULL && value == NULL)
+		/* Equal. */
+		;
+	      else if (var->value == NULL || value == NULL)
 		changed = 1;
+	      else
+		{
+		  gdb_assert (!value_lazy (var->value));
+		  gdb_assert (!value_lazy (value));
+		  
+		  if (!value_contents_equal (var->value, value))
+		    changed = 1;
+		}
 	    }
 	}
     }
     
   /* We must always keep the new value, since children depend on it. */
-  if (var->value != NULL)
+  if (var->value != NULL && var->value != value)
     value_free (var->value);
   var->value = value;
+  if (value && value_lazy (value) && intentionally_not_fetched)
+    var->not_fetched = 1;
+  else
+    var->not_fetched = 0;
   var->updated = 0;
 
   return changed;
@@ -975,18 +1034,22 @@
     -2 if the type changed
     Otherwise it is the number of children + parent changed
 
-   Only root variables can be updated... 
+   The EXPLICIT parameter specifies if this call is result
+   of MI request to update this specific variable, or 
+   result of implicit -var-update *. For implicit request, we don't
+   update frozen variables.
 
    NOTE: This function may delete the caller's varobj. If it
    returns -2, then it has done this and VARP will be modified
    to point to the new varobj. */
 
 int
-varobj_update (struct varobj **varp, struct varobj ***changelist)
+varobj_update (struct varobj **varp, struct varobj ***changelist,
+	       int explicit)
 {
   int changed = 0;
   int error = 0;
-  int type_changed;
+  int type_changed = 0;
   int i;
   int vleft;
   struct varobj *v;
@@ -1002,57 +1065,70 @@
   if (changelist == NULL)
     return -1;
 
-  /*  Only root variables can be updated... */
-  if ((*varp)->root->rootvar != *varp)
-    /* Not a root var */
-    return -1;
 
+  /* Frozen means frozen -- we don't check for any change in
+     this varobj, including its going out of scope, or
+     changing type.  One use case for frozen varobjs is
+     retaining previously evaluated expressions, and we don't
+     want them to be reevaluated at all.  */
+  if (!explicit && (*varp)->frozen)
+    return 0;
+
   /* Save the selected stack frame, since we will need to change it
      in order to evaluate expressions. */
   old_fid = get_frame_id (deprecated_selected_frame);
 
-  /* Update the root variable. value_of_root can return NULL
-     if the variable is no longer around, i.e. we stepped out of
-     the frame in which a local existed. We are letting the 
-     value_of_root variable dispose of the varobj if the type
-     has changed. */
-  type_changed = 1;
-  new = value_of_root (varp, &type_changed);
-  if (new == NULL)
-    {
-      (*varp)->error = 1;
-      return -1;
-    }
-
-  /* Initialize a stack for temporary results */
+  /* Initialize a stack for temporary results.  */
   vpush (&result, NULL);
 
-  /* If this is a "use_selected_frame" varobj, and its type has changed,
-     them note that it's changed. */
-  if (type_changed)
-    {
-      vpush (&result, *varp);
-      changed++;
-    }
+  /* Initialize a stack for traversing children.  */
+  vpush (&stack, NULL);
 
-  if (install_new_value ((*varp), new, type_changed))
+  if ((*varp)->root->rootvar == *varp)
     {
-      /* If type_changed is 1, install_new_value will never return
-	 non-zero, so we'll never report the same variable twice.  */
-      gdb_assert (!type_changed);
-      vpush (&result, (*varp));
-      changed++;
+      /* Update the root variable. value_of_root can return NULL
+	 if the variable is no longer around, i.e. we stepped out of
+	 the frame in which a local existed. We are letting the 
+	 value_of_root variable dispose of the varobj if the type
+	 has changed. */
+      type_changed = 1;
+      new = value_of_root (varp, &type_changed);
+      if (new == NULL)
+	{
+	  (*varp)->error = 1;
+	  return -1;
+	}
+      
+      /* If this is a "use_selected_frame" varobj, and its type has changed,
+	 them note that it's changed. */
+      if (type_changed)
+	{
+	  vpush (&result, *varp);
+	  changed++;
+	}
+      
+      if (install_new_value ((*varp), new, type_changed))
+	{
+	  /* If type_changed is 1, install_new_value will never return
+	     non-zero, so we'll never report the same variable twice.  */
+	  gdb_assert (!type_changed);
+	  vpush (&result, (*varp));
+	  changed++;
+	}
+      
+      /* Push the root's children */
+      if ((*varp)->children != NULL)
+	{
+	  struct varobj_child *c;
+	  for (c = (*varp)->children; c != NULL; c = c->next)
+	    if (!c->child->frozen)
+	      vpush (&stack, c->child);
+	}
     }
-
-  /* Initialize a stack */
-  vpush (&stack, NULL);
-
-  /* Push the root's children */
-  if ((*varp)->children != NULL)
+  else
     {
-      struct varobj_child *c;
-      for (c = (*varp)->children; c != NULL; c = c->next)
-	vpush (&stack, c->child);
+      /* This is not a root variable.  Push it to stack itself.  */
+      vpush (&stack, *varp);
     }
 
   /* Walk through the children, reconstructing them all. */
@@ -1064,9 +1140,10 @@
 	{
 	  struct varobj_child *c;
 	  for (c = v->children; c != NULL; c = c->next)
-	    vpush (&stack, c->child);
+	    if (!c->child->frozen)
+	      vpush (&stack, c->child);
 	}
-
+      
       /* Update this variable */
       new = value_of_child (v->parent, v->index);
       if (install_new_value (v, new, 0 /* type not changed */))
@@ -1435,6 +1512,8 @@
   var->format = 0;
   var->root = NULL;
   var->updated = 0;
+  var->frozen = 0;
+  var->not_fetched = 0;
 
   return var;
 }
@@ -2112,6 +2191,12 @@
 	    struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
 	    char *thevalue;
 
+	    if (var->not_fetched && value_lazy (var->value))
+	      /* Frozen variable and no value yet.  We don't
+		 implicitly fetch the value.  MI response will
+		 use empty string for the value, which is OK.  */
+	      return NULL;
+
 	    gdb_assert (type_changeable (var));
 	    gdb_assert (!value_lazy (var->value));
 	    common_val_print (var->value, stb,
=== gdb/varobj.h
==================================================================
--- gdb/varobj.h	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/varobj.h	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -78,6 +78,10 @@
 extern enum varobj_display_formats varobj_get_display_format (
 							struct varobj *var);
 
+extern void varobj_set_frozen (struct varobj *var, int frozen);
+
+extern int varobj_get_frozen (struct varobj *var);
+
 extern int varobj_get_num_children (struct varobj *var);
 
 extern int varobj_list_children (struct varobj *var,
@@ -97,6 +101,7 @@
 
 extern int varobj_list (struct varobj ***rootlist);
 
-extern int varobj_update (struct varobj **varp, struct varobj ***changelist);
+extern int varobj_update (struct varobj **varp, struct varobj ***changelist,
+			  int explicit);
 
 #endif /* VAROBJ_H */