This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: fix virtual base class bugs


>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> This patch fixes all the virtual base class printing bugs I could find.
Tom> I plan to check this in soon unless there are comments or objections.

[...]

Tom> I looked at this code a little, too.  AFAICT none of the DECLARED_TYPE_*
Tom> constants are ever written to a C++ structure, and I think both
Tom> DECLARED_TYPE_UNION and DECLARED_TYPE_TEMPLATE are useless as well.  In
Tom> this case I chose to change userdef.exp and then later look into
Tom> changing the DWARF reader to respect DW_TAG_class_type.

I wrote the follow-on patch today, and I'm pretty happy with it.
I intend to commit both of these soon.

This second patch removes TYPE_DECLARED_TYPE and a bunch of related
constants.  These are not currently used in gdb, except by accident:
when a cplus_struct_type is allocated, the declared_type field is set to
0 (== TYPE_DECLARED_CLASS).

In its place I put a single bit into main_type to indicate whether a
structure was declared using "struct" or "class".  I could have put this
into cplus_struct_type .. but that seems awfully heavy for a single bit
of information.

Finally, I changed the DWARF reader to set this bit when appropriate.

Built and regtested on x86-64 (compile farm).  I tested it in isolation,
so there is actually a "regression" in userdef.exp, but this is covered
by the previous patch.

I'm considering also removing TYPE_CODE_CLASS now.  Also, I noticed that
TYPE_CODE_TEMPLATE is never actually used -- and is probably not
actually usable -- so that is another candidate for deletion.

Tom

2010-01-29  Tom Tromey  <tromey@redhat.com>

	* m2-typeprint.c (m2_record_fields): Don't use
	TYPE_DECLARED_TYPE.
	* gdbtypes.h (TYPE_DECLARED_CLASS): New macro.
	(struct main_type) <flag_declared_class>: New field.
	(struct cplus_struct_type) <declared_type>: Remove.
	<ntemplate_args>: Move earlier.
	(DECLARED_TYPE_CLASS, DECLARED_TYPE_UNION, DECLARED_TYPE_STRUCT)
	(DECLARED_TYPE_TEMPLATE): Remove.
	(TYPE_DECLARED_TYPE): Remove.
	* gdbtypes.c (lookup_union): Don't use TYPE_DECLARED_TYPE.
	* dwarf2read.c (read_structure_type): Set TYPE_DECLARED_CLASS.
	* c-typeprint.c (c_type_print_base): Use TYPE_DECLARED_CLASS, not
	TYPE_DECLARED_TYPE.

2010-01-29  Tom Tromey  <tromey@redhat.com>

	* gdb.dwarf2/member-ptr-forwardref.exp: Update expected result for
	type-printing change.

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index d1af481..27746d9 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -702,35 +702,10 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show,
 
     case TYPE_CODE_STRUCT:
       c_type_print_modifier (type, stream, 0, 1);
-      /* Note TYPE_CODE_STRUCT and TYPE_CODE_CLASS have the same value,
-       * so we use another means for distinguishing them.
-       */
-      if (HAVE_CPLUS_STRUCT (type))
-	{
-	  switch (TYPE_DECLARED_TYPE (type))
-	    {
-	    case DECLARED_TYPE_CLASS:
-	      fprintf_filtered (stream, "class ");
-	      break;
-	    case DECLARED_TYPE_UNION:
-	      fprintf_filtered (stream, "union ");
-	      break;
-	    case DECLARED_TYPE_STRUCT:
-	      fprintf_filtered (stream, "struct ");
-	      break;
-	    default:
-	      /* If there is a CPLUS_STRUCT, assume class if not
-	       * otherwise specified in the declared_type field.
-	       */
-	      fprintf_filtered (stream, "class ");
-	      break;
-	    }			/* switch */
-	}
+      if (TYPE_DECLARED_CLASS (type))
+	fprintf_filtered (stream, "class ");
       else
-	{
-	  /* If not CPLUS_STRUCT, then assume it's a C struct */
-	  fprintf_filtered (stream, "struct ");
-	}
+	fprintf_filtered (stream, "struct ");
       goto struct_union;
 
     case TYPE_CODE_UNION:
@@ -786,8 +761,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show,
 	     masquerading as a class, if all members are public, there's
 	     no need for a "public:" label. */
 
-	  if ((TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_CLASS)
-	      || (TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_TEMPLATE))
+	  if (TYPE_DECLARED_CLASS (type))
 	    {
 	      QUIT;
 	      len = TYPE_NFIELDS (type);
@@ -815,8 +789,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show,
 		    }
 		}
 	    }
-	  else if ((TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_STRUCT)
-		   || (TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_UNION))
+	  else
 	    {
 	      QUIT;
 	      len = TYPE_NFIELDS (type);
@@ -863,10 +836,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show,
 		  || TYPE_FIELD_ARTIFICIAL (type, i))
 		continue;
 
-	      /* If this is a C++ class we can print the various C++ section
-	         labels. */
-
-	      if (HAVE_CPLUS_STRUCT (type) && need_access_label)
+	      if (need_access_label)
 		{
 		  if (TYPE_FIELD_PROTECTED (type, i))
 		    {
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 2f671ca..86bfecb 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -5007,11 +5007,12 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
     }
   else
     {
-      /* FIXME: TYPE_CODE_CLASS is currently defined to TYPE_CODE_STRUCT
-         in gdbtypes.h.  */
       TYPE_CODE (type) = TYPE_CODE_CLASS;
     }
 
+  if (cu->language == language_cplus && die->tag == DW_TAG_class_type)
+    TYPE_DECLARED_CLASS (type) = 1;
+
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr)
     {
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 2ff8647..46846c4 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1132,13 +1132,6 @@ lookup_union (char *name, struct block *block)
   if (TYPE_CODE (t) == TYPE_CODE_UNION)
     return t;
 
-  /* C++ unions may come out with TYPE_CODE_CLASS, but we look at
-   * a further "declared_type" field to discover it is really a union.
-   */
-  if (HAVE_CPLUS_STRUCT (t))
-    if (TYPE_DECLARED_TYPE (t) == DECLARED_TYPE_UNION)
-      return t;
-
   /* If we get here, it's not a union.  */
   error (_("This context has class, struct or enum %s, not a union."), 
 	 name);
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 72968a5..643fa03 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -279,6 +279,12 @@ enum type_instance_flag_value
 #define TYPE_OWNER(t) TYPE_MAIN_TYPE(t)->owner
 #define TYPE_OBJFILE(t) (TYPE_OBJFILE_OWNED(t)? TYPE_OWNER(t).objfile : NULL)
 
+/* True if this type was declared using the "class" keyword.  This is
+   only valid for C++ structure types, and only used for displaying
+   the type.  If false, the structure was declared as a "struct".  */
+
+#define TYPE_DECLARED_CLASS(t) (TYPE_MAIN_TYPE (t)->flag_declared_class)
+
 /* Constant type.  If this is set, the corresponding type has a
  * const modifier.
  */
@@ -386,6 +392,9 @@ struct main_type
   unsigned int flag_nottext : 1;
   unsigned int flag_fixed_instance : 1;
   unsigned int flag_objfile_owned : 1;
+  /* True if this type was declared with "class" rather than
+     "struct".  */
+  unsigned int flag_declared_class : 1;
 
   /* A discriminant telling us which field of the type_specific union
      is being used for this type, if any.  */
@@ -676,19 +685,10 @@ struct cplus_struct_type
 
     short nfn_fields_total;
 
-    /* The "declared_type" field contains a code saying how the
-       user really declared this type, e.g., "class s", "union s",
-       "struct s".
-       The 3 above things come out from the C++ compiler looking like classes, 
-       but we keep track of the real declaration so we can give
-       the correct information on "ptype". (Note: TEMPLATE may not
-       belong in this list...)  */
+    /* Number of template arguments, placed here for better struct
+       packing.  */
 
-#define DECLARED_TYPE_CLASS 0
-#define DECLARED_TYPE_UNION 1
-#define DECLARED_TYPE_STRUCT 2
-#define DECLARED_TYPE_TEMPLATE 3
-    short declared_type;	/* One of the above codes */
+    short ntemplate_args;
 
     /* For derived classes, the number of base classes is given by n_baseclasses
        and virtual_field_bits is a bit vector containing one bit per base class.
@@ -813,7 +813,6 @@ struct cplus_struct_type
      * is a name. "type" will typically just point to a "struct type" with
      * the placeholder TYPE_CODE_TEMPLATE_ARG type.
      */
-    short ntemplate_args;
     struct template_arg
       {
 	char *name;
@@ -943,7 +942,6 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_NFN_FIELDS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->nfn_fields
 #define TYPE_NFN_FIELDS_TOTAL(thistype) TYPE_CPLUS_SPECIFIC(thistype)->nfn_fields_total
 #define TYPE_NTEMPLATE_ARGS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->ntemplate_args
-#define TYPE_DECLARED_TYPE(thistype) TYPE_CPLUS_SPECIFIC(thistype)->declared_type
 #define TYPE_SPECIFIC_FIELD(thistype) \
   TYPE_MAIN_TYPE(thistype)->type_specific_field
 #define	TYPE_TYPE_SPECIFIC(thistype) TYPE_MAIN_TYPE(thistype)->type_specific
diff --git a/gdb/m2-typeprint.c b/gdb/m2-typeprint.c
index afa9978..46a35bb 100644
--- a/gdb/m2-typeprint.c
+++ b/gdb/m2-typeprint.c
@@ -547,9 +547,9 @@ m2_record_fields (struct type *type, struct ui_file *stream, int show,
   wrap_here ("    ");
   if (show < 0)
     {
-      if (TYPE_CODE (type) == DECLARED_TYPE_STRUCT)
+      if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
 	fprintf_filtered (stream, "RECORD ... END ");
-      else if (TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_UNION)
+      else if (TYPE_CODE (type) == TYPE_CODE_UNION)
 	fprintf_filtered (stream, "CASE ... END ");
     }
   else if (show > 0)
diff --git a/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp b/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp
index 0a54dfe..bb947bc 100644
--- a/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp
+++ b/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp
@@ -45,4 +45,4 @@ gdb_test "show cp-abi" {The currently selected C\+\+ ABI is "gnu-v3".*}
 
 gdb_load ${binfile}
 
-gdb_test "ptype c" "type = class C {\[\r\n \t\]*int \\(C::\\*fp\\)\\(C \\*\\);\[\r\n \t\]*}"
+gdb_test "ptype c" "type = struct C {\[\r\n \t\]*private:\[\r\n \t\]*int \\(C::\\*fp\\)\\(C \\*\\);\[\r\n \t\]*}"


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]