gdb: Show type summary for anonymous structures from c_print_typedef
authorAndrew Burgess <andrew.burgess@embecosm.com>
Fri, 12 Jul 2019 10:26:32 +0000 (11:26 +0100)
committerAndrew Burgess <andrew.burgess@embecosm.com>
Mon, 22 Jul 2019 09:43:03 +0000 (10:43 +0100)
commita8e9d2471806ef86ff7aec43164a6fe347efba3b
tree01eee30050360a6209521d30843969e7ad10023e
parenteb86c5e2e824787875187901b12fba52ef873278
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.
gdb/ChangeLog
gdb/c-typeprint.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.ada/info_auto_lang.exp
gdb/testsuite/gdb.base/info-types.c
gdb/testsuite/gdb.base/info-types.exp
This page took 0.025895 seconds and 4 git commands to generate.