Re: [RFC] MI notification on register changes

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> User can type command in console 'set $reg = foo' to modify register.
Yao> The modification may not really happen in registers, because register
Yao> is already saved in frame, and modification really goes to memory (frame).
Yao> The new MI notification this patch adds is about 'register change' from
Yao> user's point of view, instead of from machine's point of view.

Yao> Usually, MI frontend has a 'register view' to show the contents of
Yao> registers.  When users modify registers on a certain frame, this
Yao> notification is useful to tell MI frontend to refresh its 'register
Yao> view'.

This all seems reasonable to me.

Yao> This patch also extends the usage of field 'regnum' of 'struct value'.
Yao> Originally, this field is used to store the register number if content
Yao> is from register.  With this patch, this field also store the register
Yao> number if it "presents" a register.  For example, on non-innermost
Yao> frame, the value "reg" may be from memory instead of register.
Yao>    set $reg = foo

The register may be stored in memory, but it this case really ever
represented as lval_memory in gdb?  I thought the reason for
value::frame_id was to represent the notion of "register in a specific
frame", and that all registers are represented as lval_register.

Yao> +proc test_register_change { } { with_test_prefix "simple" {

For new tests I would prefer correct indentation.  That is,
with_test_prefix on its own line.  I think the existing tests are
written in this funny way to avoid a massive reindentation, but that
isn't a factor here.

Yao> +++ b/gdb/valops.c
Yao> @@ -1217,6 +1217,7 @@ value_assign (struct value *toval, struct value *fromval)
 
Yao> +  /* Figure out which frame this is in currently.  */
Yao> +  frame = frame_find_by_id (VALUE_FRAME_ID (toval));
Yao> +

I find it pretty hard to convince myself that moving this out of the
switch can't cause some obscure problem.

How about hoisting the declaration of frame, initializing it to NULL,
and then changing this:

Yao> +  if (VALUE_REGNUM (toval) >= 0)
Yao> +    observer_notify_register_changed (VALUE_REGNUM (toval), frame,
Yao> +				      inferior_ptid,
Yao> +				      value_contents (val));

... to read 'if (frame != NULL)'?

Tom