From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19496 invoked by alias); 25 Jun 2010 21:29:42 -0000 Received: (qmail 19480 invoked by uid 22791); 25 Jun 2010 21:29:39 -0000 X-SWARE-Spam-Status: No, hits=-5.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,TW_FN,TW_XZ,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; Fri, 25 Jun 2010 21:29:33 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5PLTVbX031862 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 25 Jun 2010 17:29:32 -0400 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o5PLTTY1006633 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 25 Jun 2010 17:29:31 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id o5PLTT9x031427; Fri, 25 Jun 2010 23:29:29 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id o5PLTTTV031426; Fri, 25 Jun 2010 23:29:29 +0200 Date: Fri, 25 Jun 2010 21:29:00 -0000 From: Jan Kratochvil To: Tom Tromey Cc: gdb-patches@sourceware.org, Sami Wagiaalla Subject: Re: [patch 2/2] ptype should list also class's typedefs Message-ID: <20100625212929.GB14185@host0.dyn.jankratochvil.net> References: <20100614155942.GB23639@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-12-10) X-IsSubscribed: yes 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/msg00601.txt.bz2 On Thu, 17 Jun 2010 04:31:57 +0200, Tom Tromey wrote: > >>>>> "Jan" == Jan Kratochvil writes: > 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. Asked about, no response so I have implemented it as you say, not agreed upon. gdbtypes.h #defined field accessors http://sourceware.org/ml/gdb/2010-06/msg00121.html > Access permissions are also dropped, that seems maybe a little more useful. I was not aware they apply. Filed GCC PR debug/44668 and GDB PR c++/11757. > 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? I just made s/die_type/read_type_die/. The new typedef symbol is created in process_structure_scope->process_die->read_type_die+new_symbol. While we could move this new_symbol call into this dwarf2_add_typedef I do not see the point. Currently the symbol is global (fully qualified) anyway so it fits IMO more into the general loop of process_structure_scope. > It seems like that would save a little space, There is no memory difference as read_type_die is used by both and the same TYPE_CODE_TYPEDEF type gets shared by get_die_type->htab_find_with_hash. > 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...) The global fully qualified type symbol's type is unrelated to this piece of code. And it was already TYPE_CODE_TYPEDEF (=typedef-correct) before. In fact this s/die_type/read_type_die/ change has only brought in a need of one TYPE_TARGET_TYPE dereference in c_type_print_base (for `ptype'), this field has currently no other use. I followed your recommendation as the data structure now holds more valuable information - one can easily TYPE_TARGET_TYPE dereference it but one could not do the opposite before. > 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. I do not see any change with the TYPE_CODE_TYPEDEF dereference. And I do not believe GDB should strip anything. For source: typedef long t; class C { typedef int t; t g (t i) {} } c; GDB now displays: (gdb) ptype C type = class C { private: C::t g(C::t); typedef int t; } As `g' is printed without `C::' I believe even ` t;' should not be ` C::t;'. GDB currently does not try to shorten some common namespaces and it always uses fully qualified names. It could probably shorten at least `std::': (gdb) ptype cout type = struct std::basic_ostream > : public virtual std::basic_ios > { [...] std::basic_ostream > & seekp(long, std::_Ios_Seekdir); Do you suggest to already implement some form of such namespaces shortening? I have no clue how it fits into some general GDB namespaces shortening plan. I find it less clear for debugging purposes to display just "t" there due to the possible confusion of correct `int' with incorrect global `long'. Thanks, Jan gdb/ 2010-06-25 Jan Kratochvil * c-typeprint.c (c_type_print_base): For no fields check include also TYPE_TYPEDEF_FIELD_COUNT. Print new typedefs section. * dwarf2read.c (struct typedef_field_list) (struct field_info) : New. (dwarf2_add_typedef): New. (read_structure_type): Call dwarf2_add_typedef for DW_TAG_typedef. Copy also FI.TYPEDEF_FIELD_LIST. * gdbtypes.h (struct typedef_field) (struct cplus_struct_type) (TYPE_TYPEDEF_FIELD_ARRAY, TYPE_TYPEDEF_FIELD, TYPE_TYPEDEF_FIELD_NAME) (TYPE_TYPEDEF_FIELD_TYPE, TYPE_TYPEDEF_FIELD_COUNT): New. gdb/testsuite/ 2010-06-25 Jan Kratochvil * gdb.cp/namespace.exp (ptype OtherFileClass typedefs) (ptype ::C::OtherFileClass typedefs): New. * gdb.cp/namespace1.cc (C::OtherFileClass::cOtherFileClassType2) (C::OtherFileClass::cOtherFileClassVar2): New. (C::OtherFileClass::cOtherFileClassVar_use): Use also cOtherFileClassVar2. (C::cOtherFileType2, C::cOtherFileVar2): New. (C::cOtherFileVar_use): use also cOtherFileVar2. * gdb.cp/userdef.exp (ptype &*c): Permit arbitrary trailing text. --- a/gdb/c-typeprint.c +++ b/gdb/c-typeprint.c @@ -768,7 +768,8 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show, cp_type_print_derivation_info (stream, type); fprintf_filtered (stream, "{\n"); - if ((TYPE_NFIELDS (type) == 0) && (TYPE_NFN_FIELDS (type) == 0)) + if (TYPE_NFIELDS (type) == 0 && TYPE_NFN_FIELDS (type) == 0 + && TYPE_TYPEDEF_FIELD_COUNT (type) == 0) { if (TYPE_STUB (type)) fprintfi_filtered (level + 4, stream, _("\n")); @@ -1058,6 +1059,29 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show, } } + /* Print typedefs defined in this class. */ + + if (TYPE_TYPEDEF_FIELD_COUNT (type) != 0) + { + if (TYPE_NFIELDS (type) != 0 || TYPE_NFN_FIELDS (type) != 0) + fprintf_filtered (stream, "\n"); + + for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); i++) + { + struct type *target = TYPE_TYPEDEF_FIELD_TYPE (type, i); + + /* Dereference the typedef declaration itself. */ + gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF); + target = TYPE_TARGET_TYPE (target); + + print_spaces_filtered (level + 4, stream); + fprintf_filtered (stream, "typedef "); + c_print_type (target, TYPE_TYPEDEF_FIELD_NAME (type, i), + stream, show - 1, level + 4); + fprintf_filtered (stream, ";\n"); + } + } + fprintfi_filtered (level, stream, "}"); if (TYPE_LOCALTYPE_PTR (type) && show >= 0) --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -645,6 +645,16 @@ struct field_info /* Number of entries in the fnfieldlists array. */ int nfnfields; + + /* typedefs defined inside this class. TYPEDEF_FIELD_LIST contains head of + a NULL terminated list of TYPEDEF_FIELD_LIST_COUNT elements. */ + struct typedef_field_list + { + struct typedef_field field; + struct typedef_field_list *next; + } + *typedef_field_list; + unsigned typedef_field_list_count; }; /* One item on the queue of compilation units to read in full symbols @@ -4668,6 +4678,39 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die, } } +/* Add a typedef defined in the scope of the FIP's class. */ + +static void +dwarf2_add_typedef (struct field_info *fip, struct die_info *die, + struct dwarf2_cu *cu) +{ + struct objfile *objfile = cu->objfile; + struct gdbarch *gdbarch = get_objfile_arch (objfile); + struct typedef_field_list *new_field; + struct attribute *attr; + struct typedef_field *fp; + char *fieldname = ""; + + /* Allocate a new field list entry and link it in. */ + new_field = xzalloc (sizeof (*new_field)); + make_cleanup (xfree, new_field); + + gdb_assert (die->tag == DW_TAG_typedef); + + fp = &new_field->field; + + /* Get name of field. */ + fp->name = dwarf2_name (die, cu); + if (fp->name == NULL) + return; + + fp->type = read_type_die (die, cu); + + new_field->next = fip->typedef_field_list; + fip->typedef_field_list = new_field; + fip->typedef_field_list_count++; +} + /* Create the vector of fields, and attach it to the type. */ static void @@ -5199,6 +5242,8 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu) /* C++ base class field. */ dwarf2_add_field (&fi, child_die, cu); } + else if (child_die->tag == DW_TAG_typedef) + dwarf2_add_typedef (&fi, child_die, cu); child_die = sibling_die (child_die); } @@ -5272,6 +5317,28 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu) } } } + + /* Copy fi.typedef_field_list linked list elements content into the + allocated array TYPE_TYPEDEF_FIELD_ARRAY (type). */ + if (fi.typedef_field_list) + { + int i = fi.typedef_field_list_count; + + TYPE_TYPEDEF_FIELD_ARRAY (type) + = TYPE_ALLOC (type, sizeof (TYPE_TYPEDEF_FIELD (type, 0)) * i); + TYPE_TYPEDEF_FIELD_COUNT (type) = i; + + /* Reverse the list order to keep the debug info elements order. */ + while (--i >= 0) + { + struct typedef_field *dest, *src; + + dest = &TYPE_TYPEDEF_FIELD (type, i); + src = &fi.typedef_field_list->field; + fi.typedef_field_list = fi.typedef_field_list->next; + *dest = *src; + } + } } quirk_gcc_member_function_pointer (type, cu->objfile); --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -820,6 +820,19 @@ struct cplus_struct_type int line; } *localtype_ptr; + + /* typedefs defined inside this class. TYPEDEF_FIELD points to an array of + TYPEDEF_FIELD_COUNT elements. */ + struct typedef_field + { + /* Unqualified name to be prefixed by owning class qualified name. */ + const char *name; + + /* Type this typedef named NAME represents. */ + struct type *type; + } + *typedef_field; + unsigned typedef_field_count; }; /* Struct used in computing virtual base list */ @@ -1047,6 +1060,17 @@ extern void allocate_gnat_aux_type (struct type *); #define TYPE_LOCALTYPE_FILE(thistype) (TYPE_CPLUS_SPECIFIC(thistype)->localtype_ptr->file) #define TYPE_LOCALTYPE_LINE(thistype) (TYPE_CPLUS_SPECIFIC(thistype)->localtype_ptr->line) +#define TYPE_TYPEDEF_FIELD_ARRAY(thistype) \ + TYPE_CPLUS_SPECIFIC (thistype)->typedef_field +#define TYPE_TYPEDEF_FIELD(thistype, n) \ + TYPE_CPLUS_SPECIFIC (thistype)->typedef_field[n] +#define TYPE_TYPEDEF_FIELD_NAME(thistype, n) \ + TYPE_TYPEDEF_FIELD (thistype, n).name +#define TYPE_TYPEDEF_FIELD_TYPE(thistype, n) \ + TYPE_TYPEDEF_FIELD (thistype, n).type +#define TYPE_TYPEDEF_FIELD_COUNT(thistype) \ + TYPE_CPLUS_SPECIFIC (thistype)->typedef_field_count + #define TYPE_IS_OPAQUE(thistype) (((TYPE_CODE (thistype) == TYPE_CODE_STRUCT) || \ (TYPE_CODE (thistype) == TYPE_CODE_UNION)) && \ (TYPE_NFIELDS (thistype) == 0) && \ --- a/gdb/testsuite/gdb.cp/namespace.exp +++ b/gdb/testsuite/gdb.cp/namespace.exp @@ -265,6 +265,21 @@ gdb_test "ptype OtherFileClass" "type = (class C::OtherFileClass \{\r\n public: gdb_test "ptype ::C::OtherFileClass" "type = class C::OtherFileClass \{\r\n public:\r\n int z;\r\n.*\}" gdb_test "ptype C::OtherFileClass" "No symbol \"OtherFileClass\" in namespace \"C::C\"." +# Test class typedefs printing. +set expect "type = class C::OtherFileClass \{\r\n.*\r\n *typedef short cOtherFileClassType;\r\n *typedef long cOtherFileClassType2;\r\n\}" +if {[test_compiler_info {gcc-[0-3]-*}] + || [test_compiler_info {gcc-4-[0-4]-*}]} { + # The type in class is missing in older GCCs. + setup_xfail *-*-* +} +gdb_test "ptype OtherFileClass" $expect "ptype OtherFileClass typedefs" +if {[test_compiler_info {gcc-[0-3]-*}] + || [test_compiler_info {gcc-4-[0-4]-*}]} { + # The type in class is missing in older GCCs. + setup_xfail *-*-* +} +gdb_test "ptype ::C::OtherFileClass" $expect "ptype ::C::OtherFileClass typedefs" + # Some anonymous namespace tests. gdb_test "print cX" "\\$\[0-9\].* = 6" --- a/gdb/testsuite/gdb.cp/namespace1.cc +++ b/gdb/testsuite/gdb.cp/namespace1.cc @@ -23,12 +23,14 @@ namespace C int z; typedef short cOtherFileClassType; + typedef long cOtherFileClassType2; static const cOtherFileClassType cOtherFileClassVar = 318; + static const cOtherFileClassType2 cOtherFileClassVar2 = 320; cOtherFileClassType cOtherFileClassVar_use (); }; OtherFileClass::cOtherFileClassType OtherFileClass::cOtherFileClassVar_use () { - return cOtherFileClassVar; + return cOtherFileClassVar + cOtherFileClassVar2; } namespace { @@ -45,10 +47,12 @@ namespace C } typedef short cOtherFileType; + typedef long cOtherFileType2; static const cOtherFileType cOtherFileVar = 319; + static const cOtherFileType2 cOtherFileVar2 = 321; cOtherFileType cOtherFileVar_use () { - return cOtherFileVar; + return cOtherFileVar + cOtherFileVar2; } } --- a/gdb/testsuite/gdb.cp/userdef.exp +++ b/gdb/testsuite/gdb.cp/userdef.exp @@ -148,7 +148,7 @@ gdb_test "break A2::'operator +'" ".*Breakpoint $decimal at.*" gdb_test "print c" "\\\$\[0-9\]* = {m = {z = .*}}" gdb_test "print *c" "\\\$\[0-9\]* = \\(Member &\\) @$hex: {z = .*}" gdb_test "print &*c" "\\\$\[0-9\]* = \\(Member \\*\\) $hex" -gdb_test "ptype &*c" "type = (struct|class) Member {(\[\r\n \]+public:)?\[\r\n \]+int z;\[\r\n\]+} &\\*" +gdb_test "ptype &*c" "type = (struct|class) Member {(\[\r\n \]+public:)?\[\r\n \]+int z;\[\r\n\].*} &\\*" gdb_test "print operator== (mem1, mem2)" " = false" gdb_test "print operator== (mem1, mem1)" " = true"