Re: [RFA] [4/5] Use DWARF-2 DW_AT_artificial information

[Daniel Jacobowitz <drow@mvista.com>]

On Wed, May 15, 2002 at 02:15:23PM -0400, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > On Wed, May 15, 2002 at 12:35:43AM -0400, Elena Zannoni wrote:
>  > > Daniel Jacobowitz writes:
>  > >  > I hate to be a nag, but this patch would be useful for some of my
>  > >  > current work.  Do you have a chance to look at it?
>  > >  > 
>  > > 
>  > > You are completely right, sorry.  I wonder if MichaelC could kindly
>  > > run it through his test harness. That was a big help with yesterday's
>  > > patch.  And if you can run the tests with the maintainers file script.
>  > 
>  > (Don't bother, Michael, it doesn't apply any more.)
>  > 
>  > I looked at this in passing and noticed it touched some of the same
>  > files I was working on.  I completely forgot that my previous patches
>  > completely invalidated it :)  Sorry for wasting your time.
>  > 
> 
> No prob.
> 
>  > > How much does the size increase by adding this new struct?
>  > 
>  > I don't have any numbers, unfortunately.  I'll try to get some, unless
>  > I think of a better way to do it...
>  > 
>  > > I have looked at it when you first posted it, and I had some
>  > > questions, I have to go fish for them again.  But basically, the
>  > > motivation for this change is what? You need to handle the dwarf2
>  > > information for artificial arguments, right? So that needs a change in
>  > > dwarf2read.c.  How does that bring about the change in the type
>  > > structure? Can you explain a bit? (my brain gets lazy at this time).
>  > 
>  > OK, let me explain this a bit.  Right now the only information we save
>  > for a method type are some flags, the return type, and a list of
>  > argument types - just as an array of struct type *.  I needed another
>  > bit per argument, and I couldn't find anywhere to put it.  Maintaining
>  > a separate bitmap is even uglier.
>  > 
> 
> How does the type structure look right now for a c++ class method?
> 
> There is a TYPE struct for the class. 
> 
> The TYPE_SPECIFIC field points to CPLUS_STUFF which has an array of
> unique methods (FN_FIELDLISTS).
> 
> For each of these methods there is another list (FN_FIELDS) of all the
> overloaded methods with the same name (which may be just 1 if not
> overloaded).
> 
> For each of them there is an array of arguments (ARGS). (which is just
> a type struct).
> 
> You need to add something to this array of ARGS to indicate if they
> are artificial.
> 
> There is already an occurrence of 'artificial' but that's for methods.
> 
> Yes?
> 
> The thing I don't understand in your patch (or is this messy code in
> general) is why was type_specific.arg_types changed to
> type_specific.method_args.  Wouldn't it be enough to change the args
> array inside the fn_field structure from an array of types to an array
> of method_args?  Basically I don't think I understand why the type
> system seems to be storing this info in 2 places:
> 
> in the cplus_struct:
>             /* The argument list.  Only valid if is_stub is clear.  Contains
>                the type of each argument, including `this', and ending with
>                a NULL pointer after the last argument.  Should not contain
>                a `this' pointer for static member functions.  */
> 
>             struct type **args;
> 
> in type_specific:
>         /* ARG_TYPES is for TYPE_CODE_METHOD.
>            Contains the type of each argument, ending with a void type
>            after the last argument for normal member functions or a NULL
>            pointer after the last argument for functions with variable
>            arguments.  */
> 
>         struct type **arg_types;
> 
> Argh!!!
> This occurrence of args in fn_field was *removed* in 1996 by Peter Schauer,
> and *replaced* by arg_types. 
> But guess what?? The HP merge reintroduced the args, forgetting about the
> existance of arg_types.
> 
> Grrrr. No wonder I didn't understand the layout.  Ok, let's get rid of
> the cplus_struct args again, I would suggest, and stick with the other.
> This may require a bit of careful surgery. Proably uses of args have
> propagated all over the place.
> 
> Then you would have only one place to modify for the artificial attribute.
> I bet your patch would become much much simpler.

Excuse the language, but... "Arg!"

This merge has really begun to annoy me.  It also probably explains a
bit of beauty that I found in gdbtypes.h the other day, which of course
I can no longer find now that I'm looking for it; there appear to be
some accessor macros accessing things that don't exist.

> If you get rid of the second instance of args, it would become easier.

OK, you want to kill the one in cplus_struct.  This should actually be
very easy; the only client is hpread.c.  If C++ support still worked on
HP/UX someone would probably have noticed...  I'll do that first and
then begin again.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer