[patch 2/2] ptype should list also class's typedefs

[patch 2/2] ptype should list also class's typedefs

[Jan Kratochvil]

Hi,

GDB now fully supports by patch 1/2 class's typedefs so it should also print:
type = class C::OtherFileClass {
  public:
    int z;
[...]
    C::OtherFileClass::cOtherFileClassType cOtherFileClassVar_use(void);

    typedef short cOtherFileClassType;
    typedef long cOtherFileClassType2;
}
[ the last two typedef lines are new ]


Types are now stored both at `C' main_type and (as before) the global symbols.


FYI there can be also `C::newtype' defined using:
  namespace C
    {
      typedef oldtype newtype;
    };
but in such case there must not exist `class C' otherwise C++ will reject it:
  .C:6: error: ‘struct A’ redeclared as different kind of symbol
  .C:2: error: previous declaration of ‘namespace A { }’

(A) That means if we are asked to `ptype C' it should be enough to search all
existing global symbols for types named `C::*' and print them.

(B) Another possibility is to suppress creating global symbols for such types
and always look them up through the types listed at the `C' class symbol.

As we should support even the `namespace' case in those cases there must exist
global symbols with fully qualified names `C::name' - (B) way not always
applicable.

I did not try to go the (A) way but it will be definitely much less
performance-wise.  We would have to limit search to the CU (Compilation Unit)
of the owning `C' class.

This patch creates both a local list of C's typedefs and the global symbols
for them.  It can have some memory impact but probably small enough as class's
typedefs are not common.

As a summary with the current global symbol table (not hierarchical by the
classes and namespaces) I find the current patch fits that scheme.  Any more
hierarchical change would have to be an incremental one on top of it anyway.


It is also questionable whether new TYPE_CPLUS_SPECIFIC(type)->typedef_field
list should be used as implemented by the patch or whether TYPE_FIELDS should
be used instead.  I am for removing TYPE_NFIELDS completely in the longterm
and provide all the items in union elements specific for very each type_code.
Overloading of TYPE_FIELDS for each type_code is currently very hard to read.

It also means the order of fields and typedefs is not preserved, though.
(Fields order is preserved and typedefs order is preserved but they are kept
as two separate sequences.)  I do not find it as a problem but even in such
case I would propose introducing some new index at each stored field instead
of overloading TYPE_FIELDS again.


copy_type_recursive does not try to copy the C++ specific fields - such as
currently the typedefs.
     NOTE drow/2005-12-09: We do not copy the C++-specific bits like
     base classes and methods.  There's no fundamental reason why we
     can't, but at the moment it is not needed.  */

No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu (gcc-4.4-rh).
For proper testing avoiding some XFAILs one needs at least gcc-4.5.

There should be also a new MI interface, I haven't checked it more so far.


Thanks,
Jan


gdb/
2010-06-14  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* c-typeprint.c (c_type_print_base): For no fields check include also
	TYPEDEF_FIELD_COUNT.  Print new typedefs section.
	* dwarf2read.c (struct typedef_field_list)
	(struct field_info) <typedef_field_list, typedef_field_list_count>: 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) <typedef_field, typedef_field_count>: New.

gdb/testsuite/
2010-06-14  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* 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.

--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -767,7 +767,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_CPLUS_SPECIFIC (type)->typedef_field_count == 0)
 	    {
 	      if (TYPE_STUB (type))
 		fprintfi_filtered (level + 4, stream, _("<incomplete type>\n"));
@@ -1057,6 +1058,28 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show,
 		}
 	    }
 
+	  /* Print typedefs defined in this class.  */
+
+	  if (TYPE_CPLUS_SPECIFIC (type)->typedef_field_count != 0)
+	    {
+	      if (TYPE_NFIELDS (type) != 0 || TYPE_NFN_FIELDS (type) != 0)
+		fprintf_filtered (stream, "\n");
+
+	      for (i = 0; i < TYPE_CPLUS_SPECIFIC (type)->typedef_field_count;
+	           i++)
+		{
+		  const struct typedef_field *field;
+
+		  field = &TYPE_CPLUS_SPECIFIC (type)->typedef_field[i];
+
+		  print_spaces_filtered (level + 4, stream);
+		  fprintf_filtered (stream, "typedef ");
+		  c_print_type (field->type, field->name, 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
@@ -4652,6 +4662,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 = die_type (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
@@ -5183,6 +5226,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);
 	}
 
@@ -5256,6 +5301,30 @@ 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_CPLUS_SPECIFIC (type)->typedef_field.  */
+      if (fi.typedef_field_list)
+	{
+	  int i = fi.typedef_field_list_count;
+
+	  TYPE_CPLUS_SPECIFIC (type)->typedef_field
+	    = TYPE_ALLOC (type,
+			  sizeof (*TYPE_CPLUS_SPECIFIC (type)->typedef_field)
+			  * i);
+	  TYPE_CPLUS_SPECIFIC (type)->typedef_field_count = i;
+
+	  /* Reverse the list order to keep the debug info elements order.  */
+	  while (--i >= 0)
+	    {
+	      struct typedef_field *dest, *src;
+	      
+	      dest = &TYPE_CPLUS_SPECIFIC (type)->typedef_field[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 */
--- 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;
   }
 }
 

Re: [patch 2/2] ptype should list also class's typedefs

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> 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

Re: [patch 2/2] ptype should list also class's typedefs

[Jan Kratochvil]

On Thu, 17 Jun 2010 04:31:57 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> 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<char, std::char_traits<char> > 
    : public virtual std::basic_ios<char, std::char_traits<char> > {
[...]
    std::basic_ostream<char, std::char_traits<char> > & 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  <jan.kratochvil@redhat.com>

	* 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) <typedef_field_list, typedef_field_list_count>: 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) <typedef_field, typedef_field_count>
	(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  <jan.kratochvil@redhat.com>

	* 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, _("<incomplete type>\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"

Re: [patch 2/2] ptype should list also class's typedefs

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Tom> It is customary to make new wrapper macros for new fields here.
Tom> I'm not sure that adds much benefit, but maybe just consistency is a
Tom> good enough reason.

Jan> Asked about, no response so I have implemented it as you say, not
Jan> agreed upon.  gdbtypes.h #defined field accessors
Jan> http://sourceware.org/ml/gdb/2010-06/msg00121.html

Sorry about the lack of a response.
I will reply to this separately.

Tom> Access permissions are also dropped, that seems maybe a little more
Tom> useful.

Jan> I was not aware they apply.  Filed GCC PR debug/44668 and GDB PR
Jan> c++/11757.

Thanks.

Jan> There is no memory difference as read_type_die is used by both and
Jan> the same TYPE_CODE_TYPEDEF type gets shared by
Jan> get_die_type->htab_find_with_hash.

Thanks.

Jan> GDB now displays:
Jan> 	(gdb) ptype C
Jan> 	type = class C {
Jan> 	  private:
Jan> 	    C::t g(C::t);
Jan> 	    typedef int t;
Jan> 	}

Jan> As `g' is printed without `C::' I believe even ` t;' should not be
Jan> ` C::t;'.

Yeah, I agree.  This output looks reasonable, I think I misunderstood
what would be printed here.

Jan> GDB currently does not try to shorten some common namespaces and it always
Jan> uses fully qualified names.  It could probably shorten at least `std::':

Jan> Do you suggest to already implement some form of such namespaces
Jan> shortening?  I have no clue how it fits into some general GDB
Jan> namespaces shortening plan.

I don't think we have any actual plan here.

Originally we had talked about making gdb print "whatever the user
wrote".  There's at least one GCC PR on this topic; the issue is that
GCC doesn't preserve the needed information in the DWARF, and arguably
it should not.

Roland had a couple of ideas in this area that seemed quite nice to me:

http://sourceware.org/ml/archer/2009-q3/msg00011.html
http://sourceware.org/ml/archer/2009-q3/msg00012.html

But AFAIK nobody is working on these yet.

Jan> 2010-06-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* c-typeprint.c (c_type_print_base): For no fields check include also
Jan> 	TYPE_TYPEDEF_FIELD_COUNT.  Print new typedefs section.
Jan> 	* dwarf2read.c (struct typedef_field_list)
Jan> 	(struct field_info) <typedef_field_list, typedef_field_list_count>: New.
Jan> 	(dwarf2_add_typedef): New.
Jan> 	(read_structure_type): Call dwarf2_add_typedef for DW_TAG_typedef.
Jan> 	Copy also FI.TYPEDEF_FIELD_LIST.
Jan> 	* gdbtypes.h (struct typedef_field)
Jan> 	(struct cplus_struct_type) <typedef_field, typedef_field_count>
Jan> 	(TYPE_TYPEDEF_FIELD_ARRAY, TYPE_TYPEDEF_FIELD, TYPE_TYPEDEF_FIELD_NAME)
Jan> 	(TYPE_TYPEDEF_FIELD_TYPE, TYPE_TYPEDEF_FIELD_COUNT): New.

Jan> 2010-06-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* gdb.cp/namespace.exp (ptype OtherFileClass typedefs)
Jan> 	(ptype ::C::OtherFileClass typedefs): New.
Jan> 	* gdb.cp/namespace1.cc (C::OtherFileClass::cOtherFileClassType2)
Jan> 	(C::OtherFileClass::cOtherFileClassVar2): New.
Jan> 	(C::OtherFileClass::cOtherFileClassVar_use): Use also
Jan> 	cOtherFileClassVar2.
Jan> 	(C::cOtherFileType2, C::cOtherFileVar2): New.
Jan> 	(C::cOtherFileVar_use): use also cOtherFileVar2.
Jan> 	* gdb.cp/userdef.exp (ptype &*c): Permit arbitrary trailing text.

This is ok.  Thanks.

Tom

Re: [patch 2/2] ptype should list also class's typedefs

[Jan Kratochvil]

On Mon, 28 Jun 2010 20:22:46 +0200, Tom Tromey wrote:
> Jan> 2010-06-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
> Jan> 	* c-typeprint.c (c_type_print_base): For no fields check include also
> Jan> 	TYPE_TYPEDEF_FIELD_COUNT.  Print new typedefs section.
> Jan> 	* dwarf2read.c (struct typedef_field_list)
> Jan> 	(struct field_info) <typedef_field_list, typedef_field_list_count>: New.
> Jan> 	(dwarf2_add_typedef): New.
> Jan> 	(read_structure_type): Call dwarf2_add_typedef for DW_TAG_typedef.
> Jan> 	Copy also FI.TYPEDEF_FIELD_LIST.
> Jan> 	* gdbtypes.h (struct typedef_field)
> Jan> 	(struct cplus_struct_type) <typedef_field, typedef_field_count>
> Jan> 	(TYPE_TYPEDEF_FIELD_ARRAY, TYPE_TYPEDEF_FIELD, TYPE_TYPEDEF_FIELD_NAME)
> Jan> 	(TYPE_TYPEDEF_FIELD_TYPE, TYPE_TYPEDEF_FIELD_COUNT): New.
> 
> Jan> 2010-06-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
> Jan> 	* gdb.cp/namespace.exp (ptype OtherFileClass typedefs)
> Jan> 	(ptype ::C::OtherFileClass typedefs): New.
> Jan> 	* gdb.cp/namespace1.cc (C::OtherFileClass::cOtherFileClassType2)
> Jan> 	(C::OtherFileClass::cOtherFileClassVar2): New.
> Jan> 	(C::OtherFileClass::cOtherFileClassVar_use): Use also
> Jan> 	cOtherFileClassVar2.
> Jan> 	(C::cOtherFileType2, C::cOtherFileVar2): New.
> Jan> 	(C::cOtherFileVar_use): use also cOtherFileVar2.
> Jan> 	* gdb.cp/userdef.exp (ptype &*c): Permit arbitrary trailing text.
> 
> This is ok.  Thanks.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-06/msg00199.html


Thanks,
Jan