Re: [RFC] Move ``length'' from struct main_type to struct type

[Kevin Buettner <kevinb@redhat.com>]

On Jan 29,  6:15pm, Daniel Jacobowitz wrote:

> My only comment is that I'd rather you check in replace_type that there
> are no types on the variant ring with different space qualifiers, and
> internal_error if there are.  How's that sound?

I like it.  I've appended a revised patch below.

	* gdbtypes.h (struct main_type): Move ``length'' field from here...
	(struct type): ...to here.
	(TYPE_LENGTH): Adjust to reflect different location of ``length''
	field.
	* gdbtypes.c (make_qualified_type): Set length on newly created type.
	(replace_type): Set length on all type variants for a given type.

Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.69
diff -u -p -r1.69 gdbtypes.c
--- gdbtypes.c	17 Jan 2003 19:12:18 -0000	1.69
+++ gdbtypes.c	30 Jan 2003 00:51:04 -0000
@@ -469,6 +469,9 @@ make_qualified_type (struct type *type, 
   /* Now set the instance flags and return the new type.  */
   TYPE_INSTANCE_FLAGS (ntype) = new_flags;
 
+  /* Set length of new type to that of the original type.  */
+  TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+
   return ntype;
 }
 
@@ -556,9 +559,25 @@ make_cv_type (int cnst, int voltl, struc
 void
 replace_type (struct type *ntype, struct type *type)
 {
-  struct type *cv_chain, *as_chain, *ptr, *ref;
+  struct type *chain;
 
   *TYPE_MAIN_TYPE (ntype) = *TYPE_MAIN_TYPE (type);
+
+  /* The type length is not a part of the main type.  Update it for each
+     type on the variant chain.  */
+  chain = ntype;
+  do {
+    /* Assert that this element of the chain has no address-class bits
+       set in its flags.  Such type variants might have type lengths
+       which are supposed to be different from the non-address-class
+       variants.  This assertion shouldn't ever be triggered because
+       symbol readers which do construct address-class variants don't
+       call replace_type().  */
+    gdb_assert (TYPE_ADDRESS_CLASS_ALL (chain) == 0);
+
+    TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+    chain = TYPE_CHAIN (chain);
+  } while (ntype != chain);
 
   /* Assert that the two types have equivalent instance qualifiers.
      This should be true for at least all of our debug readers.  */
Index: gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.42
diff -u -p -r1.42 gdbtypes.h
--- gdbtypes.h	19 Jan 2003 04:06:45 -0000	1.42
+++ gdbtypes.h	30 Jan 2003 00:51:05 -0000
@@ -297,32 +297,6 @@ struct main_type
 
   char *tag_name;
 
-  /* Length of storage for a value of this type.  This is what
-     sizeof(type) would return; use it for address arithmetic,
-     memory reads and writes, etc.  This size includes padding.  For
-     example, an i386 extended-precision floating point value really
-     only occupies ten bytes, but most ABI's declare its size to be
-     12 bytes, to preserve alignment.  A `struct type' representing
-     such a floating-point type would have a `length' value of 12,
-     even though the last two bytes are unused.
-
-     There's a bit of a host/target mess here, if you're concerned
-     about machines whose bytes aren't eight bits long, or who don't
-     have byte-addressed memory.  Various places pass this to memcpy
-     and such, meaning it must be in units of host bytes.  Various
-     other places expect they can calculate addresses by adding it
-     and such, meaning it must be in units of target bytes.  For
-     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
-     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
-
-     One fix would be to make this field in bits (requiring that it
-     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
-     the other choice would be to make it consistently in units of
-     HOST_CHAR_BIT.  However, this would still fail to address
-     machines based on a ternary or decimal representation.  */
-  
-  unsigned length;
-
   /* FIXME, these should probably be restricted to a Fortran-specific
      field in some fashion.  */
 #define BOUND_CANNOT_BE_DETERMINED   5
@@ -489,15 +463,42 @@ struct type
   struct type *reference_type;
 
   /* Variant chain.  This points to a type that differs from this one only
-     in qualifiers.  Currently, the possible qualifiers are const, volatile,
-     code-space, and data-space.  The variants are linked in a circular
-     ring and share MAIN_TYPE.  */
+     in qualifiers and length.  Currently, the possible qualifiers are
+     const, volatile, code-space, data-space, and address class.  The
+     length may differ only when one of the address class flags are set.
+     The variants are linked in a circular ring and share MAIN_TYPE.  */
   struct type *chain;
 
   /* Flags specific to this instance of the type, indicating where
      on the ring we are.  */
   int instance_flags;
 
+  /* Length of storage for a value of this type.  This is what
+     sizeof(type) would return; use it for address arithmetic,
+     memory reads and writes, etc.  This size includes padding.  For
+     example, an i386 extended-precision floating point value really
+     only occupies ten bytes, but most ABI's declare its size to be
+     12 bytes, to preserve alignment.  A `struct type' representing
+     such a floating-point type would have a `length' value of 12,
+     even though the last two bytes are unused.
+
+     There's a bit of a host/target mess here, if you're concerned
+     about machines whose bytes aren't eight bits long, or who don't
+     have byte-addressed memory.  Various places pass this to memcpy
+     and such, meaning it must be in units of host bytes.  Various
+     other places expect they can calculate addresses by adding it
+     and such, meaning it must be in units of target bytes.  For
+     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
+     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
+
+     One fix would be to make this field in bits (requiring that it
+     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
+     the other choice would be to make it consistently in units of
+     HOST_CHAR_BIT.  However, this would still fail to address
+     machines based on a ternary or decimal representation.  */
+  
+  unsigned length;
+
   /* Core type, shared by a group of qualified types.  */
   struct main_type *main_type;
 };
@@ -758,7 +759,7 @@ extern void allocate_cplus_struct_type (
    But check_typedef does set the TYPE_LENGTH of the TYPEDEF type,
    so you only have to call check_typedef once.  Since allocate_value
    calls check_typedef, TYPE_LENGTH (VALUE_TYPE (X)) is safe.  */
-#define TYPE_LENGTH(thistype) TYPE_MAIN_TYPE(thistype)->length
+#define TYPE_LENGTH(thistype) (thistype)->length
 #define TYPE_OBJFILE(thistype) TYPE_MAIN_TYPE(thistype)->objfile
 #define TYPE_FLAGS(thistype) TYPE_MAIN_TYPE(thistype)->flags
 /* Note that TYPE_CODE can be TYPE_CODE_TYPEDEF, so if you want the real