This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] [4/5] Use DWARF-2 DW_AT_artificial information
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