From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43883 invoked by alias); 19 Jul 2019 12:38:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 43795 invoked by uid 89); 19 Jul 2019 12:38:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=HX-Google-Smtp-Source:APXvYqxP X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 Jul 2019 12:38:39 +0000 Received: by mail-wr1-f66.google.com with SMTP id r1so32109887wrl.7 for ; Fri, 19 Jul 2019 05:38:38 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id p18sm30243868wrm.16.2019.07.19.05.38.36 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Fri, 19 Jul 2019 05:38:36 -0700 (PDT) Subject: Re: [PATCH 3/3] gdb: Show type summary for anonymous structures from c_print_typedef To: Andrew Burgess , gdb-patches@sourceware.org References: <4b4c8db945d3fbd3334b303b7acfa6d2c71b6bf6.1562931337.git.andrew.burgess@embecosm.com> From: Pedro Alves Message-ID: <3df72c85-469b-de6d-adb3-7e7d9ffd8278@redhat.com> Date: Fri, 19 Jul 2019 12:38:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <4b4c8db945d3fbd3334b303b7acfa6d2c71b6bf6.1562931337.git.andrew.burgess@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-07/txt/msg00455.txt.bz2 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