From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5039 invoked by alias); 17 Jun 2010 02:32:08 -0000 Received: (qmail 5027 invoked by uid 22791); 17 Jun 2010 02:32:06 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 17 Jun 2010 02:32:01 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5H2VxB1023945 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 16 Jun 2010 22:32:00 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o5H2Vxkh020759; Wed, 16 Jun 2010 22:31:59 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id o5H2Vwbd005691; Wed, 16 Jun 2010 22:31:58 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id C6BDD3782E3; Wed, 16 Jun 2010 20:31:57 -0600 (MDT) From: Tom Tromey To: Jan Kratochvil Cc: gdb-patches@sourceware.org, Sami Wagiaalla Subject: Re: [patch 2/2] ptype should list also class's typedefs References: <20100614155942.GB23639@host0.dyn.jankratochvil.net> Reply-To: tromey@redhat.com Date: Thu, 17 Jun 2010 02:32:00 -0000 In-Reply-To: <20100614155942.GB23639@host0.dyn.jankratochvil.net> (Jan Kratochvil's message of "Mon, 14 Jun 2010 17:59:42 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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 X-SW-Source: 2010-06/txt/msg00380.txt.bz2 >>>>> "Jan" == Jan Kratochvil writes: Jan> GDB now fully supports by patch 1/2 class's typedefs so it should Jan> also print: Jan> type = class C::OtherFileClass { Jan> public: Jan> int z; Jan> [...] Jan> C::OtherFileClass::cOtherFileClassType cOtherFileClassVar_use(void); Jan> typedef short cOtherFileClassType; Jan> typedef long cOtherFileClassType2; Jan> } Jan> [ the last two typedef lines are new ] I agree. Jan> (B) Another possibility is to suppress creating global symbols for Jan> such types and always look them up through the types listed at the Jan> `C' class symbol. Yeah. Jan> As we should support even the `namespace' case in those cases there Jan> must exist global symbols with fully qualified names `C::name' - Jan> (B) way not always applicable. If we really wanted multi-level symbol tables, then a namespace could also contain the symbols defined inside it. For me the key question is what practical advantage is there of a multi-level symbol table. It is a cleaner design, but I'm only really interested in it if it confers some other benefit. One hidden benefit of our current system, btw, is that things like lookups of imported variables work even if they are defined in some other CU, because the lookups are done by name. If we had a multi-level table we would also have to solve a cross-objfile problem here. (We may have to solve it anyway ...) Jan> I did not try to go the (A) way but it will be definitely much less Jan> performance-wise. We would have to limit search to the CU Jan> (Compilation Unit) of the owning `C' class. I'm not so sure. In fact due to ODR and GCC omitting things I think we would need a global search. Jan> It is also questionable whether new Jan> TYPE_CPLUS_SPECIFIC(type)->typedef_field list should be used as Jan> implemented by the patch or whether TYPE_FIELDS should be used Jan> instead. I am for removing TYPE_NFIELDS completely in the longterm Jan> and provide all the items in union elements specific for very each Jan> type_code. Overloading of TYPE_FIELDS for each type_code is Jan> currently very hard to read. I think your approach is fine. It is customary to make new wrapper macros for new fields here. I'm not sure that adds much benefit, but maybe just consistency is a good enough reason. Jan> It also means the order of fields and typedefs is not preserved, Jan> though. (Fields order is preserved and typedefs order is preserved Jan> but they are kept as two separate sequences.) I do not find it as Jan> a problem but even in such case I would propose introducing some Jan> new index at each stored field instead of overloading TYPE_FIELDS Jan> again. I'm not super concerned about the relative ordering of fields and typedefs. Access permissions are also dropped, that seems maybe a little more useful. I don't think we have to go completely overboard since usually the user has the source anyhow. Jan> There should be also a new MI interface, I haven't checked it more Jan> so far. ISTR that MI is weak for ptype anyhow. Jan> + gdb_assert (die->tag == DW_TAG_typedef); Jan> + Jan> + fp = &new_field->field; Jan> + Jan> + /* Get name of field. */ Jan> + fp->name = dwarf2_name (die, cu); Jan> + if (fp->name == NULL) Jan> + return; Jan> + Jan> + fp->type = die_type (die, cu); This seems a little odd. Why not just make a new typedef type, and store that directly in the outer type? It seems like that would save a little space, but more importantly give "C::t" the correct type -- the typedef and not the underlying type. (We aren't typedef-correct right now, but want to be...) Jan> + /* Unqualified name to be prefixed by owning class qualified name. */ Jan> + const char *name; I guess with the above approach you'd have to strip the qualifiers before printing the typedef name. But, it seems like that should be simple, since the qualifier is just the enclosing class' name. Tom