Re: [RFA] Delayed physname computation

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> The solution I've implemented is to simply delay processing of method
Keith> physnames until the entire CU has been read/processed.

I read through this and I think it looks good.
I do have some questions, nothing major.

Keith> Unfortunately, this is another example in which the compiler used may
Keith> alter the test outcome. As a result, I've had to hack in a test case
Keith> which is independent of the compiler. I would appreciate special
Keith> attention to the test source file (pr11465.S), which is an edited
Keith> version of gcc's --save-temps output.

This sort of thing is fine, there are other tests made the same way.
I didn't try to read the assembly, but Jan's updated test is ok when
this patch goes in.

Keith> +static void
Keith> +add_to_method_list (struct type *type, int fnfield_index, int index,
Keith> +		    const char *name, struct die_info *die,
Keith> +		    struct dwarf2_cu *cu)

This needs some kind of header comment.  It can be pretty mild.

Keith> +      physname = (char *) dwarf2_physname ((char *) mi->name, mi->die, cu);

It would be nice to make all this stuff const-correct.  It is definitely
not needed for this patch, I just didn't notice this earlier.

Keith> +  /* If methods were found in the partial symbol table, we allocate one
Keith> +     big buffer to hold the entire delayed list for the CU.  */
Keith> +  if (per_cu->num_methods)
Keith> +    {
Keith> +      cu->method_list
Keith> +	= xmalloc (per_cu->num_methods * sizeof (struct delayed_method_info));
Keith> +    }

I didn't go dig through the code, so consider this a conditional
request.  If we are careful about cleanups here, then this needs a
cleanup; otherwise an error results in a memory leak.  If we aren't
already careful, then don't bother, I think we will need to make a pass
through this to make the reader a bit more robust in the face of weird
DWARF.  In this case, either file a bug or let me know and I will file
one.

Keith>      complaint (&symfile_complaints, _("member function type missing for '%s'"),
Keith> -	       physname);
Keith> +	       dwarf2_physname (fieldname, die, cu));

Can this call to dwarf2_physname exhibit the problem that this patch is
trying to circumvent?

Tom