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

[Daniel Jacobowitz <drow@mvista.com>]

On Wed, Jan 29, 2003 at 03:48:29PM -0700, Kevin Buettner wrote:
> A while back, I introduced a change to dwarf2read.c in
> read_tag_pointer_type() which was supposed to create a pointer type
> variant of a (potentially) different length:
> 
>   if (TYPE_LENGTH (type) != byte_size || addr_class != DW_ADDR_none)
>     {
>       if (ADDRESS_CLASS_TYPE_FLAGS_P ())
> 	{
> 	  int type_flags;
> 
> 	  type_flags = ADDRESS_CLASS_TYPE_FLAGS (byte_size, addr_class);
> 	  gdb_assert ((type_flags & ~TYPE_FLAG_ADDRESS_CLASS_ALL) == 0);
> 	  type = make_type_with_address_space (type, type_flags);
> 	}
>     ...
>     }
> 
>   TYPE_LENGTH (type) = byte_size;
> 
> However, this code doesn't work correctly.  As it stands now, the type
> length is shared between all type variants (which differ only in
> qualifiers).  This means that as soon as the above TYPE_LENGTH
> assignment is performed, all type variants end up getting the same
> length.  I should note that when I initially developed this code, I
> did so on a branch which did not yet implement this sharing via struct
> main_type.
> 
> The patch below corrects this problem by moving the length field from
> struct main_type to struct type.  I am not entirely happy with this
> approach, but the other approaches I've considered are even less
> palatable.
> 
> E.g, another approach that I considered would be to create a new type
> which has a different main_type that varies only in the length field. 
> The problem with this is that the names end up being the same, and it
> seems to me that there will be problems with searching and finding the
> right type when the user specifies it by name.
> 
> Certainly, if anyone can think of a better approach, I'd be happy to
> hear about it.

It won't actually create a problem if you give them different main
types; the main type is purely internal to the type system, and is
never searched directly.

That said, while I'm not really happy with your approach either, I
think it is more correct and less fragile in general.

> Those of you reviewing this patch should carefully consider the
> comment in replace_type().  I don't think the problem that I mention
> there will actually arise since replace_type() is only called by symbol
> readers which don't know how to (and indeed can't) create variants of
> different sizes, so the situation described in the replace_type() comment
> should never (at the moment anyway) arise.
> 
> Comments?  (I'll wait at least a week before checking this one in.)

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?


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer