Re: [RFA] Limited DW_OP_piece support

[Kevin Buettner <kevinb at redhat dot com>]

On May 22,  2:19pm, Daniel Jacobowitz wrote:

> On Thu, May 22, 2003 at 10:00:39AM -0700, Kevin Buettner wrote:
> > The patch below adds limited DW_OP_piece support to dwarf2expr.c.  I
> > will post a patch to rs6000-tdep.c which illustrates what a
> > ``dwarf2_compose_register_pieces'' method should look like.
> > 
> > Okay?
> 
> I would really strongly prefer that we not do it this way.
> 
> You'll notice that there are no other gdbarch calls in the expression
> evaluator.  There might be some implicit ones through macros, for
> instance there is TARGET_ADDR_BIT.  That needs to be fixed properly
> some day already.
> 
> Instead, IMHO, we should devise a way to represent multiple locations
> in the evaluator's return value.  This is not suggesting the complete
> overhaul that we need to support multiple locations in the rest of GDB.
> Then have the expression evaluator properly return a list of locations,
> and have the massaging done via gdbarch in the evaluator's client. 
> Does that sound reasonable?

I must admit that it sure sounded reasonable when I first read it. 
I've been looking at the code to see how doable it is, and it's
looking less reasonable to me now.  It appears to me that there are
multiple clients and it seems ugly to do the massaging that you speak
of in multiple places.  (Or perhaps I misunderstand who the client
is?)

I don't want to argue too vigorously for the limited DW_OP_piece patch
that I submitted because I agree that it'd be better to have it
implemented in a more generic manner.  The one thing that my patch
does have going for it though is that, aside from the addition of the
gdbarch method and the architecture dependent support, it consists
solely of the addition of one case to a switch statement.  This makes
it very easy to back out when we finally do things "right".  Simply
delete that case (and the gdbarch method) and it's out.  I'd even be
willing to add some comments and possibly an ifdef to that case to
further delimit it from the rest of the code.  The comments would make
it clear that this is a stopgap measure that will eventually be
removed and would describe precisely how to remove it.

Kevin