Re: [RFA] Typedef'd method parameters [2/4]

[Keith Seitz <keiths at redhat dot com>]

On 04/27/2011 11:52 AM, Jan Kratochvil wrote:
> On Thu, 21 Apr 2011 23:15:33 +0200, Keith Seitz wrote:
> > Comments/questions/concerns?
>
> This patchset has a regression for:
> FAIL: gdb.fortran/library-module.exp: print var_i in lib
> FAIL: gdb.fortran/library-module.exp: print var_i in main
> FAIL: gdb.fortran/library-module.exp: print var_j
> FAIL: gdb.fortran/module.exp: stopped language detection
> FAIL: gdb.fortran/module.exp: fully qualified name of DW_TAG_constant
> FAIL: gdb.fortran/module.exp: print var_i value 1
> FAIL: gdb.fortran/module.exp: print var_i value 2
> FAIL: gdb.fortran/module.exp: print var_b
> FAIL: gdb.fortran/module.exp: print var_d
> FAIL: gdb.fortran/module.exp: print var_i value 14
>
> As this Fortran modules code is by me going to check more if I haven't misused
> some C++ structures inappropriately.

Nope, this regression is caused by this patch.. I don't normally test 
anything other than C, C++, and Java. I have updated the patch to 
correct these failures.

> What is both LINKAGE_NAME and PRINT_NAME are set?  There is already introduced
> `enum name_kind' in this patchset for that, it would make the caller
> statements more clear.

I've changed this.

> > --- dwarf2read.c	15 Apr 2011 15:05:04 -0000	1.524
> > +++ dwarf2read.c	20 Apr 2011 23:38:52 -0000
> [...]
> > @@ -11002,6 +11023,20 @@ new_symbol_full (struct die_info *die, s
> >         linkagename = dwarf2_physname (name, die, cu);
> >         SYMBOL_SET_NAMES (sym, linkagename, strlen (linkagename), 0, objfile);
> >
> > +      /* For C++ set the symbol's demangled name if it is different than
> > +	 the computed physname.  This can happen when the source defines
> > +	 a method with typedef'd parameters.  This is ultimately used by
> > +	 the type printer.  */
> > +      if (cu->language == language_cplus&&  die->tag == DW_TAG_subprogram)
> > +	{
> > +	  const char *print_name = dwarf2_print_name (name, die, cu);
>
> Missing empty line.

Fixed.

> > --- symtab.c	19 Apr 2011 18:04:07 -0000	1.266
> > +++ symtab.c	20 Apr 2011 23:38:55 -0000
> > @@ -740,17 +740,21 @@ symbol_demangled_name (const struct gene
> >     return NULL;
> >   }
> >
> > -/* Return the search name of a symbol---generally the demangled or
> > -   linkage name of the symbol, depending on how it will be searched for.
> > -   If there is no distinct demangled name, then returns the same value
> > -   (same pointer) as SYMBOL_LINKAGE_NAME.  */
> > +/* Return the search name of a symbol.  This is typically the smybol's
>                                                                   ^^^^^^=symbol

Fixed.

> > +   physname (SYMBOL_LINKAGE_NAME). Symbols with mangled names
> > +   are special: return their SYMBOL_NATURAL_NAME instead.  */
>
> I do not find this comment sufficient.  With the extended macros comments
> I would now prefer just:
>
> /* Return the SYMBOL_SEARCH_NAME kind of name for GSYMBOL.  */

Done.

> >   /* Initialize the structure fields to zero values.  */
> > @@ -2900,8 +2904,8 @@ compare_search_syms (const void *sa, con
> >     struct symbol_search **sym_a = (struct symbol_search **) sa;
> >     struct symbol_search **sym_b = (struct symbol_search **) sb;
> >
> > -  return strcmp (SYMBOL_PRINT_NAME ((*sym_a)->symbol),
> > -		 SYMBOL_PRINT_NAME ((*sym_b)->symbol));
> > +  return strcmp (SYMBOL_SEARCH_NAME ((*sym_a)->symbol),
> > +		 SYMBOL_SEARCH_NAME ((*sym_b)->symbol));
>
> As this comparison is used only for symbols sorting (and not even any symbols
> duplication removal) and print_symbol_info still uses SYMBOL_PRINT_NAME
> I would prefer to keep it as is.  Or do you see any problem with it?

I've removed this change. It is superfluous to the patch, and we can 
revisit later if we can find a demonstrable test case for it (which I 
have not been able to do).

> I would check in / include with this patchset a fixup of the sometimes already
> incorrect before this patchset and definitely incorrect after this patchset
> comments.

I will make sure that this is committed at the appropriate time.

I've attached the revised patch.

Keith

Attachment: printname-2.patch
Description: Text document