This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Typedef'd method parameters [2/4]
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Keith Seitz <keiths at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 27 Apr 2011 20:52:49 +0200
- Subject: Re: [RFA] Typedef'd method parameters [2/4]
- References: <4DB09E75.20509@redhat.com>
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.
> --- c-typeprint.c 22 Mar 2011 17:35:22 -0000 1.70
> +++ c-typeprint.c 20 Apr 2011 23:38:49 -0000
> @@ -44,14 +44,22 @@ static void c_type_print_varspec_prefix
> static void c_type_print_modifier (struct type *,
> struct ui_file *,
> int, int);
> +
> +static void c_type_print_base_internal (struct type *, struct ui_file *,
> + int, int, int);
> +
>
> -/* LEVEL is the depth to indent lines by. */
> +/* The real c_print_type. See c_print_type below for a description
> + of parameters and usage.
>
> -void
> -c_print_type (struct type *type,
> - const char *varstring,
> - struct ui_file *stream,
> - int show, int level)
> + LINKAGE_NAME should be non-zero if we are printing a linkage name
> + on the stream. */
> +
> +static void
> +c_print_type_internal (struct type *type,
Such kind of functions is called c_print_type_1 in GDB AFAIK, not requesting
a change.
[...]
> @@ -386,17 +406,25 @@ c_type_print_modifier (struct type *type
>
>
> /* Print out the arguments of TYPE, which should have TYPE_CODE_METHOD
> - or TYPE_CODE_FUNC, to STREAM. Artificial arguments, such as "this"
> - in non-static methods, are displayed if LINKAGE_NAME is zero. If
> - LINKAGE_NAME is non-zero and LANGUAGE is language_cplus the topmost
> - parameter types get removed their possible const and volatile qualifiers to
> + or TYPE_CODE_FUNC, to STREAM.
> +
> + LINKAGE_NAME is non-zero when the SYMBOL_LINKAGE_NAME of TYPE
> + (a function or method) is printed. Artifical parameters such as "this"
> + in non-static methods are skipped and all typedefs will be removed/expanded.
> + Additionally if LANGUAGE is language_cplus, topmost parameter types
> + may have any possible const and volatile qualifiers removed to
> match demangled linkage name parameters part of such function type.
> +
> + PRINT_NAME is non-zero when the SYMBOL_PRINT_NAME of TYPE is
> + printed. In this case, artificial parameters are skipped, but
> + any typedef'd parameter types are left intact.
> +
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.
> +/* Print the name of the type (or the ultimate pointer target,
> + function value or array element), or the description of a structure
> + or union.
> +
> + SHOW positive means print details about the type (e.g. enum
> + values), and print structure elements passing SHOW - 1 for show.
> +
> + SHOW negative means just print the type name or struct tag if there
> + is one. If there is no name, print something sensible but concise
> + like "struct {...}".
> +
> + SHOW zero means just print the type name or struct tag if there is
> + one. If there is no name, print something sensible but not as
> + concise like "struct {int x; int y;}".
> +
> + LEVEL is the number of spaces to indent by.
> + We increase it for some recursive calls. */
> +
> +void
> +c_type_print_base (struct type *type, struct ui_file *stream,
> + int show, int level)
> +{
> + c_type_print_base_internal (type, stream, show, level, 0);
> +}
It has 4 callers in total, it may not be worth creating a wrapper.
[...]
> --- 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.
> + if (strcmp (print_name, linkagename))
> + {
> + symbol_set_demangled_name (&(sym->ginfo),
> + (char *) print_name, NULL);
> + }
> + }
> +
> /* Fortran does not have mangling standard and the mangling does differ
> between gfortran, iFort etc. */
> if (cu->language == language_fortran
[...]
> --- 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
> + 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. */
> char *
> symbol_search_name (const struct general_symbol_info *gsymbol)
> {
> if (gsymbol->language == language_ada)
> return gsymbol->name;
> else
> - return symbol_natural_name (gsymbol);
> + {
> + if (gsymbol->name && gsymbol->name[0] == '_' && gsymbol->name[1] == 'Z')
> + return symbol_natural_name (gsymbol);
> + else
> + return gsymbol->name;
> + }
> }
>
> /* 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));
This change is difficult to evaluate as SYMBOL_PRINT_NAME
#define SYMBOL_PRINT_NAME(symbol) \
(demangle ? SYMBOL_NATURAL_NAME (symbol) : SYMBOL_LINKAGE_NAME (symbol))
is question about whether to DEMANGLE or NOT to DEMANGLE, that is about
`set print demangle', which is already broken by physname:
[Bug symtab/12707] New: physname regression: set print demangle off
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?
This function should affect only:
rbreak
info variables
info functions
info types
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.
Thanks,
Jan
gdb/
2011-04-27 Jan Kratochvil <jan.kratochvil@redhat.com>
* symtab.h (SYMBOL_NATURAL_NAME, SYMBOL_LINKAGE_NAME)
(SYMBOL_DEMANGLED_NAME, SYMBOL_SEARCH_NAME): Extend the comments.
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -219,21 +219,33 @@ extern void symbol_set_names (struct general_symbol_info *symbol,
/* Return SYMBOL's "natural" name, i.e. the name that it was called in
the original source code. In languages like C++ where symbols may
be mangled for ease of manipulation by the linker, this is the
- demangled name. */
+ demangled name. This function always returns original typedefs and
+ const/volatile qualifiers present in parameter types such as
+ `f(int_typedef const)' */
#define SYMBOL_NATURAL_NAME(symbol) \
(symbol_natural_name (&(symbol)->ginfo))
extern char *symbol_natural_name (const struct general_symbol_info *symbol);
-/* Return SYMBOL's name from the point of view of the linker. In
- languages like C++ where symbols may be mangled for ease of
- manipulation by the linker, this is the mangled name; otherwise,
- it's the same as SYMBOL_NATURAL_NAME. */
+/* For minimal symbols return SYMBOL's name from the point of view of the
+ linker. In languages like C++ where symbols may be mangled for ease of
+ manipulation by the linker, this is the mangled name,
+ for `f(int_typedef const)' it is `_Z1fi'; otherwise, it's the same as
+ SYMBOL_NATURAL_NAME.
+
+ For full symbols return its demangled form of view of the linker, that is
+ with typedefs and toplevel const/volatile qualifiers of parameters removed,
+ for `f(int_typedef const)' it is `f(int)'. If no typedefs/qualifiers are
+ in use it's the same as SYMBOL_NATURAL_NAME. The mangled symbol name is not
+ available for full symbols. */
#define SYMBOL_LINKAGE_NAME(symbol) (symbol)->ginfo.name
/* Return the demangled name for a symbol based on the language for
- that symbol. If no demangled name exists, return NULL. */
+ that symbol. This function always returns typedefs and toplevel
+ const/volatile qualifiers of parameters preserved. If no demangled name
+ exists, return NULL. */
+
#define SYMBOL_DEMANGLED_NAME(symbol) \
(symbol_demangled_name (&(symbol)->ginfo))
extern char *symbol_demangled_name (const struct general_symbol_info *symbol);
@@ -265,10 +277,22 @@ extern char *symbol_demangled_name (const struct general_symbol_info *symbol);
(strcmp_iw (SYMBOL_NATURAL_NAME (symbol), (name)) == 0)
/* Macro that returns the name to be used when sorting and searching symbols.
- In C++, Chill, and Java, we search for the demangled form of a name,
- and so sort symbols accordingly. In Ada, however, we search by mangled
- name. If there is no distinct demangled name, then SYMBOL_SEARCH_NAME
- returns the same value (same pointer) as SYMBOL_LINKAGE_NAME. */
+ In C++, Chill, and Java, we search for the demangled form of a linkage name,
+ and so sort symbols accordingly:
+
+ For minimal symbols the demangled SYMBOL_NATURAL_NAME form is returned, no
+ typedefs nor toplevel const/volatile qualifiers are relevant for minimal
+ symbols, for `f(int_typedef const)' it returns `f(int)'.
+
+ For full symbols SYMBOL_LINKAGE_NAME is already the demangled form with
+ typedefs and toplevel const/volatile qualifiers removed. It is preferred
+ over SYMBOL_NATURAL_NAME which may contain the typedefs.
+ For `f(int_typedef const)' it also returns `f(int)'.
+
+ In Ada, however, we search by mangled name. If there is no distinct
+ demangled name, then SYMBOL_SEARCH_NAME returns the same value (same
+ pointer) as SYMBOL_LINKAGE_NAME. */
+
#define SYMBOL_SEARCH_NAME(symbol) \
(symbol_search_name (&(symbol)->ginfo))
extern char *symbol_search_name (const struct general_symbol_info *);