RFC: fix virtual base class bugs

RFC: fix virtual base class bugs

[Tom Tromey]

This patch fixes all the virtual base class printing bugs I could find.

I plan to check this in soon unless there are comments or objections.

There were a few:

* search_struct_field and check_field did not call check_typedef, so if
  a base class was a stub, they failed.

* cp_print_value had the wrong sanity check for whether the virtual base
  offset ran off the end of the enclosing object.  This bug was further
  obscured by the fact that the wrong enclosing type was passed in to
  this function in some cases.

* cp_print_value was also looking for the baseclass offset using the
  wrong inferior address.

* The Python code failed to offset the address as well as the contents
  by the embedded offset.


While doing this I ran into some oddities.

common_val_print passes in the embedded_offset from the value it
unpacks, but it doesn't pass in the enclosing class.  I looked at fixing
this, either by adding the enclosing class to the whole hierarchy, or by
changing the whole hierarchy to pass around a value plus a type+offset
representing the current subobject to print.  Both of these were bigger
changes than I wanted to make; but I do think that eventually we ought
to do the latter.

I found some confusion in the code as to the meaning and use of the
various val_print arguments.  So, I documented them to the best of my
knowledge in language.h.  There are probably still calls which are
wrong, I did not audit them all.

I introduced a new cp_print_value_fields_rtti to get the correct
enclosing class.  This has a funny side effect -- it calls
value_rtti_type, which can modify the argument type, adding a
c++-specific structure to it.  This in turn means that a type that
formerly printed as "struct x" will now print as "class x".  This showed
up in the test suite as the userdef.exp change.

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

Built and regtested on x86-64 (compile farm).  No regressions.

Tom


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

	PR c++/11226, PR c++/9629, PR c++/9688, PR c++/8890:
	* valops.c (search_struct_field): Compute nbases after calling
	CHECK_TYPEDEF.
	(check_field): Call CHECK_TYPEDEF.
	* cp-valprint.c (cp_print_value): Pass correct address to
	baseclass_offset.  Fix check for virtual base past the end of the
	object.  Don't offset address passed to cp_print_value_fields or
	apply_val_pretty_printer.
	(cp_print_value_fields): Fix call to val_print.
	(cp_print_value_fields_rtti): New function.
	* c-valprint.c (c_val_print): Use cp_print_value_fields_rtti.
	* p-valprint.c (pascal_object_print_value_fields): Fix call to
	val_print.
	* python/py-prettyprint.c (apply_val_pretty_printer): Add embedded
	offset to address.
	* language.h (struct language_defn) <la_val_print>: Document.
	* c-lang.h (cp_print_value_fields_rtti): Declare.

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

	PR c++/11226, PR c++/9629, PR c++/9688, PR c++/8890:
	* gdb.cp/virtbase.cc: New file.
	* gdb.cp/virtbase.exp: New file.
	* gdb.cp/userdef.exp: Allow 'struct' or 'class'.

Index: c-lang.h
===================================================================
RCS file: /cvs/src/src/gdb/c-lang.h,v
retrieving revision 1.26
diff -u -r1.26 c-lang.h
--- c-lang.h	14 Jan 2010 08:03:35 -0000	1.26
+++ c-lang.h	29 Jan 2010 16:16:46 -0000
@@ -102,6 +102,12 @@
 				   const struct value_print_options *,
 				   struct type **, int);
 
+extern void cp_print_value_fields_rtti (struct type *,
+					const gdb_byte *, int, CORE_ADDR,
+					struct ui_file *, int,
+					const struct value_print_options *,
+					struct type **, int);
+
 extern int cp_is_vtbl_ptr_type (struct type *);
 
 extern int cp_is_vtbl_member (struct type *);
Index: c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.66
diff -u -r1.66 c-valprint.c
--- c-valprint.c	25 Jan 2010 19:31:23 -0000	1.66
+++ c-valprint.c	29 Jan 2010 16:16:46 -0000
@@ -389,8 +389,9 @@
 					  options->addressprint);
 	}
       else
-	cp_print_value_fields (type, type, valaddr, embedded_offset, address, stream,
-			       recurse, options, NULL, 0);
+	cp_print_value_fields_rtti (type, valaddr,
+				    embedded_offset, address, stream,
+				    recurse, options, NULL, 0);
       break;
 
     case TYPE_CODE_ENUM:
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.233
diff -u -r1.233 valops.c
--- valops.c	26 Jan 2010 16:47:34 -0000	1.233
+++ valops.c	29 Jan 2010 16:16:46 -0000
@@ -1787,9 +1787,10 @@
 		     struct type *type, int looking_for_baseclass)
 {
   int i;
-  int nbases = TYPE_N_BASECLASSES (type);
+  int nbases;
 
   CHECK_TYPEDEF (type);
+  nbases = TYPE_N_BASECLASSES (type);
 
   if (!looking_for_baseclass)
     for (i = TYPE_NFIELDS (type) - 1; i >= nbases; i--)
@@ -2763,6 +2764,9 @@
 {
   int i;
 
+  /* The type may be a stub.  */
+  CHECK_TYPEDEF (type);
+
   for (i = TYPE_NFIELDS (type) - 1; i >= TYPE_N_BASECLASSES (type); i--)
     {
       char *t_field_name = TYPE_FIELD_NAME (type, i);
Index: cp-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/cp-valprint.c,v
retrieving revision 1.62
diff -u -r1.62 cp-valprint.c
--- cp-valprint.c	1 Jan 2010 07:31:30 -0000	1.62
+++ cp-valprint.c	29 Jan 2010 16:16:46 -0000
@@ -295,7 +295,7 @@
 		  opts.deref_ref = 0;
 		  val_print (TYPE_FIELD_TYPE (type, i),
 			     valaddr, offset + TYPE_FIELD_BITPOS (type, i) / 8,
-			     address + TYPE_FIELD_BITPOS (type, i) / 8,
+			     address,
 			     stream, recurse + 1, &opts,
 			     current_language);
 		}
@@ -321,6 +321,39 @@
   fprintf_filtered (stream, "}");
 }
 
+/* Like cp_print_value_fields, but find the runtime type of the object
+   and pass it as the `real_type' argument to cp_print_value_fields.
+   This function is a hack to work around the fact that
+   common_val_print passes the embedded offset to val_print, but not
+   the enclosing type.  */
+
+void
+cp_print_value_fields_rtti (struct type *type,
+			    const gdb_byte *valaddr, int offset,
+			    CORE_ADDR address,
+			    struct ui_file *stream, int recurse,
+			    const struct value_print_options *options,
+			    struct type **dont_print_vb, int dont_print_statmem)
+{
+  struct value *value;
+  int full, top, using_enc;
+  struct type *real_type;
+
+  /* Ugh, we have to convert back to a value here.  */
+  value = value_from_contents_and_address (type, valaddr + offset,
+					   address + offset);
+  /* We don't actually care about most of the result here -- just the
+     type.  We already have the correct offset, due to how val_print
+     was initially called.  */
+  real_type = value_rtti_type (value, &full, &top, &using_enc);
+  if (!real_type)
+    real_type = type;
+
+  cp_print_value_fields (type, real_type, valaddr, offset,
+			 address, stream, recurse, options,
+			 dont_print_vb, dont_print_statmem);
+}
+
 /* Special val_print routine to avoid printing multiple copies of virtual
    baseclasses.  */
 
@@ -373,7 +406,7 @@
       thisoffset = offset;
       thistype = real_type;
 
-      boffset = baseclass_offset (type, i, valaddr + offset, address);
+      boffset = baseclass_offset (type, i, valaddr + offset, address + offset);
       skip = ((boffset == -1) || (boffset + offset) < 0) ? 1 : -1;
 
       if (BASETYPE_VIA_VIRTUAL (type, i))
@@ -384,7 +417,7 @@
 
 	  if (boffset != -1
 	      && ((boffset + offset) < 0
-		  || (boffset + offset) >= TYPE_LENGTH (type)))
+		  || (boffset + offset) >= TYPE_LENGTH (real_type)))
 	    {
 	      /* FIXME (alloca): unsafe if baseclass is really really large. */
 	      gdb_byte *buf = alloca (TYPE_LENGTH (baseclass));
@@ -427,14 +460,14 @@
 	  if (!options->raw)
 	    result = apply_val_pretty_printer (baseclass, base_valaddr,
 					       thisoffset + boffset,
-					       address + boffset,
+					       address,
 					       stream, recurse,
 					       options,
 					       current_language);
 	  	  
 	  if (!result)
 	    cp_print_value_fields (baseclass, thistype, base_valaddr,
-				   thisoffset + boffset, address + boffset,
+				   thisoffset + boffset, address,
 				   stream, recurse, options,
 				   ((struct type **)
 				    obstack_base (&dont_print_vb_obstack)),
@@ -501,7 +534,8 @@
 		    sizeof (CORE_ADDR));
 
       CHECK_TYPEDEF (type);
-      cp_print_value_fields (type, type, value_contents_all (val),
+      cp_print_value_fields (type, value_enclosing_type (val),
+			     value_contents_all (val),
 			     value_embedded_offset (val), addr,
 			     stream, recurse, options, NULL, 1);
       return;
Index: language.h
===================================================================
RCS file: /cvs/src/src/gdb/language.h,v
retrieving revision 1.62
diff -u -r1.62 language.h
--- language.h	14 Jan 2010 08:03:36 -0000	1.62
+++ language.h	29 Jan 2010 16:16:46 -0000
@@ -208,11 +208,32 @@
     void (*la_print_typedef) (struct type *type, struct symbol *new_symbol,
 			      struct ui_file *stream);
 
-    /* Print a value using syntax appropriate for this language. */
+    /* Print a value using syntax appropriate for this language.
+       
+       TYPE is the type of the sub-object to be printed.
 
-    int (*la_val_print) (struct type *, const gdb_byte *, int, CORE_ADDR,
-			 struct ui_file *, int,
-			 const struct value_print_options *);
+       CONTENTS holds the bits of the value.  This holds the entire
+       enclosing object.
+
+       EMBEDDED_OFFSET is the offset into the outermost object of the
+       sub-object represented by TYPE.  This is the object which this
+       call should print.  Note that the enclosing type is not
+       available.
+
+       ADDRESS is the address in the inferior of the enclosing object.
+
+       STREAM is the stream on which the value is to be printed.
+
+       RECURSE is the recursion depth.  It is zero-based.
+
+       OPTIONS are the formatting options to be used when
+       printing.  */
+
+    int (*la_val_print) (struct type *type,
+			 const gdb_byte *contents,
+			 int embedded_offset, CORE_ADDR address,
+			 struct ui_file *stream, int recurse,
+			 const struct value_print_options *options);
 
     /* Print a top-level value using syntax appropriate for this language. */
 
Index: python/py-prettyprint.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-prettyprint.c,v
retrieving revision 1.4
diff -u -r1.4 py-prettyprint.c
--- python/py-prettyprint.c	14 Jan 2010 08:03:37 -0000	1.4
+++ python/py-prettyprint.c	29 Jan 2010 16:16:46 -0000
@@ -538,7 +538,8 @@
   /* Instantiate the printer.  */
   if (valaddr)
     valaddr += embedded_offset;
-  value = value_from_contents_and_address (type, valaddr, address);
+  value = value_from_contents_and_address (type, valaddr,
+					   address + embedded_offset);
 
   val_obj = value_to_value_object (value);
   if (! val_obj)
Index: testsuite/gdb.cp/userdef.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/userdef.exp,v
retrieving revision 1.12
diff -u -r1.12 userdef.exp
--- testsuite/gdb.cp/userdef.exp	1 Jan 2010 07:32:02 -0000	1.12
+++ testsuite/gdb.cp/userdef.exp	29 Jan 2010 16:16:48 -0000
@@ -153,7 +153,7 @@
 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 Member {\[\r\n \]+int z;\[\r\n\]+} &\\*"
+gdb_test "ptype &*c" "type = (struct|class) Member {(\[\r\n \]+public:)?\[\r\n \]+int z;\[\r\n\]+} &\\*"
 
 gdb_exit
 return 0
Index: testsuite/gdb.cp/virtbase.cc
===================================================================
RCS file: testsuite/gdb.cp/virtbase.cc
diff -N testsuite/gdb.cp/virtbase.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/virtbase.cc	29 Jan 2010 16:16:48 -0000
@@ -0,0 +1,57 @@
+// This first batch of classes are for PR 11226.
+namespace mc {
+  class Base {
+  protected:
+    int x;
+    
+  public:
+    Base(void) { x = 2; };
+  };
+}
+
+namespace ph {
+  class Middle: public virtual mc::Base {
+  protected:
+    int y;
+    
+  public:
+    Middle(void): mc::Base() { y = 3; };
+
+    int get_y(void)
+    {
+      return y;			// breakpoint 1
+    };
+  };
+
+  class Derived: public virtual Middle {
+  protected:
+    int z;
+    
+  public:
+    Derived(void): Middle() { z = 4; };
+
+    int get_z(void)
+    {
+      return z;			// breakpoint 2
+    };
+  };
+}
+
+// These classes are for PR 9629.
+struct A {};
+struct B : virtual A {};
+
+struct C {int v; C() {v=11;};};
+struct D:virtual C{};
+
+class E:B,D{};
+
+int main() {
+  ph::Derived tst;
+  tst.get_y();
+  tst.get_z();
+
+  E *e = new E;
+
+  return 0;			// breakpoint 3
+}
Index: testsuite/gdb.cp/virtbase.exp
===================================================================
RCS file: testsuite/gdb.cp/virtbase.exp
diff -N testsuite/gdb.cp/virtbase.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/virtbase.exp	29 Jan 2010 16:16:48 -0000
@@ -0,0 +1,57 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+if { [skip_cplus_tests] } { continue }
+
+set testfile "virtbase"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+     untested virtbase.exp
+     return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if {![runto_main]} then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "breakpoint 1"]
+gdb_continue_to_breakpoint "first breakpoint"
+
+# In PR 11226, we failed to print x correctly in the "print *this"
+# case.
+gdb_test "print *this" " = {<mc::Base> = {x = 2}, _vptr.Middle = $hex, y = 3}"
+gdb_test "print x" " = 2"
+
+gdb_breakpoint [gdb_get_line_number "breakpoint 2"]
+gdb_continue_to_breakpoint "second breakpoint"
+
+# In PR 11226, we could not find x here.
+gdb_test "print x" " = 2"
+
+gdb_breakpoint [gdb_get_line_number "breakpoint 3"]
+gdb_continue_to_breakpoint "third breakpoint"
+
+# In PR 9629, we failed to print v correctly here.
+gdb_test "print *(D *) e" " = {<C> = {v = 11}, _vptr.D = $hex}"

Re: RFC: fix virtual base class bugs

[Tom Tromey]

>>>>> "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\]*}"

Re: RFC: fix virtual base class bugs

[Tom Tromey]

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

I checked this in.

Tom

Re: RFC: fix virtual base class bugs

[Tom Tromey]

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

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

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

I've committed this as well.

Tom

Re: RFC: fix virtual base class bugs

[Jan Kratochvil]

On Tue, 02 Feb 2010 17:45:33 +0100, Tom Tromey wrote:
> >>>>> "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.
> 
> I checked this in.

On x86_64-unknown-linux-gnu (Fedora 12, gcc-c++-4.4.2-20.fc12.x86_64):

Running ./gdb.cp/virtbase.exp ...
PASS: gdb.cp/virtbase.exp: continue to breakpoint: first breakpoint
PASS: gdb.cp/virtbase.exp: print *this
FAIL: gdb.cp/virtbase.exp: print x
PASS: gdb.cp/virtbase.exp: continue to breakpoint: second breakpoint
PASS: gdb.cp/virtbase.exp: print x
PASS: gdb.cp/virtbase.exp: continue to breakpoint: third breakpoint

 print x
-$2 = 2
-(gdb) PASS: gdb.cp/virtbase.exp: print x
+$2 = 4197400
+(gdb) FAIL: gdb.cp/virtbase.exp: print x

("print x" name could be unique)

Unaware of the reason.


Regards,
Jan

Re: RFC: fix virtual base class bugs

[Tom Tromey]

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

Jan> FAIL: gdb.cp/virtbase.exp: print x

Thanks.  I reproduced this on the compile farm and I am debugging it
now.

Tom