Re: [RFA/RFC] Restore old handling of multi-register variables

[Joel Brobecker <brobecker at adacore dot com>]

Hi Pedro,

Thanks for the review. One question:

> > +/* VALUE must be an lval_register value.  If regnum is the value's
> > +   associated register number, and len the length of the values type,
> > +   read one or more registers in FRAME, starting with register REGNUM,
> > +   until we've read LEN bytes.  */
> > +
> > +void
> > +read_frame_register_value (struct value *value, struct frame_info *frame)
> 
> I think this should be in frame.c instead.  value.c is for core
> struct value stuff.

That's what I thought originally too.  The reason why I didn't put
that function there is because I thought that the only way to access
some of the fields was by using the deprecated_[...]_hack functions.
So I thought we weren't supposed to be able to access those components
of a struct value.  But looking closer, I think I get the reason why
it's called a hack and deprecated - it's to allow the previous usage
of using the VALUE_something macros to change the value of the
associated component. So I'm assuming that...

        regnum = VALUE_REGNUM (val)

... is OK. While...

        VALUE_REGNUM (val) = regnum

... is definitely frowned upon.

I will make that change if you agree.

> > +  const int len = TYPE_LENGTH (value_type (value));
> 
> Do we need check_typedefs here?

I haven't faced a situation where this might make a difference,
but I think you are right. When taking the length of a type,
it should never be a typedef.  One might even wonder if it would
make sense to adjust TYPE_LENGTH to to a check_typedef systematically...

-- 
Joel