[patch] Change cplus_specific to an alocated struct

Tom Tromey tromey@redhat.com
Tue Jun 8 17:02:00 GMT 2010


>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> I plan to expand the cplus_specific struct to store template
Sami> information needed, of course, only for C++. Tom suggested that I move
Sami> the substruct out of struct general_symbol_info and change its
Sami> reference to a pointer to be assigned to the struct if allocated.

Sami> Thoughts ?

I think the idea is fine.

Sami>  char *
Sami>  ada_decode_symbol (const struct general_symbol_info *gsymbol)
Sami>  {
Sami> -  char **resultp =
Sami> -    (char **) &gsymbol->language_specific.cplus_specific.demangled_name;
Sami> -  if (*resultp == NULL)
Sami> +  char *result = symbol_get_cplus_demangled_name (gsymbol);

I don't think this change is correct -- but FWIW I don't think the
existing code is correct, either.

The code is written this way because this function caches the demangled
name in the cplus_specific struct.  Your change removes this caching.

However, this takes a general_symbol_info and so, presumably, can be
used for partial symbols.  But, modifying a partial symbol is a no-no,
because they are stored in a bcache.

All this reminds me -- you should look at bcache utilization and memory
use before and after your changes to make sure we aren't hitting a
memory use regression here.  There is some command that will show you
bcache statistics, though I forget what it is offhand.

Sami> +char *
Sami> +symbol_get_cplus_demangled_name (const struct general_symbol_info *gsymbol)
Sami> +{
Sami> +  if (gsymbol->language_specific.cplus_specific != NULL)
Sami> +    return gsymbol->language_specific.cplus_specific->demangled_name;
Sami> +
Sami> +  return NULL;
Sami> +}

Perhaps we should have different types of structs in the
language_specific union, and have functions like this examine the
language field.

It seems to me that soon we're going to want to add a bunch of
C++-specific fields, and we don't want to unnecessarily penalize the
other languages with our baggage.

If you were planning that for a follow-on patch, that is fine -- but
also the sort of thing that it is handy to note in your submissions.

Sami> +/* Initialize the cplus_specific structure.  'cplus_specific' is intended to
Sami> +   be allocated lazily.  So this should only be called if the structure is
Sami> +   needed.  */
Sami> +static void
Sami> +symbol_init_cplus_specific (struct general_symbol_info *gsymbol,
Sami> +                           struct objfile *objfile)

Initializing lazily is usually good, but it breaks the bcache.
However perhaps this can be fixed by having the partial symbol code
force the issue.

Tom



More information about the Gdb-patches mailing list