[PATCH 0/3] Improving "info types" command

[PATCH 0/3] Improving "info types" command

[Andrew Burgess]

A small set aimed at improving the "info types" command.

---

Andrew Burgess (3):
  gdb: Switch "info types" over to use the gdb::options framework
  gdb: Improve output from "info types" commad
  gdb: Show type summary for anonymous structures from c_print_typedef

 gdb/ChangeLog                            | 25 ++++++++
 gdb/NEWS                                 |  4 ++
 gdb/c-typeprint.c                        |  2 +-
 gdb/doc/ChangeLog                        |  5 ++
 gdb/doc/gdb.texinfo                      |  6 +-
 gdb/symtab.c                             | 98 ++++++++++++++++++++++++++------
 gdb/testsuite/ChangeLog                  | 13 +++++
 gdb/testsuite/gdb.ada/info_auto_lang.exp |  5 +-
 gdb/testsuite/gdb.base/info-types.c      | 72 +++++++++++++++++++++++
 gdb/testsuite/gdb.base/info-types.exp    | 53 +++++++++++++++++
 gdb/testsuite/gdb.cp/info-types.cc       | 33 +++++++++++
 gdb/testsuite/gdb.cp/info-types.exp      | 42 ++++++++++++++
 12 files changed, 334 insertions(+), 24 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/info-types.c
 create mode 100644 gdb/testsuite/gdb.base/info-types.exp
 create mode 100644 gdb/testsuite/gdb.cp/info-types.cc
 create mode 100644 gdb/testsuite/gdb.cp/info-types.exp

-- 
2.14.5

[PATCH 1/3] gdb: Switch "info types" over to use the gdb::options framework

[Andrew Burgess]

Adds a new -q flag to "info types" using the gdb::option framework.
This -q flag is similar to the -q flag already present for "info
variables" and "info functions".

gdb/ChangeLog:

	* NEWS: Mention adding -q option to "info types".
	* symtab.c (struct info_types_options): New struct.
	(info_types_options_defs): New variable.
	(make_info_types_options_def_group): New function.
	(info_types_command): Use gdb::option framework to parse options.
	(info_types_command_completer): New function.
	(_initialize_symtab): Extend the help text on "info types" and
	register command completer.

gdb/doc/ChangeLog:

	* gdb.texinfo (Symbols): Add information about -q flag to "info
	types".
---
 gdb/ChangeLog       | 11 +++++++++
 gdb/NEWS            |  4 ++++
 gdb/doc/ChangeLog   |  5 +++++
 gdb/doc/gdb.texinfo |  6 +++--
 gdb/symtab.c        | 64 +++++++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 4e479bf738b..cc1d58520d4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -211,6 +211,10 @@ maint show test-options-completion-result
 
     (gdb) print -raw -pretty -object off -- *myptr
 
+  ** The "info types" command now supports the '-q' flag to disable
+     printing of some header information in a similar fashion to "info
+     variables" and "info functions".
+
 * Completion improvements
 
   ** GDB can now complete the options of the "thread apply all" and
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index eddd939869a..777a412df67 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18428,8 +18428,7 @@
 of such variables.
 
 @kindex info types
-@item info types @var{regexp}
-@itemx info types
+@item info types [-q] [@var{regexp}]
 Print a brief description of all types whose names match the regular
 expression @var{regexp} (or all types in your program, if you supply
 no argument).  Each complete typename is matched as though it were a
@@ -18449,6 +18448,9 @@
 @code{whatis}, it does not print a detailed description; second, it
 lists all source files and line numbers where a type is defined.
 
+The optional flag @samp{-q}, which stands for @samp{quiet}, disables
+printing header information.
+
 @kindex info type-printers
 @item info type-printers
 Versions of @value{GDBN} that ship with Python scripting enabled may
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 41898992c19..7b7f1298419 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4769,11 +4769,62 @@ info_functions_command (const char *args, int from_tty)
 		      opts.type_regexp, from_tty);
 }
 
+/* Holds the -q option for the 'info types' command.  */
+
+struct info_types_options
+{
+  int quiet = false;
+};
+
+/* The options used by the 'info types' command.  */
+
+static const gdb::option::option_def info_types_options_defs[] = {
+  gdb::option::boolean_option_def<info_types_options> {
+    "q",
+    [] (info_types_options *opt) { return &opt->quiet; },
+    nullptr, /* show_cmd_cb */
+    nullptr /* set_doc */
+  }
+};
+
+/* Returns the option group used by 'info types'.  */
+
+static gdb::option::option_def_group
+make_info_types_options_def_group (info_types_options *opts)
+{
+  return {{info_types_options_defs}, opts};
+}
+
+/* Implement the 'info types' command.  */
 
 static void
-info_types_command (const char *regexp, int from_tty)
+info_types_command (const char *args, int from_tty)
 {
-  symtab_symbol_info (false, regexp, TYPES_DOMAIN, NULL, from_tty);
+  info_types_options opts;
+
+  auto grp = make_info_types_options_def_group (&opts);
+  gdb::option::process_options
+    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp);
+  if (args != nullptr && *args == '\0')
+    args = nullptr;
+  symtab_symbol_info (opts.quiet, args, TYPES_DOMAIN, NULL, from_tty);
+}
+
+/* Command completer for 'info types' command.  */
+
+static void
+info_types_command_completer (struct cmd_list_element *ignore,
+			      completion_tracker &tracker,
+			      const char *text, const char * /* word */)
+{
+  const auto group
+    = make_info_types_options_def_group (nullptr);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+    return;
+
+  const char *word = advance_to_expression_complete_word_point (tracker, text);
+  symbol_completer (ignore, tracker, text, word);
 }
 
 /* Breakpoint all functions matching regular expression.  */
@@ -6035,8 +6086,13 @@ Prints the functions.\n"),
      print "struct foo *".
      I also think "ptype" or "whatis" is more likely to be useful (but if
      there is much disagreement "info types" can be fixed).  */
-  add_info ("types", info_types_command,
-	    _("All type names, or those matching REGEXP."));
+  c = add_info ("types", info_types_command, _("\
+All type names, or those matching REGEXP.\n\
+Usage: info types [-q] [REGEXP]\n\
+Print information about all types matching REGEXP, or all types if no\n\
+REGEXP is given.  The optional flag -q disables printing of some headers\n\
+and messages."));
+  set_cmd_completer_handle_brkchars (c, info_types_command_completer);
 
   add_info ("sources", info_sources_command,
 	    _("Source files in the program."));
-- 
2.14.5

Re: [PATCH 1/3] gdb: Switch "info types" over to use the gdb::options framework

[Eli Zaretskii]

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Fri, 12 Jul 2019 12:37:04 +0100
> 
> Adds a new -q flag to "info types" using the gdb::option framework.
> This -q flag is similar to the -q flag already present for "info
> variables" and "info functions".

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 4e479bf738b..cc1d58520d4 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -211,6 +211,10 @@ maint show test-options-completion-result
>  
>      (gdb) print -raw -pretty -object off -- *myptr
>  
> +  ** The "info types" command now supports the '-q' flag to disable
> +     printing of some header information in a similar fashion to "info
> +     variables" and "info functions".
> +

This part is OK.

> +@item info types [-q] [@var{regexp}]
>  Print a brief description of all types whose names match the regular
>  expression @var{regexp} (or all types in your program, if you supply
>  no argument).  Each complete typename is matched as though it were a
> @@ -18449,6 +18448,9 @@
>  @code{whatis}, it does not print a detailed description; second, it
>  lists all source files and line numbers where a type is defined.
>  
> +The optional flag @samp{-q}, which stands for @samp{quiet}, disables
> +printing header information.

I don't see "headers" described anywhere in the preceding text, so
"disables printing header information" here is not clear enough.
Besides, the command's doc string says "some headers and messages",
which seems to imply more than just "headers" is suppressed by this
switch.

Can you please make the effect of this switch more clear?

Thanks.

Re: [PATCH 1/3] gdb: Switch "info types" over to use the gdb::options framework

[Andrew Burgess]

* Eli Zaretskii <eliz@gnu.org> [2019-07-12 15:48:38 +0300]:

> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> > Date: Fri, 12 Jul 2019 12:37:04 +0100
> > 
> > Adds a new -q flag to "info types" using the gdb::option framework.
> > This -q flag is similar to the -q flag already present for "info
> > variables" and "info functions".
> 
> Thanks.
> 
> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index 4e479bf738b..cc1d58520d4 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -211,6 +211,10 @@ maint show test-options-completion-result
> >  
> >      (gdb) print -raw -pretty -object off -- *myptr
> >  
> > +  ** The "info types" command now supports the '-q' flag to disable
> > +     printing of some header information in a similar fashion to "info
> > +     variables" and "info functions".
> > +
> 
> This part is OK.
> 
> > +@item info types [-q] [@var{regexp}]
> >  Print a brief description of all types whose names match the regular
> >  expression @var{regexp} (or all types in your program, if you supply
> >  no argument).  Each complete typename is matched as though it were a
> > @@ -18449,6 +18448,9 @@
> >  @code{whatis}, it does not print a detailed description; second, it
> >  lists all source files and line numbers where a type is defined.
> >  
> > +The optional flag @samp{-q}, which stands for @samp{quiet}, disables
> > +printing header information.
> 
> I don't see "headers" described anywhere in the preceding text, so
> "disables printing header information" here is not clear enough.
> Besides, the command's doc string says "some headers and messages",
> which seems to imply more than just "headers" is suppressed by this
> switch.
> 
> Can you please make the effect of this switch more clear?

Thanks for your feedback.

Part of the problem here is that many of the commands in this area are
similarly poorly documented, and I more or less copied the description
over.

I've simplified the command help text to actually reflect reality
(only headers are suppressed) and extended the manual text to mention
the headers before describing that the -q flag disables them.

Hopefully this all makes more sense.

Thanks,
Andrew

---

    gdb: Switch "info types" over to use the gdb::options framework
    
    Adds a new -q flag to "info types" using the gdb::option framework.
    This -q flag is similar to the -q flag already present for "info
    variables" and "info functions".
    
    gdb/ChangeLog:
    
            * NEWS: Mention adding -q option to "info types".
            * symtab.c (struct info_types_options): New struct.
            (info_types_options_defs): New variable.
            (make_info_types_options_def_group): New function.
            (info_types_command): Use gdb::option framework to parse options.
            (info_types_command_completer): New function.
            (_initialize_symtab): Extend the help text on "info types" and
            register command completer.
    
    gdb/doc/ChangeLog:
    
            * gdb.texinfo (Symbols): Add information about -q flag to "info
            types".

diff --git a/gdb/NEWS b/gdb/NEWS
index 4e479bf738b..cc1d58520d4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -211,6 +211,10 @@ maint show test-options-completion-result
 
     (gdb) print -raw -pretty -object off -- *myptr
 
+  ** The "info types" command now supports the '-q' flag to disable
+     printing of some header information in a similar fashion to "info
+     variables" and "info functions".
+
 * Completion improvements
 
   ** GDB can now complete the options of the "thread apply all" and
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index eddd939869a..be65d528d28 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18428,8 +18428,7 @@
 of such variables.
 
 @kindex info types
-@item info types @var{regexp}
-@itemx info types
+@item info types [-q] [@var{regexp}]
 Print a brief description of all types whose names match the regular
 expression @var{regexp} (or all types in your program, if you supply
 no argument).  Each complete typename is matched as though it were a
@@ -18449,6 +18448,11 @@
 @code{whatis}, it does not print a detailed description; second, it
 lists all source files and line numbers where a type is defined.
 
+The output from @samp{into types} is proceeded with a header line
+describing what types are being listed.  The optional flag @samp{-q},
+which stands for @samp{quiet}, disables printing this header
+information.
+
 @kindex info type-printers
 @item info type-printers
 Versions of @value{GDBN} that ship with Python scripting enabled may
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 719e5b2ee9a..16861e2bc92 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4769,11 +4769,62 @@ info_functions_command (const char *args, int from_tty)
 		      opts.type_regexp, from_tty);
 }
 
+/* Holds the -q option for the 'info types' command.  */
+
+struct info_types_options
+{
+  int quiet = false;
+};
+
+/* The options used by the 'info types' command.  */
+
+static const gdb::option::option_def info_types_options_defs[] = {
+  gdb::option::boolean_option_def<info_types_options> {
+    "q",
+    [] (info_types_options *opt) { return &opt->quiet; },
+    nullptr, /* show_cmd_cb */
+    nullptr /* set_doc */
+  }
+};
+
+/* Returns the option group used by 'info types'.  */
+
+static gdb::option::option_def_group
+make_info_types_options_def_group (info_types_options *opts)
+{
+  return {{info_types_options_defs}, opts};
+}
+
+/* Implement the 'info types' command.  */
 
 static void
-info_types_command (const char *regexp, int from_tty)
+info_types_command (const char *args, int from_tty)
 {
-  symtab_symbol_info (false, regexp, TYPES_DOMAIN, NULL, from_tty);
+  info_types_options opts;
+
+  auto grp = make_info_types_options_def_group (&opts);
+  gdb::option::process_options
+    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp);
+  if (args != nullptr && *args == '\0')
+    args = nullptr;
+  symtab_symbol_info (opts.quiet, args, TYPES_DOMAIN, NULL, from_tty);
+}
+
+/* Command completer for 'info types' command.  */
+
+static void
+info_types_command_completer (struct cmd_list_element *ignore,
+			      completion_tracker &tracker,
+			      const char *text, const char * /* word */)
+{
+  const auto group
+    = make_info_types_options_def_group (nullptr);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+    return;
+
+  const char *word = advance_to_expression_complete_word_point (tracker, text);
+  symbol_completer (ignore, tracker, text, word);
 }
 
 /* Breakpoint all functions matching regular expression.  */
@@ -6037,8 +6088,12 @@ Prints the functions.\n"),
      print "struct foo *".
      I also think "ptype" or "whatis" is more likely to be useful (but if
      there is much disagreement "info types" can be fixed).  */
-  add_info ("types", info_types_command,
-	    _("All type names, or those matching REGEXP."));
+  c = add_info ("types", info_types_command, _("\
+All type names, or those matching REGEXP.\n\
+Usage: info types [-q] [REGEXP]\n\
+Print information about all types matching REGEXP, or all types if no\n\
+REGEXP is given.  The optional flag -q disables printing of headers."));
+  set_cmd_completer_handle_brkchars (c, info_types_command_completer);
 
   add_info ("sources", info_sources_command,
 	    _("Source files in the program."));

Re: [PATCH 1/3] gdb: Switch "info types" over to use the gdb::options framework

[Pedro Alves]

On 7/12/19 12:37 PM, Andrew Burgess wrote:
> Adds a new -q flag to "info types" using the gdb::option framework.
> This -q flag is similar to the -q flag already present for "info
> variables" and "info functions".
> 
LGTM.

Thanks,
Pedro Alves

[PATCH 3/3] gdb: Show type summary for anonymous structures from c_print_typedef

[Andrew Burgess]

Currently each language has a la_print_typedef method, this is only
used for the "info types" command.

The documentation for "info types" says:

   Print a brief description of all types whose names match the regular
   expression @var{regexp} (or all types in your program, if you supply
   no argument).

However, if we consider this C code:

   typedef struct {
     int a;
   } my_type;

Then currently with "info types" this will be printed like this:

   3:      typedef struct {
       int a;
   } my_type;

I see two problems with this, first the indentation is clearly broken,
second, if the struct contained more fields then the it feels like the
actual type names could easily get lost in the noise.

Given that "info types" is about discovering type names, I think there
is an argument to be made that we should focus on giving _only_ the
briefest summary for "info types", and if the user wants to know more
they can take the type name and plug it into "ptype".  As such, I
propose that a better output would be:

   3:      typedef struct {...} my_type;

The user understands that there is a type called `my_type`, and that
it's an alias for an anonymous structure type.

The change to achieve this turns out to be pretty simple, but only
effects languages that make use of c_print_typedef, which are C, C++,
asm, minimal, d, go, objc, and opencl.  Other languages will for now
do whatever they used to do.

I did look at ada, as this is the only language to actually have some
tests for "info types", however, as I understand it ada doesn't really
support typedefs, however, by forcing the language we can see what ada
would print.  So, if we 'set language ada', then originally we printed
this:

   3:      record
       a: int;
   end record

Again the indentation is clearly broken, but we also have no mention
of the type name at all, which is odd, but understandable given the
lack of typedefs.  If I make a similar change as I'm proposing for C,
then we now get this output:

   3:      record ... end record

Which is even less informative I think.  However, the original output
_is_ tested for in gdb.ada/info_auto_lang.exp, and its not clear to me
if the change is a good one or not, so for now I have left this out.

gdb/ChangeLog:

	* c-typeprint.c (c_print_typedef): Pass -1 instead of 0 to
	type_print.

gdb/testsuite/ChangeLog:

	* gdb.ada/info_auto_lang.exp: Update expected results.
	* gdb.base/info-types.c: Add anonymous struct typedef.
	* gdb.base/info-types.exp: Update expected results.
---
 gdb/ChangeLog                            |  5 +++++
 gdb/c-typeprint.c                        |  2 +-
 gdb/testsuite/ChangeLog                  |  6 ++++++
 gdb/testsuite/gdb.ada/info_auto_lang.exp |  5 +----
 gdb/testsuite/gdb.base/info-types.c      | 10 ++++++++++
 gdb/testsuite/gdb.base/info-types.exp    |  2 ++
 6 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 6690ca53bcd..43ad3b3e0e6 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -205,7 +205,7 @@ c_print_typedef (struct type *type,
 {
   type = check_typedef (type);
   fprintf_filtered (stream, "typedef ");
-  type_print (type, "", stream, 0);
+  type_print (type, "", stream, -1);
   if (TYPE_NAME ((SYMBOL_TYPE (new_symbol))) == 0
       || strcmp (TYPE_NAME ((SYMBOL_TYPE (new_symbol))),
 		 SYMBOL_LINKAGE_NAME (new_symbol)) != 0
diff --git a/gdb/testsuite/gdb.ada/info_auto_lang.exp b/gdb/testsuite/gdb.ada/info_auto_lang.exp
index be1deae99ef..68457827d2f 100644
--- a/gdb/testsuite/gdb.ada/info_auto_lang.exp
+++ b/gdb/testsuite/gdb.ada/info_auto_lang.exp
@@ -53,10 +53,7 @@ set func_in_c(ada_syntax)    "${decimal}:	procedure proc_in_c;"
 set func_in_ada(c_syntax)    "${decimal}:	void proc_in_ada\\\(void\\\);"
 set func_in_ada(ada_syntax)  "${decimal}:	procedure proc_in_ada;"
 
-set type_in_c(c_syntax) [multi_line \
-			    "${decimal}:	typedef struct {" \
-			    "    int some_component_in_c;" \
-			    "} some_type_in_c;" ]
+set type_in_c(c_syntax) "${decimal}:	typedef struct {\\.\\.\\.} some_type_in_c;"
 set type_in_c(ada_syntax) [multi_line \
 			      "${decimal}:	record" \
 			      "    some_component_in_c: int;" \
diff --git a/gdb/testsuite/gdb.base/info-types.c b/gdb/testsuite/gdb.base/info-types.c
index d07866544b6..94d1f6c9938 100644
--- a/gdb/testsuite/gdb.base/info-types.c
+++ b/gdb/testsuite/gdb.base/info-types.c
@@ -38,6 +38,14 @@ enum enum_t
 typedef enum enum_t my_enum_t;
 typedef my_enum_t nested_enum_t;
 
+typedef struct
+{
+  double d;
+  float f;
+} anon_struct_t;
+
+typedef anon_struct_t nested_anon_struct_t;
+
 volatile int var_a;
 volatile float var_b;
 volatile my_int_t var_c;
@@ -53,6 +61,8 @@ volatile baz_ptr var_l;
 volatile enum enum_t var_m;
 volatile my_enum_t var_n;
 volatile nested_enum_t var_o;
+volatile anon_struct_t var_p;
+volatile nested_anon_struct_t var_q;
 
 int
 main ()
diff --git a/gdb/testsuite/gdb.base/info-types.exp b/gdb/testsuite/gdb.base/info-types.exp
index 2ebd76f0e94..781f8988f13 100644
--- a/gdb/testsuite/gdb.base/info-types.exp
+++ b/gdb/testsuite/gdb.base/info-types.exp
@@ -32,6 +32,7 @@ gdb_test "info types" \
 	 "All defined types:" \
 	 "" \
 	 "File .*:" \
+	 "45:\[\t \]+typedef struct {\\.\\.\\.} anon_struct_t;" \
 	 "28:\[\t \]+typedef struct baz_t baz;" \
 	 "31:\[\t \]+typedef struct baz_t \\* baz_ptr;" \
 	 "21:\[\t \]+struct baz_t;" \
@@ -42,6 +43,7 @@ gdb_test "info types" \
 	 "38:\[\t \]+typedef enum enum_t my_enum_t;" \
 	 "17:\[\t \]+typedef float my_float_t;" \
 	 "16:\[\t \]+typedef int my_int_t;" \
+	 "47:\[\t \]+typedef struct {\\.\\.\\.} nested_anon_struct_t;" \
 	 "30:\[\t \]+typedef struct baz_t nested_baz;" \
 	 "29:\[\t \]+typedef struct baz_t nested_baz_t;" \
 	 "39:\[\t \]+typedef enum enum_t nested_enum_t;" \
-- 
2.14.5

Re: [PATCH 3/3] gdb: Show type summary for anonymous structures from c_print_typedef

[Pedro Alves]

On 7/12/19 12:37 PM, Andrew Burgess wrote:
> Currently each language has a la_print_typedef method, this is only
> used for the "info types" command.
> 
> The documentation for "info types" says:
> 
>    Print a brief description of all types whose names match the regular
>    expression @var{regexp} (or all types in your program, if you supply
>    no argument).
> 
> However, if we consider this C code:
> 
>    typedef struct {
>      int a;
>    } my_type;
> 
> Then currently with "info types" this will be printed like this:
> 
>    3:      typedef struct {
>        int a;
>    } my_type;
> 
> I see two problems with this, first the indentation is clearly broken,
> second, if the struct contained more fields then the it feels like the
> actual type names could easily get lost in the noise.

Something odd in "then the it feels"?

> 
> Given that "info types" is about discovering type names, I think there
> is an argument to be made that we should focus on giving _only_ the
> briefest summary for "info types", and if the user wants to know more
> they can take the type name and plug it into "ptype".  As such, I
> propose that a better output would be:
> 
>    3:      typedef struct {...} my_type;
> 

I think the same rationale applies to enums too?  We currently
print anonymous enums like:

 16:     typedef enum {A, B, C} EEE;

The difference here is that we don't print newline between
each enumerator.

It's as if we printed your struct example like this:

 3:     typedef struct {int a;} my_type;

which would be a bit more reasonable than the current output.

I did the 0 -> -1 change locally, and your patch addresses
enums as well already:

 16:     typedef enum {...} EEE;

But I think you should add that to the testcase.

> The user understands that there is a type called `my_type`, and that
> it's an alias for an anonymous structure type.

It's worth explicitly showing (in the commit log, IMO) that this
only affects anonymous structs/enums.  For named structs/enums, we do
print an abbreviated form with no fields/enumerators:

 11:     struct named_struct;
 18:     enum named_enum;

So it makes sense to me to be consistent and use an abbreviated
form for anonymous types too, like in your patch.

Thanks,
Pedro Alves

Re: [PATCH 3/3] gdb: Show type summary for anonymous structures from c_print_typedef

[Andrew Burgess]

* Pedro Alves <palves@redhat.com> [2019-07-19 13:38:35 +0100]:

> On 7/12/19 12:37 PM, Andrew Burgess wrote:
> > Currently each language has a la_print_typedef method, this is only
> > used for the "info types" command.
> > 
> > The documentation for "info types" says:
> > 
> >    Print a brief description of all types whose names match the regular
> >    expression @var{regexp} (or all types in your program, if you supply
> >    no argument).
> > 
> > However, if we consider this C code:
> > 
> >    typedef struct {
> >      int a;
> >    } my_type;
> > 
> > Then currently with "info types" this will be printed like this:
> > 
> >    3:      typedef struct {
> >        int a;
> >    } my_type;
> > 
> > I see two problems with this, first the indentation is clearly broken,
> > second, if the struct contained more fields then the it feels like the
> > actual type names could easily get lost in the noise.
> 
> Something odd in "then the it feels"?
> 
> > 
> > Given that "info types" is about discovering type names, I think there
> > is an argument to be made that we should focus on giving _only_ the
> > briefest summary for "info types", and if the user wants to know more
> > they can take the type name and plug it into "ptype".  As such, I
> > propose that a better output would be:
> > 
> >    3:      typedef struct {...} my_type;
> > 
> 
> I think the same rationale applies to enums too?  We currently
> print anonymous enums like:
> 
>  16:     typedef enum {A, B, C} EEE;
> 
> The difference here is that we don't print newline between
> each enumerator.
> 
> It's as if we printed your struct example like this:
> 
>  3:     typedef struct {int a;} my_type;
> 
> which would be a bit more reasonable than the current output.
> 
> I did the 0 -> -1 change locally, and your patch addresses
> enums as well already:
> 
>  16:     typedef enum {...} EEE;
> 
> But I think you should add that to the testcase.
> 
> > The user understands that there is a type called `my_type`, and that
> > it's an alias for an anonymous structure type.
> 
> It's worth explicitly showing (in the commit log, IMO) that this
> only affects anonymous structs/enums.  For named structs/enums, we do
> print an abbreviated form with no fields/enumerators:
> 
>  11:     struct named_struct;
>  18:     enum named_enum;
> 
> So it makes sense to me to be consistent and use an abbreviated
> form for anonymous types too, like in your patch.

I've extended the test case to cover anonymous enums, and also unions
that I'd completely forgotten to cover at all before (though they are
mostly just handled like structs, so no surprises there).

I've extended the commit message to talk about the difference between
named and anonymous structs/enums, to mention that unions are like
structs, and also to mention the differences between C and C++ (with
C++ being less changes by this patch as its output before was more
compact anyway).

Thanks,
Andrew

--

    gdb: Show type summary for anonymous structures from c_print_typedef
    
    Currently each language has a la_print_typedef method, this is only
    used for the "info types" command.
    
    The documentation for "info types" says:
    
       Print a brief description of all types whose names match the regular
       expression @var{regexp} (or all types in your program, if you supply
       no argument).
    
    However, if we consider this C code:
    
       typedef struct {
         int a;
       } my_type;
    
    Then currently with "info types" this will be printed like this:
    
       3:      typedef struct {
           int a;
       } my_type;
    
    I see two problems with this, first the indentation is clearly broken,
    second, if the struct contained more fields then it feels like the
    actual type names could easily get lost in the noise.
    
    Given that "info types" is about discovering type names, I think there
    is an argument to be made that we should focus on giving _only_ the
    briefest summary for "info types", and if the user wants to know more
    they can take the type name and plug it into "ptype".  As such, I
    propose that a better output would be:
    
       3:      typedef struct {...} my_type;
    
    The user understands that there is a type called `my_type`, and that
    it's an alias for an anonymous structure type.
    
    The change to achieve this turns out to be pretty simple, but only
    effects languages that make use of c_print_typedef, which are C, C++,
    asm, minimal, d, go, objc, and opencl.  Other languages will for now
    do whatever they used to do.
    
    The patch to change how anonymous structs are displayed also changes
    the display of anonymous enums, consider this code sample:
    
       typedef enum {
         AA, BB, CC
       } anon_enum_t;
    
    This used to be displayed like this:
    
       3:      typedef enum {AA, BB, CC} anon_enum_t;
    
    Which will quickly become cluttered for enums with a large number of
    values.  The modified output looks like this:
    
       3:      typedef enum {...} anon_enum_t;
    
    Again, the user can always make use of ptype if they want to see the
    details of the anon_enum_t type.
    
    It is worth pointing out that this change (to use {...}) only effects
    anonymous structs and enums, named types don't change with this patch,
    consider this code:
    
       struct struct_t {
         int i;
       };
       enum enum_t {
        AA, BB, CC
       };
    
    The output from 'info types' remains unchanged, like this:
    
       4:      enum enum_t;
       1:      struct struct_t;
    
    An additional area of interest is how C++ handles anonymous types used
    within a typedef; enums are handled basically inline with how C
    handles them, but structs (and classes) are slightly different.  The
    behaviour before the patch is different, and is unchanged by this
    patch.  Consider this code compiled for C++:
    
       typedef struct {
         int i;
       } struct_t;
    
    Both before and after this patch, this is show by 'info types' as:
    
       3:      typedef struct_t struct_t;
    
    Unions are displayed similarly to structs in both C and C++, the
    handling of anonymous unions changes for C in the same way that
    it changes for anonymous structs.
    
    I did look at ada, as this is the only language to actually have some
    tests for "info types", however, as I understand it ada doesn't really
    support typedefs, however, by forcing the language we can see what ada
    would print.  So, if we 'set language ada', then originally we printed
    this:
    
       3:      record
           a: int;
       end record
    
    Again the indentation is clearly broken, but we also have no mention
    of the type name at all, which is odd, but understandable given the
    lack of typedefs.  If I make a similar change as I'm proposing for C,
    then we now get this output:
    
       3:      record ... end record
    
    Which is even less informative I think.  However, the original output
    _is_ tested for in gdb.ada/info_auto_lang.exp, and its not clear to me
    if the change is a good one or not, so for now I have left this out.
    
    gdb/ChangeLog:
    
            * c-typeprint.c (c_print_typedef): Pass -1 instead of 0 to
            type_print.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.ada/info_auto_lang.exp: Update expected results.
            * gdb.base/info-types.c: Add additional types to check.
            * gdb.base/info-types.exp: Update expected results.

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 6690ca53bcd..43ad3b3e0e6 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -205,7 +205,7 @@ c_print_typedef (struct type *type,
 {
   type = check_typedef (type);
   fprintf_filtered (stream, "typedef ");
-  type_print (type, "", stream, 0);
+  type_print (type, "", stream, -1);
   if (TYPE_NAME ((SYMBOL_TYPE (new_symbol))) == 0
       || strcmp (TYPE_NAME ((SYMBOL_TYPE (new_symbol))),
 		 SYMBOL_LINKAGE_NAME (new_symbol)) != 0
diff --git a/gdb/testsuite/gdb.ada/info_auto_lang.exp b/gdb/testsuite/gdb.ada/info_auto_lang.exp
index be1deae99ef..68457827d2f 100644
--- a/gdb/testsuite/gdb.ada/info_auto_lang.exp
+++ b/gdb/testsuite/gdb.ada/info_auto_lang.exp
@@ -53,10 +53,7 @@ set func_in_c(ada_syntax)    "${decimal}:	procedure proc_in_c;"
 set func_in_ada(c_syntax)    "${decimal}:	void proc_in_ada\\\(void\\\);"
 set func_in_ada(ada_syntax)  "${decimal}:	procedure proc_in_ada;"
 
-set type_in_c(c_syntax) [multi_line \
-			    "${decimal}:	typedef struct {" \
-			    "    int some_component_in_c;" \
-			    "} some_type_in_c;" ]
+set type_in_c(c_syntax) "${decimal}:	typedef struct {\\.\\.\\.} some_type_in_c;"
 set type_in_c(ada_syntax) [multi_line \
 			      "${decimal}:	record" \
 			      "    some_component_in_c: int;" \
diff --git a/gdb/testsuite/gdb.base/info-types.c b/gdb/testsuite/gdb.base/info-types.c
index 6746238801d..de9320da888 100644
--- a/gdb/testsuite/gdb.base/info-types.c
+++ b/gdb/testsuite/gdb.base/info-types.c
@@ -38,6 +38,37 @@ enum enum_t
 typedef enum enum_t my_enum_t;
 typedef my_enum_t nested_enum_t;
 
+typedef struct
+{
+  double d;
+  float f;
+} anon_struct_t;
+
+typedef anon_struct_t nested_anon_struct_t;
+
+typedef enum
+{
+ DD, EE, FF
+} anon_enum_t;
+
+typedef anon_enum_t nested_anon_enum_t;
+
+union union_t
+{
+  int i;
+  float f;
+};
+
+typedef union union_t nested_union_t;
+
+typedef union
+{
+  int i;
+  double d;
+} anon_union_t;
+
+typedef anon_union_t nested_anon_union_t;
+
 volatile int var_a;
 volatile float var_b;
 volatile my_int_t var_c;
@@ -53,6 +84,14 @@ volatile baz_ptr var_l;
 volatile enum enum_t var_m;
 volatile my_enum_t var_n;
 volatile nested_enum_t var_o;
+volatile anon_struct_t var_p;
+volatile nested_anon_struct_t var_q;
+volatile anon_enum_t var_r;
+volatile nested_anon_enum_t var_s;
+volatile union union_t var_t;
+volatile nested_union_t var_u;
+volatile anon_union_t var_v;
+volatile nested_anon_union_t var_w;
 
 #ifdef __cplusplus
 
diff --git a/gdb/testsuite/gdb.base/info-types.exp b/gdb/testsuite/gdb.base/info-types.exp
index a5808425398..3a514b5bc19 100644
--- a/gdb/testsuite/gdb.base/info-types.exp
+++ b/gdb/testsuite/gdb.base/info-types.exp
@@ -57,25 +57,35 @@ proc run_test { lang } {
 		 "All defined types:" \
 		 "" \
 		 "File .*:" \
-		 "59:\[\t \]+CL;" \
+		 "98:\[\t \]+CL;" \
+		 "42:\[\t \]+anon_struct_t;" \
+		 "65:\[\t \]+anon_union_t;" \
 		 "21:\[\t \]+baz_t;" \
 		 "33:\[\t \]+enum_t;" \
+		 "56:\[\t \]+union_t;" \
+		 "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
+		 "45:\[\t \]+typedef anon_struct_t anon_struct_t;" \
+		 "68:\[\t \]+typedef anon_union_t anon_union_t;" \
 		 "28:\[\t \]+typedef baz_t baz;" \
 		 "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
 		 "27:\[\t \]+typedef baz_t baz_t;" \
 		 "\[\t \]+double" \
 		 "\[\t \]+float" \
 		 "\[\t \]+int" \
-		 "64:\[\t \]+typedef CL my_cl;" \
+		 "103:\[\t \]+typedef CL my_cl;" \
 		 "38:\[\t \]+typedef enum_t my_enum_t;" \
 		 "17:\[\t \]+typedef float my_float_t;" \
 		 "16:\[\t \]+typedef int my_int_t;" \
-		 "65:\[\t \]+typedef CL \\* my_ptr;" \
+		 "104:\[\t \]+typedef CL \\* my_ptr;" \
+		 "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
+		 "47:\[\t \]+typedef anon_struct_t nested_anon_struct_t;" \
+		 "70:\[\t \]+typedef anon_union_t nested_anon_union_t;" \
 		 "30:\[\t \]+typedef baz_t nested_baz;" \
 		 "29:\[\t \]+typedef baz_t nested_baz_t;" \
 		 "39:\[\t \]+typedef enum_t nested_enum_t;" \
 		 "19:\[\t \]+typedef float nested_float_t;" \
 		 "18:\[\t \]+typedef int nested_int_t;" \
+		 "62:\[\t \]+typedef union_t nested_union_t;" \
 		 "\[\t \]+unsigned int"]
     } else {
 	set output_re \
@@ -83,6 +93,9 @@ proc run_test { lang } {
 		 "All defined types:" \
 		 "" \
 		 "File .*:" \
+		 "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
+		 "45:\[\t \]+typedef struct {\\.\\.\\.} anon_struct_t;" \
+		 "68:\[\t \]+typedef union {\\.\\.\\.} anon_union_t;" \
 		 "28:\[\t \]+typedef struct baz_t baz;" \
 		 "31:\[\t \]+typedef struct baz_t \\* baz_ptr;" \
 		 "21:\[\t \]+struct baz_t;" \
@@ -93,11 +106,16 @@ proc run_test { lang } {
 		 "38:\[\t \]+typedef enum enum_t my_enum_t;" \
 		 "17:\[\t \]+typedef float my_float_t;" \
 		 "16:\[\t \]+typedef int my_int_t;" \
+		 "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
+		 "47:\[\t \]+typedef struct {\\.\\.\\.} nested_anon_struct_t;" \
+		 "70:\[\t \]+typedef union {\\.\\.\\.} nested_anon_union_t;" \
 		 "30:\[\t \]+typedef struct baz_t nested_baz;" \
 		 "29:\[\t \]+typedef struct baz_t nested_baz_t;" \
 		 "39:\[\t \]+typedef enum enum_t nested_enum_t;" \
 		 "19:\[\t \]+typedef float nested_float_t;" \
 		 "18:\[\t \]+typedef int nested_int_t;" \
+		 "62:\[\t \]+typedef union union_t nested_union_t;" \
+		 "56:\[\t \]+union union_t;" \
 		 "\[\t \]+unsigned int" ]
     }
 

Re: [PATCH 3/3] gdb: Show type summary for anonymous structures from c_print_typedef

[Pedro Alves]

On 7/19/19 10:06 PM, Andrew Burgess wrote:

>     An additional area of interest is how C++ handles anonymous types used
>     within a typedef; enums are handled basically inline with how C
>     handles them, but structs (and classes) are slightly different.  The
>     behaviour before the patch is different, and is unchanged by this
>     patch.  Consider this code compiled for C++:
>     
>        typedef struct {
>          int i;
>        } struct_t;
>     
>     Both before and after this patch, this is show by 'info types' as:
>     
>        3:      typedef struct_t struct_t;

Curious, and odd.

>     gdb/ChangeLog:
>     
>             * c-typeprint.c (c_print_typedef): Pass -1 instead of 0 to
>             type_print.
>     
>     gdb/testsuite/ChangeLog:
>     
>             * gdb.ada/info_auto_lang.exp: Update expected results.
>             * gdb.base/info-types.c: Add additional types to check.
>             * gdb.base/info-types.exp: Update expected results.

LGTM.

Thanks,
Pedro Alves

Re: [PATCH 3/3] gdb: Show type summary for anonymous structures from c_print_typedef

[Tom Tromey]

>> typedef struct {
>> int i;
>> } struct_t;
>> 
>> Both before and after this patch, this is show by 'info types' as:
>> 
>> 3:      typedef struct_t struct_t;

Pedro> Curious, and odd.

There's a weird special case in C++ where this sort of typedef provides
the linkage name for an otherwise anonymous structure, and ages ago we
needed to work around this.

See

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48603#c5

In the DWARF this looks like:

 <1><1d>: Abbrev Number: 2 (DW_TAG_structure_type)
    <1e>   DW_AT_byte_size   : 4
    <1f>   DW_AT_decl_file   : 1
    <20>   DW_AT_decl_line   : 1
    <21>   DW_AT_decl_column : 16
    <22>   DW_AT_linkage_name: (indirect string, offset: 0x0): 8struct_t
    <26>   DW_AT_sibling     : <0x36>

... in particular the linkage name is picked up by gdb.

Search for "47510" in dwar2read.c.  There are a few spots.

Tom

[PATCH 2/3] gdb: Improve output from "info types" commad

[Andrew Burgess]

This commit makes two changes to the "info types" command:

First, only use typedef_print for printing typedefs, and use
type_print for printing non-typedef scalar (non-struct) types.  The
result of this is the output for builtin types goes from this:

    typedef double;
    typedef float;
    typedef int;

to this:

    double;
    float;
    int;

which seems to make more sense.

Next GDB no longer matches msymbols as possible type names.  When
looking for function symbols it makes sense to report matching
msymbols from the text sections, and for variables msymbols from the
data/bss sections, but when reporting types GDB would match msymbols
of type absolute.  But I don't see why these are likely to indicate
type names.  As such I've updated the msymbol matching lists in
symtab.c:search_symbols so that when searching in the TYPES_DOMAIN, we
never match any msymbols.

gdb/ChangeLog:

	* symtab.c (search_symbols): Adjust msymbol matching type arrays
	so that GDB doesn't match any msymbols when searching in the
	TYPES_DOMAIN.
	(print_symbol_info): Print using typedef_print or type_print based
	on the type of the symbol.  Add updated FIXME comment mobed from...
	(_initialize_symtab): ... move and update FIXME comment to above.

gdb/testsuite/ChangeLog:

	* gdb.base/info-types.c: New file.
	* gdb.base/info-types.exp: New file.
	* gdb.cp/info-types.cc: New file.
	* gdb.cp/info-types.exp: New file.
---
 gdb/ChangeLog                         |  9 +++++
 gdb/symtab.c                          | 34 +++++++++++--------
 gdb/testsuite/ChangeLog               |  7 ++++
 gdb/testsuite/gdb.base/info-types.c   | 62 +++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/info-types.exp | 51 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/info-types.cc    | 33 +++++++++++++++++++
 gdb/testsuite/gdb.cp/info-types.exp   | 42 ++++++++++++++++++++++++
 7 files changed, 225 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/info-types.c
 create mode 100644 gdb/testsuite/gdb.base/info-types.exp
 create mode 100644 gdb/testsuite/gdb.cp/info-types.cc
 create mode 100644 gdb/testsuite/gdb.cp/info-types.exp

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 7b7f1298419..6364605fc60 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4339,13 +4339,13 @@ search_symbols (const char *regexp, enum search_domain kind,
   struct symbol *sym;
   int found_misc = 0;
   static const enum minimal_symbol_type types[]
-    = {mst_data, mst_text, mst_abs};
+    = {mst_data, mst_text, mst_unknown};
   static const enum minimal_symbol_type types2[]
-    = {mst_bss, mst_file_text, mst_abs};
+    = {mst_bss, mst_file_text, mst_unknown};
   static const enum minimal_symbol_type types3[]
-    = {mst_file_data, mst_solib_trampoline, mst_abs};
+    = {mst_file_data, mst_solib_trampoline, mst_unknown};
   static const enum minimal_symbol_type types4[]
-    = {mst_file_bss, mst_text_gnu_ifunc, mst_abs};
+    = {mst_file_bss, mst_text_gnu_ifunc, mst_unknown};
   enum minimal_symbol_type ourtype;
   enum minimal_symbol_type ourtype2;
   enum minimal_symbol_type ourtype3;
@@ -4628,7 +4628,23 @@ print_symbol_info (enum search_domain kind,
   /* Typedef that is not a C++ class.  */
   if (kind == TYPES_DOMAIN
       && SYMBOL_DOMAIN (sym) != STRUCT_DOMAIN)
-    typedef_print (SYMBOL_TYPE (sym), sym, gdb_stdout);
+    {
+      /* FIXME: For C (and C++) we end up with a difference in output here
+	 between how a typedef is printed, and non-typedefs are printed.
+	 The TYPEDEF_PRINT code places a ";" at the end in an attempt to
+	 appear C-like, while TYPE_PRINT doesn't.
+
+	 For the struct printing case below, things are worse, we force
+	 printing of the ";" in this function, which is going to be wrong
+	 for languages that don't require a ";" between statements.  */
+      if (TYPE_CODE (SYMBOL_TYPE (sym)) == TYPE_CODE_TYPEDEF)
+	typedef_print (SYMBOL_TYPE (sym), sym, gdb_stdout);
+      else
+	{
+	  type_print (SYMBOL_TYPE (sym), "", gdb_stdout, -1);
+	  printf_filtered ("\n");
+	}
+    }
   /* variable, func, or typedef-that-is-c++-class.  */
   else if (kind < TYPES_DOMAIN
 	   || (kind == TYPES_DOMAIN
@@ -6078,14 +6094,6 @@ Prints the functions.\n"),
 				  _("functions")));
   set_cmd_completer_handle_brkchars (c, info_print_command_completer);
 
-  /* FIXME:  This command has at least the following problems:
-     1.  It prints builtin types (in a very strange and confusing fashion).
-     2.  It doesn't print right, e.g. with
-     typedef struct foo *FOO
-     type_print prints "FOO" when we want to make it (in this situation)
-     print "struct foo *".
-     I also think "ptype" or "whatis" is more likely to be useful (but if
-     there is much disagreement "info types" can be fixed).  */
   c = add_info ("types", info_types_command, _("\
 All type names, or those matching REGEXP.\n\
 Usage: info types [-q] [REGEXP]\n\
diff --git a/gdb/testsuite/gdb.base/info-types.c b/gdb/testsuite/gdb.base/info-types.c
new file mode 100644
index 00000000000..d07866544b6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-types.c
@@ -0,0 +1,62 @@
+/* Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+typedef int my_int_t;
+typedef float my_float_t;
+typedef my_int_t nested_int_t;
+typedef my_float_t nested_float_t;
+
+struct baz_t
+{
+  float f;
+  double d;
+};
+
+typedef struct baz_t baz_t;
+typedef struct baz_t baz;
+typedef baz_t nested_baz_t;
+typedef baz nested_baz;
+typedef struct baz_t *baz_ptr;
+
+enum enum_t
+{
+ AA, BB, CC
+};
+
+typedef enum enum_t my_enum_t;
+typedef my_enum_t nested_enum_t;
+
+volatile int var_a;
+volatile float var_b;
+volatile my_int_t var_c;
+volatile my_float_t var_d;
+volatile nested_int_t var_e;
+volatile nested_float_t var_f;
+volatile struct baz_t var_g;
+volatile baz_t var_h;
+volatile baz var_i;
+volatile nested_baz_t var_j;
+volatile nested_baz var_k;
+volatile baz_ptr var_l;
+volatile enum enum_t var_m;
+volatile my_enum_t var_n;
+volatile nested_enum_t var_o;
+
+int
+main ()
+{
+  asm ("" ::: "memory");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/info-types.exp b/gdb/testsuite/gdb.base/info-types.exp
new file mode 100644
index 00000000000..2ebd76f0e94
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-types.exp
@@ -0,0 +1,51 @@
+# Copyright 2019 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that 'info types' produces the expected output for an inferior
+# containing a number of different types.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+gdb_test "info types" \
+    [multi_line \
+	 "All defined types:" \
+	 "" \
+	 "File .*:" \
+	 "28:\[\t \]+typedef struct baz_t baz;" \
+	 "31:\[\t \]+typedef struct baz_t \\* baz_ptr;" \
+	 "21:\[\t \]+struct baz_t;" \
+	 "\[\t \]+double" \
+	 "33:\[\t \]+enum enum_t;" \
+	 "\[\t \]+float" \
+	 "\[\t \]+int" \
+	 "38:\[\t \]+typedef enum enum_t my_enum_t;" \
+	 "17:\[\t \]+typedef float my_float_t;" \
+	 "16:\[\t \]+typedef int my_int_t;" \
+	 "30:\[\t \]+typedef struct baz_t nested_baz;" \
+	 "29:\[\t \]+typedef struct baz_t nested_baz_t;" \
+	 "39:\[\t \]+typedef enum enum_t nested_enum_t;" \
+	 "19:\[\t \]+typedef float nested_float_t;" \
+	 "18:\[\t \]+typedef int nested_int_t;" \
+	 "\[\t \]+unsigned int" ]
+
diff --git a/gdb/testsuite/gdb.cp/info-types.cc b/gdb/testsuite/gdb.cp/info-types.cc
new file mode 100644
index 00000000000..f3ee67a5f52
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/info-types.cc
@@ -0,0 +1,33 @@
+/* Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+class AA
+{
+  int a;
+};
+
+typedef AA my_a;
+typedef AA *my_ptr;
+
+volatile AA var_a;
+volatile my_a var_b;
+volatile my_ptr var_c;
+
+int
+main ()
+{
+  asm ("" ::: "memory");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/info-types.exp b/gdb/testsuite/gdb.cp/info-types.exp
new file mode 100644
index 00000000000..4834b52f8f1
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/info-types.exp
@@ -0,0 +1,42 @@
+# Copyright 2019 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that 'info types' produces the expected output for some C++
+# types.  Most basic 'info types' functionality is tested in
+# gdb.base/info-types.exp.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+gdb_test "info types" \
+    [multi_line \
+	 "All defined types:" \
+	 "" \
+	 "File .*:" \
+	 "16:\[\t \]+AA;" \
+	 "\[\t \]+int" \
+	 "21:\[\t \]+typedef AA my_a;" \
+	 "22:\[\t \]+typedef AA \\* my_ptr;" ]
+
-- 
2.14.5

Re: [PATCH 2/3] gdb: Improve output from "info types" commad

[Pedro Alves]

On 7/12/19 12:37 PM, Andrew Burgess wrote:
> This commit makes two changes to the "info types" command:
> 
> First, only use typedef_print for printing typedefs, and use
> type_print for printing non-typedef scalar (non-struct) types.  The
> result of this is the output for builtin types goes from this:
> 
>     typedef double;
>     typedef float;
>     typedef int;
> 
> to this:
> 
>     double;
>     float;
>     int;
> 
> which seems to make more sense.
> 
> Next GDB no longer matches msymbols as possible type names.  When
> looking for function symbols it makes sense to report matching
> msymbols from the text sections, and for variables msymbols from the
> data/bss sections, but when reporting types GDB would match msymbols
> of type absolute.  But I don't see why these are likely to indicate
> type names.  As such I've updated the msymbol matching lists in
> symtab.c:search_symbols so that when searching in the TYPES_DOMAIN, we
> never match any msymbols.
> 

That sounds fine to me.

> 	* symtab.c (search_symbols): Adjust msymbol matching type arrays
> 	so that GDB doesn't match any msymbols when searching in the
> 	TYPES_DOMAIN.
> 	(print_symbol_info): Print using typedef_print or type_print based
> 	on the type of the symbol.  Add updated FIXME comment mobed from...
> 	(_initialize_symtab): ... move and update FIXME comment to above.
> 

Typo: "mobed".

> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/info-types.c: New file.
> 	* gdb.base/info-types.exp: New file.
> 	* gdb.cp/info-types.cc: New file.
> 	* gdb.cp/info-types.exp: New file.

I'm not immediately seeing what the C++ testcase is trying to
test.  If you replace "class" with struct, and use "typedef struct AA"
instead of "typedef AA", isn't that code basically C code as well?

Also, might it be a good idea to check the info-types.c stuff in
C++ mode as well, to make sure we normalize C and C++ modes?
I.e., make gdb.base/info-types.exp compile once as a C program, and
once as a C++ program?

Thanks,
Pedro Alves

Re: [PATCH 2/3] gdb: Improve output from "info types" commad

[Andrew Burgess]

* Pedro Alves <palves@redhat.com> [2019-07-18 21:07:10 +0100]:

> On 7/12/19 12:37 PM, Andrew Burgess wrote:
> > This commit makes two changes to the "info types" command:
> > 
> > First, only use typedef_print for printing typedefs, and use
> > type_print for printing non-typedef scalar (non-struct) types.  The
> > result of this is the output for builtin types goes from this:
> > 
> >     typedef double;
> >     typedef float;
> >     typedef int;
> > 
> > to this:
> > 
> >     double;
> >     float;
> >     int;
> > 
> > which seems to make more sense.
> > 
> > Next GDB no longer matches msymbols as possible type names.  When
> > looking for function symbols it makes sense to report matching
> > msymbols from the text sections, and for variables msymbols from the
> > data/bss sections, but when reporting types GDB would match msymbols
> > of type absolute.  But I don't see why these are likely to indicate
> > type names.  As such I've updated the msymbol matching lists in
> > symtab.c:search_symbols so that when searching in the TYPES_DOMAIN, we
> > never match any msymbols.
> > 
> 
> That sounds fine to me.
> 
> > 	* symtab.c (search_symbols): Adjust msymbol matching type arrays
> > 	so that GDB doesn't match any msymbols when searching in the
> > 	TYPES_DOMAIN.
> > 	(print_symbol_info): Print using typedef_print or type_print based
> > 	on the type of the symbol.  Add updated FIXME comment mobed from...
> > 	(_initialize_symtab): ... move and update FIXME comment to above.
> > 
> 
> Typo: "mobed".
> 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.base/info-types.c: New file.
> > 	* gdb.base/info-types.exp: New file.
> > 	* gdb.cp/info-types.cc: New file.
> > 	* gdb.cp/info-types.exp: New file.
> 
> I'm not immediately seeing what the C++ testcase is trying to
> test.  If you replace "class" with struct, and use "typedef struct AA"
> instead of "typedef AA", isn't that code basically C code as well?
> 
> Also, might it be a good idea to check the info-types.c stuff in
> C++ mode as well, to make sure we normalize C and C++ modes?
> I.e., make gdb.base/info-types.exp compile once as a C program, and
> once as a C++ program?

I've removed the gdb.cp/info-types.{cc,exp} files, and extended the
gdb.base test to include an example of 'class'.  Yes, we know
internally they are handled just like structs, but I'd like to cover
it "just in case".  The gdb.base case is now compiled as C and C++.

Thanks,
Andrew

---

    gdb: Improve output from "info types" commad
    
    This commit makes two changes to the "info types" command:
    
    First, only use typedef_print for printing typedefs, and use
    type_print for printing non-typedef scalar (non-struct) types.  The
    result of this is the output for builtin types goes from this:
    
        typedef double;
        typedef float;
        typedef int;
    
    to this:
    
        double;
        float;
        int;
    
    which seems to make more sense.
    
    Next GDB no longer matches msymbols as possible type names.  When
    looking for function symbols it makes sense to report matching
    msymbols from the text sections, and for variables msymbols from the
    data/bss sections, but when reporting types GDB would match msymbols
    of type absolute.  But I don't see why these are likely to indicate
    type names.  As such I've updated the msymbol matching lists in
    symtab.c:search_symbols so that when searching in the TYPES_DOMAIN, we
    never match any msymbols.
    
    gdb/ChangeLog:
    
            * symtab.c (search_symbols): Adjust msymbol matching type arrays
            so that GDB doesn't match any msymbols when searching in the
            TYPES_DOMAIN.
            (print_symbol_info): Print using typedef_print or type_print based
            on the type of the symbol.  Add updated FIXME comment moved from...
            (_initialize_symtab): ... move and update FIXME comment to above.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.base/info-types.c: New file.
            * gdb.base/info-types.exp: New file.

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 16861e2bc92..ba0d5eed6b3 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4339,13 +4339,13 @@ search_symbols (const char *regexp, enum search_domain kind,
   struct symbol *sym;
   int found_misc = 0;
   static const enum minimal_symbol_type types[]
-    = {mst_data, mst_text, mst_abs};
+    = {mst_data, mst_text, mst_unknown};
   static const enum minimal_symbol_type types2[]
-    = {mst_bss, mst_file_text, mst_abs};
+    = {mst_bss, mst_file_text, mst_unknown};
   static const enum minimal_symbol_type types3[]
-    = {mst_file_data, mst_solib_trampoline, mst_abs};
+    = {mst_file_data, mst_solib_trampoline, mst_unknown};
   static const enum minimal_symbol_type types4[]
-    = {mst_file_bss, mst_text_gnu_ifunc, mst_abs};
+    = {mst_file_bss, mst_text_gnu_ifunc, mst_unknown};
   enum minimal_symbol_type ourtype;
   enum minimal_symbol_type ourtype2;
   enum minimal_symbol_type ourtype3;
@@ -4628,7 +4628,23 @@ print_symbol_info (enum search_domain kind,
   /* Typedef that is not a C++ class.  */
   if (kind == TYPES_DOMAIN
       && SYMBOL_DOMAIN (sym) != STRUCT_DOMAIN)
-    typedef_print (SYMBOL_TYPE (sym), sym, gdb_stdout);
+    {
+      /* FIXME: For C (and C++) we end up with a difference in output here
+	 between how a typedef is printed, and non-typedefs are printed.
+	 The TYPEDEF_PRINT code places a ";" at the end in an attempt to
+	 appear C-like, while TYPE_PRINT doesn't.
+
+	 For the struct printing case below, things are worse, we force
+	 printing of the ";" in this function, which is going to be wrong
+	 for languages that don't require a ";" between statements.  */
+      if (TYPE_CODE (SYMBOL_TYPE (sym)) == TYPE_CODE_TYPEDEF)
+	typedef_print (SYMBOL_TYPE (sym), sym, gdb_stdout);
+      else
+	{
+	  type_print (SYMBOL_TYPE (sym), "", gdb_stdout, -1);
+	  printf_filtered ("\n");
+	}
+    }
   /* variable, func, or typedef-that-is-c++-class.  */
   else if (kind < TYPES_DOMAIN
 	   || (kind == TYPES_DOMAIN
@@ -6080,14 +6096,6 @@ Prints the functions.\n"),
 				  _("functions")));
   set_cmd_completer_handle_brkchars (c, info_print_command_completer);
 
-  /* FIXME:  This command has at least the following problems:
-     1.  It prints builtin types (in a very strange and confusing fashion).
-     2.  It doesn't print right, e.g. with
-     typedef struct foo *FOO
-     type_print prints "FOO" when we want to make it (in this situation)
-     print "struct foo *".
-     I also think "ptype" or "whatis" is more likely to be useful (but if
-     there is much disagreement "info types" can be fixed).  */
   c = add_info ("types", info_types_command, _("\
 All type names, or those matching REGEXP.\n\
 Usage: info types [-q] [REGEXP]\n\
diff --git a/gdb/testsuite/gdb.base/info-types.c b/gdb/testsuite/gdb.base/info-types.c
new file mode 100644
index 00000000000..6746238801d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-types.c
@@ -0,0 +1,78 @@
+/* Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+typedef int my_int_t;
+typedef float my_float_t;
+typedef my_int_t nested_int_t;
+typedef my_float_t nested_float_t;
+
+struct baz_t
+{
+  float f;
+  double d;
+};
+
+typedef struct baz_t baz_t;
+typedef struct baz_t baz;
+typedef baz_t nested_baz_t;
+typedef baz nested_baz;
+typedef struct baz_t *baz_ptr;
+
+enum enum_t
+{
+ AA, BB, CC
+};
+
+typedef enum enum_t my_enum_t;
+typedef my_enum_t nested_enum_t;
+
+volatile int var_a;
+volatile float var_b;
+volatile my_int_t var_c;
+volatile my_float_t var_d;
+volatile nested_int_t var_e;
+volatile nested_float_t var_f;
+volatile struct baz_t var_g;
+volatile baz_t var_h;
+volatile baz var_i;
+volatile nested_baz_t var_j;
+volatile nested_baz var_k;
+volatile baz_ptr var_l;
+volatile enum enum_t var_m;
+volatile my_enum_t var_n;
+volatile nested_enum_t var_o;
+
+#ifdef __cplusplus
+
+class CL
+{
+  int a;
+};
+
+typedef CL my_cl;
+typedef CL *my_ptr;
+
+volatile CL var_cpp_a;
+volatile my_cl var_cpp_b;
+volatile my_ptr var_cpp_c;
+
+#endif /* __cplusplus */
+
+int
+main ()
+{
+  asm ("" ::: "memory");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/info-types.exp b/gdb/testsuite/gdb.base/info-types.exp
new file mode 100644
index 00000000000..a5808425398
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-types.exp
@@ -0,0 +1,109 @@
+# Copyright 2019 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that 'info types' produces the expected output for an inferior
+# containing a number of different types.
+
+# Only test C++ if we are able.  Always use C.
+if { [skip_cplus_tests] || [get_compiler_info "c++"] } {
+    set lang {c}
+} else {
+    set lang {c c++}
+}
+
+foreach l $lang {
+    set dir "$l"
+    remote_exec host "rm -rf [standard_output_file ${dir}]"
+    remote_exec host "mkdir -p [standard_output_file ${dir}]"
+}
+
+# Run 'info types' test, compiling the test file for language LANG,
+# which should be either 'c' or 'c++'.
+proc run_test { lang } {
+    global testfile
+    global srcfile
+    global binfile
+    global subdir
+    global srcdir
+    global compile_flags
+
+    standard_testfile .c
+
+    if {[prepare_for_testing "failed to prepare" \
+	     "${lang}/${testfile}" $srcfile "debug $lang"]} {
+	return -1
+    }
+
+    if ![runto_main] then {
+	fail "can't run to main"
+	return 0
+    }
+
+    if { $lang == "c++" } {
+	set output_re \
+	    [multi_line \
+		 "All defined types:" \
+		 "" \
+		 "File .*:" \
+		 "59:\[\t \]+CL;" \
+		 "21:\[\t \]+baz_t;" \
+		 "33:\[\t \]+enum_t;" \
+		 "28:\[\t \]+typedef baz_t baz;" \
+		 "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
+		 "27:\[\t \]+typedef baz_t baz_t;" \
+		 "\[\t \]+double" \
+		 "\[\t \]+float" \
+		 "\[\t \]+int" \
+		 "64:\[\t \]+typedef CL my_cl;" \
+		 "38:\[\t \]+typedef enum_t my_enum_t;" \
+		 "17:\[\t \]+typedef float my_float_t;" \
+		 "16:\[\t \]+typedef int my_int_t;" \
+		 "65:\[\t \]+typedef CL \\* my_ptr;" \
+		 "30:\[\t \]+typedef baz_t nested_baz;" \
+		 "29:\[\t \]+typedef baz_t nested_baz_t;" \
+		 "39:\[\t \]+typedef enum_t nested_enum_t;" \
+		 "19:\[\t \]+typedef float nested_float_t;" \
+		 "18:\[\t \]+typedef int nested_int_t;" \
+		 "\[\t \]+unsigned int"]
+    } else {
+	set output_re \
+	    [multi_line \
+		 "All defined types:" \
+		 "" \
+		 "File .*:" \
+		 "28:\[\t \]+typedef struct baz_t baz;" \
+		 "31:\[\t \]+typedef struct baz_t \\* baz_ptr;" \
+		 "21:\[\t \]+struct baz_t;" \
+		 "\[\t \]+double" \
+		 "33:\[\t \]+enum enum_t;" \
+		 "\[\t \]+float" \
+		 "\[\t \]+int" \
+		 "38:\[\t \]+typedef enum enum_t my_enum_t;" \
+		 "17:\[\t \]+typedef float my_float_t;" \
+		 "16:\[\t \]+typedef int my_int_t;" \
+		 "30:\[\t \]+typedef struct baz_t nested_baz;" \
+		 "29:\[\t \]+typedef struct baz_t nested_baz_t;" \
+		 "39:\[\t \]+typedef enum enum_t nested_enum_t;" \
+		 "19:\[\t \]+typedef float nested_float_t;" \
+		 "18:\[\t \]+typedef int nested_int_t;" \
+		 "\[\t \]+unsigned int" ]
+    }
+
+    gdb_test "info types" $output_re
+}
+
+foreach_with_prefix l $lang {
+    run_test $l
+}

Re: [PATCH 2/3] gdb: Improve output from "info types" commad

[Pedro Alves]

On 7/19/19 8:43 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2019-07-18 21:07:10 +0100]:
> 
>> On 7/12/19 12:37 PM, Andrew Burgess wrote:

>> I'm not immediately seeing what the C++ testcase is trying to
>> test.  If you replace "class" with struct, and use "typedef struct AA"
>> instead of "typedef AA", isn't that code basically C code as well?
>>
>> Also, might it be a good idea to check the info-types.c stuff in
>> C++ mode as well, to make sure we normalize C and C++ modes?
>> I.e., make gdb.base/info-types.exp compile once as a C program, and
>> once as a C++ program?
> 
> I've removed the gdb.cp/info-types.{cc,exp} files, and extended the
> gdb.base test to include an example of 'class'.

Ah, OK, it's "class" specifically that you were looking at test.
I wasn't sure.

> Yes, we know
> internally they are handled just like structs, but I'd like to cover
> it "just in case".  The gdb.base case is now compiled as C and C++.

Thanks.

Pedro Alves

Re: [PATCH 2/3] gdb: Improve output from "info types" commad

[Pedro Alves]

On 7/19/19 8:43 PM, Andrew Burgess wrote:

> I've removed the gdb.cp/info-types.{cc,exp} files, and extended the
> gdb.base test to include an example of 'class'.  Yes, we know
> internally they are handled just like structs, but I'd like to cover
> it "just in case".  The gdb.base case is now compiled as C and C++.
> 
LGTM.

Thanks,
Pedro Alves