Re: [PATCH 3/4] gdb: add type::length / type::set_length

[Lancelot SIX <lsix@lancelotsix.com>]

Hi,

On Fri, Sep 16, 2022 at 11:08:35AM -0400, Simon Marchi via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> Add the `length` and `set_length` methods on `struct type`, in order to remove
> the `TYPE_LENGTH` macro.  In this patch, the macro is changed to use the
> getter, so all the call sites of the macro that are used as a setter are
> changed to use the setter method directly.  The next patch will remove the
> macro completely.
> 
> Change-Id: Id1090244f15c9856969b9be5006aefe8d8897ca4
> ---
>  gdb/ada-lang.c            | 47 +++++++++++++-------------
>  gdb/coffread.c            | 14 ++++----
>  gdb/cp-valprint.c         |  2 +-
>  gdb/ctfread.c             | 10 +++---
>  gdb/dwarf2/read.c         | 44 ++++++++++--------------
>  gdb/eval.c                |  2 +-
>  gdb/f-valprint.c          |  6 ++--
>  gdb/gdbtypes.c            | 71 ++++++++++++++++++++-------------------
>  gdb/gdbtypes.h            | 16 +++++++--
>  gdb/mdebugread.c          |  4 +--
>  gdb/opencl-lang.c         |  2 +-
>  gdb/rust-lang.c           |  5 ++-
>  gdb/stabsread.c           | 10 +++---
>  gdb/target-descriptions.c |  2 +-
>  14 files changed, 119 insertions(+), 116 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index d0b13a6bed2..06eab777f57 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -2165,7 +2165,7 @@ ada_type_of_array (struct value *arr, int bounds)
>  		  int array_bitsize =
>  			(hi - lo + 1) * TYPE_FIELD_BITSIZE (elt_type, 0);
>  
> -		  TYPE_LENGTH (array_type) = (array_bitsize + 7) / 8;
> +		  array_type->set_length ((array_bitsize + 7) / 8);

Do you think that while at it you could also replace the 7 and 8
literals with appropriate configured values?

    array_type->set_length ((array_bitsize + HOST_CHAR_BIT - 1)
                            / HOST_CHAR_BIT);

This is not directly linked to what you are changing, might be done in
a separate patch.

>  		}
>  	    }
>  	}
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 843e0efb321..1bead475f5f 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -1307,12 +1307,11 @@ update_static_array_size (struct type *type)
>  	     special, it's still just a single element array) so do
>  	     consider that case when touching this code.  */
>  	  LONGEST element_count = std::abs (high_bound - low_bound + 1);
> -	  TYPE_LENGTH (type)
> -	    = ((std::abs (stride) * element_count) + 7) / 8;
> +	  type->set_length (((std::abs (stride) * element_count) + 7) / 8);

Same here.

>  	}
>        else
> -	TYPE_LENGTH (type) =
> -	  TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
> +	type->set_length (TYPE_LENGTH (element_type)
> +			  * (high_bound - low_bound + 1));
>  
>        /* If this array's element is itself an array with a bit stride,
>  	 then we want to update this array's bit stride to reflect the
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 6f4b2d61ca8..6d12a489b15 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1443,7 +1453,7 @@ struct type
>    bool bit_size_differs_p () const
>    {
>      return (main_type->type_specific_field == TYPE_SPECIFIC_INT
> -	    && main_type->type_specific.int_stuff.bit_size != 8 * length);
> +	    && main_type->type_specific.int_stuff.bit_size != 8 * length ());

Same, HOST_CHAR_BIT * length ()

>    }

Best,
Lancelot.