[3/3] 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 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(-)
  

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