Re: [RFA] 12266 (typedef'd parameters) revisited again - what should go for gdb-7.3?

[Keith Seitz <keiths@redhat.com>]

On 06/07/2011 01:11 PM, Jan Kratochvil wrote:

> You have said off-list you have a fix but you still run the real-world .debug
> files regression tests on top of it.

A fix, but not a complete fix. I am running tests against libstdc++, and 
that is uncovering some additional problems.

> I believe the replace_typedefs part (DEMANGLE_COMPONENT_* typedef
> replacements) should be cross-checked against `set debug check-physname on'
> which I guess hasn't been done yet.

I agree, and I will do that when I can successfully get through 
libstdc++ (and libwebkit, others?).

> The question (at least for me) is now which parts of this patch should go for
> 7.3 and which only for 7.4+:
>
> It can be split into the parts before before:
> 1/4: changes cp-name-parser.y to not use shared memory.
> 2/4: differences between print names and "linkage names" (physnames).
> 3/4: cp_canonicalize_no_typedefs to decode_variable and decode_compound
> 4/4: tests for this new feature.

#2 is quite independent of the others. #1 is only needed for #3 (as is 
#4). IMO, #3 is a long ways away, and as it addresses new features, it 
is far too risky to include in 7.3.

So the only one I would consider is #2, which addresses differences in 
physname/DW_AT_linkage_name and "print" names. If that has actual 
benefits to something today, we can decide to include it. You have, 
however, found other ways to "skin the cat," so this might not be needed 
until the rest of the 12266 patch is really ready to go in.

> PR 12506 is I believe fixed by 2/4.  But for compilers featuring
> DW_AT_MIPS_linkage_name (=G++) it is fixed also by the more general:
> 	Re: [patch] Follow DW_AT_linkage_name for methods #2
> 	http://sourceware.org/ml/gdb-patches/2011-06/msg00040.html
> And I believe I could still find a countercase where DW_AT_linkage_name is
> more correct than the current physname code.

I would say this part is really at your/maintainer's discretion(s). I 
have been working under the assumption that we were all in agreement 
about moving to preferring DW_AT_linkage_name over physnames.

In this context, I would say (as I always do), take the least risky 
approach, and I do not think today that 
cp_canonicalize_string_no_typedefs is the least risky. Like physnames, 
it works in a great many instances, but it is far from perfect/bug-free.

> ctors/dtors do not have DW_AT_MIPS_linkage_name but for class based naming
> (C::C or C::~C) one can drop to the minimal symbols (the fallback needs to
> work even for other cases):
> 	[patch 1/2] physname reg.: linespec minsym fallback
> 	http://sourceware.org/ml/gdb-patches/2011-06/msg00079.html
> and one IMO cannot use linespec to refer to ctors/dtors (only to regular
> methods) during variable base naming (var.C or var.~C) - where the minsym
> fallback is not applicable and where decode_compound could be useful.

Additionally, I would even suggest adding a fallback to decode_variable 
if decode_compound fails. This is, I believe, the biggest linespec 
regression honey pot we've seen.

> The 3/4 is a new feature for PR 12266.  GDB could never
> do `break f(typedefparam)', I do not think it needs to go for 7.3 so late.

Absolutely agreed.

Keith