Re: Ping: frozen variable objects

[Daniel Jacobowitz <drow@false.org>]

On Sun, Mar 25, 2007 at 12:51:42PM +0300, Vladimir Prus wrote:
> --- gdb/doc/gdb.texinfo	(/mirrors/gdb_mainline)	(revision 3540)
> +++ gdb/doc/gdb.texinfo	(/patches/gdb/frozen/gdb_mainline)	(revision 3540)
> @@ -19730,6 +19730,8 @@ access this functionality:
>  @tab set the value of this variable
>  @item @code{-var-update}
>  @tab update the variable and its children
> +@item @code{-var-set-frozen}
> +@tab set frozeness attribute
>  @end multitable
>  
>  In the next subsection we describe each operation in detail and suggest

Hmm, that line at the bottom says there's detailed documentation
below.  But you didn't add any for -var-set-frozen.

There's also this:

> +  if (varobj_get_frozen (var))
> +    ui_out_field_int (uiout, "frozen", 1);

Somewhere in the manual should mention how that can happen, and what
it means.

> +    mi_check_varobj_value V1.nested.j 2 "check V1.nested.j: 2"
> +    mi_check_varobj_value V1.nested.k 3 "check V1.nested.j: 3"    
> +    # Check that explicit update for elements of structures
> +    # works.
> +    # Update v1.j
> +    mi_varobj_update V1.nested.j {V1.nested.j} "update V1.nested.j"
> +    mi_check_varobj_value V1.i 1 "check V1.i: 1"
> +    mi_check_varobj_value V1.nested.j 8 "check V1.nested.j: 8"
> +    mi_check_varobj_value V1.nested.k 3 "check V1.nested.j: 3"    
> +    # Update v1.nested, check that children is updated.
> +    mi_varobj_update V1.nested {V1.nested.k} "update V1.nested"
> +    mi_check_varobj_value V1.i 1 "check V1.i: 1"
> +    mi_check_varobj_value V1.nested.j 8 "check V1.nested.j: 8"
> +    mi_check_varobj_value V1.nested.k 9 "check V1.nested.j: 9"    

You've got a bunch of .j's in the test names that ought to be .k's.
Happens some more below.

> +  if (argc != 2)
> +    error (_("mi_cmd_var_set_format: Usage: NAME FROZEN_FLAG."));

I think we decided this should be "-var-set-format:" instead of the
function name.

> +  /* Is this variable frozen. Frozen variables are never implicitly
> +     updated by -var-update * or -var-update <direct-or-indirect-parent>.
> +  */

Spaces between sentences, and */ on a line by itself - this is one of
the awkward bits of our formatting standards when the */ doesn't quite
fit on the previous line but I usually just wrap early.  I think
emacs's M-q will too.

> @@ -943,10 +974,23 @@ install_new_value (struct varobj *var, s
>    /* 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.  */     

This added a bunch of trailing whitespace to the line for some reason.

> +      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;
> +	}

Does this mean "-var-assign VAR value" is always going to read VAR?
If it's frozen that seems undesirable; maybe we should just assume
it's been updated.

For hardware mapped registers, some of this may not be right: I
noticed that we're saving the installed value, but the hardware could
swizzle it as it is entered.  The most common place this happens is
with registers where a bit always reads as zero, no matter what you
write to it.  But that's a read-sensitivity problem, not a frozen
varobjs problem.

> -	  
> -	  /* Quick comparison of NULL values.  */
> -	  if (var->value == NULL && value == NULL)
> -	    /* Equal. */
> -	    ;
> -	  else if (var->value == NULL || value == NULL)
> +	  if (var->not_fetched && value_lazy (var->value))
>  	    {
> -	      xfree (var->print_value);
> -	      var->print_value = value_get_print_value (value, var->format);
> +	      /* 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
> +          else
>  	    {

You don't actually need to change the nesting here.  You can just add
a leading if to the existing if / else if / else.  That'd be easier to read.

>  	  if (cvalue && value)
>  	    {
> -	      *cvalue = value_cast (TYPE_FIELD_TYPE (type, index), value);
> +              *cvalue = value_cast (TYPE_FIELD_TYPE (type, index), value);
>  	    }

Random tab damage?

-- 
Daniel Jacobowitz
CodeSourcery