This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]