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

[Pedro Alves <pedro@codesourcery.com>]

On Tuesday 25 October 2011 21:30:22, Joel Brobecker wrote:
> 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.

Correct.  Ideally we'd replace those hacks by separate 
setter and a getter, or better yet, see if we can get
rid of the need to have a setter.  VALUE_REGNUM as an
rval is definitely okay, as an lval, not so okay.

> 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.  

I'd think we'd reach here with typedef'ed variables that lives in
a registers, but maybe we're stripping typedefs earlier per chance.

> One might even wonder if it would
> make sense to adjust TYPE_LENGTH to to a check_typedef systematically...

Maybe.  I guess that we'd need profiling to make sure that wouldn't
regress performance.  I don't know if we have paths were we'd
needlessly call check_typedef more times more.

-- 
Pedro Alves