Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch 2/2] ptype should list also class's typedefs
@ 2010-06-14 15:59 Jan Kratochvil
  2010-06-17  2:32 ` Tom Tromey
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kratochvil @ 2010-06-14 15:59 UTC (permalink / raw)
  To: gdb-patches; +Cc: Sami Wagiaalla

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;
   }
 }
 


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-06-28 20:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-14 15:59 [patch 2/2] ptype should list also class's typedefs Jan Kratochvil
2010-06-17  2:32 ` Tom Tromey
2010-06-25 21:29   ` Jan Kratochvil
2010-06-28 18:22     ` Tom Tromey
2010-06-28 20:42       ` Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox