[rfc] Ada simplification: desc_data_type

[rfc] Ada simplification: desc_data_type

[Ulrich Weigand]

Hello,

another problematic use of lookup_pointer_type in Ada architecture-independent
code: desc_data_type is supposed to return a pointer type, but doesn't really
know which architecture to use to determine pointer size.

Fortunately, all but one callers of that routine immediately dereference the
pointer again.  The one instance that actually uses the pointer at least
operates on a *value*, which is already architecture-dependent ...

The patch replaces desc_data_type with a new "desc_data_target_type" routine
which always returns the pointed-to type.  Only thin_data_pntr then does the
lookup_pointer_type itself.

Overall, this patch also should have no change in behaviour.

Joel, would this be OK?

Bye,
Ulrich


ChangeLog:

	* ada-lang.c (desc_data_type): Remove, replace by ...
	(desc_data_target_type): ... this.
	(thin_data_pntr): Use desc_data_target_type instead of desc_data_type.
	(ada_is_array_descriptor_type): Likewise.
	(ada_type_of_array): Likewise.
	(ada_coerce_to_simple_array_type): Likewise.
	(ada_array_element_type): Likewise.


Index: gdb-head/gdb/ada-lang.c
===================================================================
--- gdb-head.orig/gdb/ada-lang.c
+++ gdb-head/gdb/ada-lang.c
@@ -79,7 +79,7 @@ static int fat_pntr_bounds_bitpos (struc
 
 static int fat_pntr_bounds_bitsize (struct type *);
 
-static struct type *desc_data_type (struct type *);
+static struct type *desc_data_target_type (struct type *);
 
 static struct value *desc_data (struct value *);
 
@@ -1281,11 +1281,13 @@ static struct value *
 thin_data_pntr (struct value *val)
 {
   struct type *type = value_type (val);
+  struct type *data_type = desc_data_target_type (thin_descriptor_type (type));
+  data_type = lookup_pointer_type (data_type);
+
   if (TYPE_CODE (type) == TYPE_CODE_PTR)
-    return value_cast (desc_data_type (thin_descriptor_type (type)),
-                       value_copy (val));
+    return value_cast (data_type, value_copy (val));
   else
-    return value_from_longest (desc_data_type (thin_descriptor_type (type)),
+    return value_from_longest (data_type,
                                VALUE_ADDRESS (val) + value_offset (val));
 }
 
@@ -1389,23 +1391,27 @@ fat_pntr_bounds_bitsize (struct type *ty
 }
 
 /* If TYPE is the type of an array descriptor (fat or thin pointer) or a
-   pointer to one, the type of its array data (a
-   pointer-to-array-with-no-bounds type); otherwise, NULL.  Use
-   ada_type_of_array to get an array type with bounds data.  */
+   pointer to one, the type of its array data (a array-with-no-bounds type);
+   otherwise, NULL.  Use ada_type_of_array to get an array type with bounds
+   data.  */
 
 static struct type *
-desc_data_type (struct type *type)
+desc_data_target_type (struct type *type)
 {
   type = desc_base_type (type);
 
   /* NOTE: The following is bogus; see comment in desc_bounds.  */
   if (is_thin_pntr (type))
-    return lookup_pointer_type
-      (desc_base_type (TYPE_FIELD_TYPE (thin_descriptor_type (type), 1)));
+    return desc_base_type (TYPE_FIELD_TYPE (thin_descriptor_type (type), 1));
   else if (is_thick_pntr (type))
-    return lookup_struct_elt_type (type, "P_ARRAY", 1);
-  else
-    return NULL;
+    {
+      struct type *data_type = lookup_struct_elt_type (type, "P_ARRAY", 1);
+      if (data_type
+	  && TYPE_CODE (ada_check_typedef (data_type)) == TYPE_CODE_PTR)
+	return TYPE_TARGET_TYPE (data_type);
+    }
+
+  return NULL;
 }
 
 /* If ARR is an array descriptor (fat or thin pointer), a pointer to
@@ -1556,17 +1562,14 @@ ada_is_simple_array_type (struct type *t
 int
 ada_is_array_descriptor_type (struct type *type)
 {
-  struct type *data_type = desc_data_type (type);
+  struct type *data_type = desc_data_target_type (type);
 
   if (type == NULL)
     return 0;
   type = ada_check_typedef (type);
   return
     data_type != NULL
-    && ((TYPE_CODE (data_type) == TYPE_CODE_PTR
-         && TYPE_TARGET_TYPE (data_type) != NULL
-         && TYPE_CODE (TYPE_TARGET_TYPE (data_type)) == TYPE_CODE_ARRAY)
-        || TYPE_CODE (data_type) == TYPE_CODE_ARRAY)
+    && TYPE_CODE (data_type) == TYPE_CODE_ARRAY
     && desc_arity (desc_bounds_type (type)) > 0;
 }
 
@@ -1605,7 +1608,7 @@ ada_type_of_array (struct value *arr, in
 
   if (!bounds)
     return
-      ada_check_typedef (TYPE_TARGET_TYPE (desc_data_type (value_type (arr))));
+      ada_check_typedef (desc_data_target_type (value_type (arr)));
   else
     {
       struct type *elt_type;
@@ -1693,7 +1696,7 @@ ada_coerce_to_simple_array_type (struct 
     return decode_packed_array_type (type);
 
   if (ada_is_array_descriptor_type (type))
-    return ada_check_typedef (TYPE_TARGET_TYPE (desc_data_type (type)));
+    return ada_check_typedef (desc_data_target_type (type));
 
   return type;
 }
@@ -2377,7 +2380,7 @@ ada_array_element_type (struct type *typ
       int k;
       struct type *p_array_type;
 
-      p_array_type = desc_data_type (type);
+      p_array_type = desc_data_target_type (type);
 
       k = ada_array_arity (type);
       if (k == 0)
@@ -2386,7 +2389,6 @@ ada_array_element_type (struct type *typ
       /* Initially p_array_type = elt_type(*)[]...(k times)...[].  */
       if (nindices >= 0 && k > nindices)
         k = nindices;
-      p_array_type = TYPE_TARGET_TYPE (p_array_type);
       while (k > 0 && p_array_type != NULL)
         {
           p_array_type = ada_check_typedef (TYPE_TARGET_TYPE (p_array_type));
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Ada simplification: desc_data_type

[Joel Brobecker]

> 	* ada-lang.c (desc_data_type): Remove, replace by ...
> 	(desc_data_target_type): ... this.
> 	(thin_data_pntr): Use desc_data_target_type instead of desc_data_type.
> 	(ada_is_array_descriptor_type): Likewise.
> 	(ada_type_of_array): Likewise.
> 	(ada_coerce_to_simple_array_type): Likewise.
> 	(ada_array_element_type): Likewise.

Yes, that looks good to me.

Would you mind making a couple of tiny adjustments for me, please?

> +    {
> +      struct type *data_type = lookup_struct_elt_type (type, "P_ARRAY", 1);
> +      if (data_type
> +	  && TYPE_CODE (ada_check_typedef (data_type)) == TYPE_CODE_PTR)
> +	return TYPE_TARGET_TYPE (data_type);
> +    }
> +
> +  return NULL;
>  }

Can you add an empty line after the declaration part? Mark usually
requests us to follow this style, and I like it, so...

>    return
>      data_type != NULL
> -    && ((TYPE_CODE (data_type) == TYPE_CODE_PTR
> -         && TYPE_TARGET_TYPE (data_type) != NULL
> -         && TYPE_CODE (TYPE_TARGET_TYPE (data_type)) == TYPE_CODE_ARRAY)
> -        || TYPE_CODE (data_type) == TYPE_CODE_ARRAY)
> +    && TYPE_CODE (data_type) == TYPE_CODE_ARRAY
>      && desc_arity (desc_bounds_type (type)) > 0;
>  }

Can you bracket the return condition in between parens? This is not
strictly necessary, but I remember reading a coding style document
that suggests it in order to help formatters such as GNU indent.

Independently of gdbarch dependency, the code does look nicer this way.

-- 
Joel

Re: [rfc] Ada simplification: desc_data_type

[Ulrich Weigand]

Joel Brobecker wrote:
> > 	* ada-lang.c (desc_data_type): Remove, replace by ...
> > 	(desc_data_target_type): ... this.
> > 	(thin_data_pntr): Use desc_data_target_type instead of desc_data_type.
> > 	(ada_is_array_descriptor_type): Likewise.
> > 	(ada_type_of_array): Likewise.
> > 	(ada_coerce_to_simple_array_type): Likewise.
> > 	(ada_array_element_type): Likewise.
> 
> Yes, that looks good to me.
> 
> Would you mind making a couple of tiny adjustments for me, please?

Sure -- here's the version I've checked in.

Thanks,
Ulrich


ChangeLog:

	* ada-lang.c (desc_data_type): Remove, replace by ...
	(desc_data_target_type): ... this.
	(thin_data_pntr): Use desc_data_target_type instead of desc_data_type.
	(ada_is_array_descriptor_type): Likewise.
	(ada_type_of_array): Likewise.
	(ada_coerce_to_simple_array_type): Likewise.
	(ada_array_element_type): Likewise.


Index: gdb-head/gdb/ada-lang.c
===================================================================
--- gdb-head.orig/gdb/ada-lang.c
+++ gdb-head/gdb/ada-lang.c
@@ -79,7 +79,7 @@ static int fat_pntr_bounds_bitpos (struc
 
 static int fat_pntr_bounds_bitsize (struct type *);
 
-static struct type *desc_data_type (struct type *);
+static struct type *desc_data_target_type (struct type *);
 
 static struct value *desc_data (struct value *);
 
@@ -1281,11 +1281,13 @@ static struct value *
 thin_data_pntr (struct value *val)
 {
   struct type *type = value_type (val);
+  struct type *data_type = desc_data_target_type (thin_descriptor_type (type));
+  data_type = lookup_pointer_type (data_type);
+
   if (TYPE_CODE (type) == TYPE_CODE_PTR)
-    return value_cast (desc_data_type (thin_descriptor_type (type)),
-                       value_copy (val));
+    return value_cast (data_type, value_copy (val));
   else
-    return value_from_longest (desc_data_type (thin_descriptor_type (type)),
+    return value_from_longest (data_type,
                                VALUE_ADDRESS (val) + value_offset (val));
 }
 
@@ -1389,23 +1391,28 @@ fat_pntr_bounds_bitsize (struct type *ty
 }
 
 /* If TYPE is the type of an array descriptor (fat or thin pointer) or a
-   pointer to one, the type of its array data (a
-   pointer-to-array-with-no-bounds type); otherwise, NULL.  Use
-   ada_type_of_array to get an array type with bounds data.  */
+   pointer to one, the type of its array data (a array-with-no-bounds type);
+   otherwise, NULL.  Use ada_type_of_array to get an array type with bounds
+   data.  */
 
 static struct type *
-desc_data_type (struct type *type)
+desc_data_target_type (struct type *type)
 {
   type = desc_base_type (type);
 
   /* NOTE: The following is bogus; see comment in desc_bounds.  */
   if (is_thin_pntr (type))
-    return lookup_pointer_type
-      (desc_base_type (TYPE_FIELD_TYPE (thin_descriptor_type (type), 1)));
+    return desc_base_type (TYPE_FIELD_TYPE (thin_descriptor_type (type), 1));
   else if (is_thick_pntr (type))
-    return lookup_struct_elt_type (type, "P_ARRAY", 1);
-  else
-    return NULL;
+    {
+      struct type *data_type = lookup_struct_elt_type (type, "P_ARRAY", 1);
+
+      if (data_type
+	  && TYPE_CODE (ada_check_typedef (data_type)) == TYPE_CODE_PTR)
+	return TYPE_TARGET_TYPE (data_type);
+    }
+
+  return NULL;
 }
 
 /* If ARR is an array descriptor (fat or thin pointer), a pointer to
@@ -1556,18 +1563,14 @@ ada_is_simple_array_type (struct type *t
 int
 ada_is_array_descriptor_type (struct type *type)
 {
-  struct type *data_type = desc_data_type (type);
+  struct type *data_type = desc_data_target_type (type);
 
   if (type == NULL)
     return 0;
   type = ada_check_typedef (type);
-  return
-    data_type != NULL
-    && ((TYPE_CODE (data_type) == TYPE_CODE_PTR
-         && TYPE_TARGET_TYPE (data_type) != NULL
-         && TYPE_CODE (TYPE_TARGET_TYPE (data_type)) == TYPE_CODE_ARRAY)
-        || TYPE_CODE (data_type) == TYPE_CODE_ARRAY)
-    && desc_arity (desc_bounds_type (type)) > 0;
+  return (data_type != NULL
+	  && TYPE_CODE (data_type) == TYPE_CODE_ARRAY
+	  && desc_arity (desc_bounds_type (type)) > 0);
 }
 
 /* Non-zero iff type is a partially mal-formed GNAT array
@@ -1605,7 +1608,7 @@ ada_type_of_array (struct value *arr, in
 
   if (!bounds)
     return
-      ada_check_typedef (TYPE_TARGET_TYPE (desc_data_type (value_type (arr))));
+      ada_check_typedef (desc_data_target_type (value_type (arr)));
   else
     {
       struct type *elt_type;
@@ -1693,7 +1696,7 @@ ada_coerce_to_simple_array_type (struct 
     return decode_packed_array_type (type);
 
   if (ada_is_array_descriptor_type (type))
-    return ada_check_typedef (TYPE_TARGET_TYPE (desc_data_type (type)));
+    return ada_check_typedef (desc_data_target_type (type));
 
   return type;
 }
@@ -2377,7 +2380,7 @@ ada_array_element_type (struct type *typ
       int k;
       struct type *p_array_type;
 
-      p_array_type = desc_data_type (type);
+      p_array_type = desc_data_target_type (type);
 
       k = ada_array_arity (type);
       if (k == 0)
@@ -2386,7 +2389,6 @@ ada_array_element_type (struct type *typ
       /* Initially p_array_type = elt_type(*)[]...(k times)...[].  */
       if (nindices >= 0 && k > nindices)
         k = nindices;
-      p_array_type = TYPE_TARGET_TYPE (p_array_type);
       while (k > 0 && p_array_type != NULL)
         {
           p_array_type = ada_check_typedef (TYPE_TARGET_TYPE (p_array_type));

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com