Re: Ping: frozen variable objects

Daniel Jacobowitz
2007-04-10T19:03:56Z

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