Re: [RFA/Ada] remove ref to fields and ref to slices

Re: [RFA/Ada] remove ref to fields and ref to slices

[Joel Brobecker]

> 2008-11-27  Jerome Guitton  <guitton@adacore.com>
> 
> 	* ada-lang.c (ada_value_slice_ptr): Return a lazy value instead
> 	of a reference. Update comment.
> 	(ada_value_struct_elt): Ditto if arg is a pointer or a reference.
> 	Update comment as well.

OK, but a couple of comments:

>  static struct value *
>  ada_value_slice_ptr (struct value *array_ptr, struct type *type,
>                       int low, int high)

Could you rename this function to "ada_value_slice"? Since we no longer
return a pointer to the slice but the slice itself, the _ptr suffix
could be confusing.

> @@ -6178,9 +6178,7 @@ ada_index_struct_field_1 (int *index_p, 
>  /* Given ARG, a value of type (pointer or reference to a)*
>     structure/union, extract the component named NAME from the ultimate
>     target structure/union and return it as a value with its
> -   appropriate type.  If ARG is a pointer or reference and the field
> -   is not packed, returns a reference to the field, otherwise the
> -   value of the field (an lvalue if ARG is an lvalue).     
> +   appropriate type.  Return the value of the field.

The last sentence you added seems redundant with the one before it
(that you adjusted).

-- 
Joel

Re: [RFA/Ada] remove ref to fields and ref to slices

[Jerome Guitton]

Joel Brobecker (brobecker@adacore.com):

> >  static struct value *
> >  ada_value_slice_ptr (struct value *array_ptr, struct type *type,
> >                       int low, int high)
> 
> Could you rename this function to "ada_value_slice"? Since we no longer
> return a pointer to the slice but the slice itself, the _ptr suffix
> could be confusing.

Not really. ada_value_slice already exists: same function, but it works
on array. ada_value_slice_ptr, at the contrary, works on pointers to
array or references. Maybe ada_value_slice_from_ptr would be clearer?



> 
> > @@ -6178,9 +6178,7 @@ ada_index_struct_field_1 (int *index_p, 
> >  /* Given ARG, a value of type (pointer or reference to a)*
> >     structure/union, extract the component named NAME from the ultimate
> >     target structure/union and return it as a value with its
> > -   appropriate type.  If ARG is a pointer or reference and the field
> > -   is not packed, returns a reference to the field, otherwise the
> > -   value of the field (an lvalue if ARG is an lvalue).     
> > +   appropriate type.  Return the value of the field.
> 
> The last sentence you added seems redundant with the one before it
> (that you adjusted).

Right. Thank you for catching it.

Re: [RFA/Ada] remove ref to fields and ref to slices

[Joel Brobecker]

> Not really. ada_value_slice already exists: same function, but it works
> on array. ada_value_slice_ptr, at the contrary, works on pointers to
> array or references. Maybe ada_value_slice_from_ptr would be clearer?

Rats! I'm looking at the code, and I am thinking that there is a bit
of simplification that we could look at, now that the contents of lazy
values is allocated later. For instance:

ada_coerce_to_simple_array_ptr vs ada_coerce_to_simple_array

If we got rid of ada_coerce_to_simple_array_ptr, then we probably
wouldn't need ada_value_slice_ptr..

But in the meantime, I do think that ada_value_slice_from_ptr is
clearer.

Thanks,
-- 
Joel

Re: [RFA/Ada] remove ref to fields and ref to slices

[Jerome Guitton]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

Joel Brobecker (brobecker@adacore.com):

> Rats! I'm looking at the code, and I am thinking that there is a bit
> of simplification that we could look at, now that the contents of lazy
> values is allocated later. For instance:
> 
> ada_coerce_to_simple_array_ptr vs ada_coerce_to_simple_array
> 
> If we got rid of ada_coerce_to_simple_array_ptr, then we probably
> wouldn't need ada_value_slice_ptr..
> 
> But in the meantime, I do think that ada_value_slice_from_ptr is
> clearer.


Here is a new patch, taking the comments into account. I'll work on
the rest of the simplification on the coming month. In the meantime,
and if you're OK, I'll commit this first change.







[-- Attachment #2: ref_fields.diff --]
[-- Type: text/x-diff, Size: 3185 bytes --]

Index: ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.184
diff -u -p -r1.184 ada-lang.c
--- ada-lang.c	24 Nov 2008 23:21:15 -0000	1.184
+++ ada-lang.c	12 Dec 2008 11:58:42 -0000
@@ -2346,12 +2346,12 @@ ada_value_ptr_subscript (struct value *a
 }
 
 /* Given that ARRAY_PTR is a pointer or reference to an array of type TYPE (the
-   actual type of ARRAY_PTR is ignored), returns a reference to
-   the Ada slice of HIGH-LOW+1 elements starting at index LOW.  The lower
-   bound of this array is LOW, as per Ada rules. */
+   actual type of ARRAY_PTR is ignored), returns the Ada slice of HIGH-LOW+1
+   elements starting at index LOW.  The lower bound of this array is LOW, as
+   per Ada rules. */
 static struct value *
-ada_value_slice_ptr (struct value *array_ptr, struct type *type,
-                     int low, int high)
+ada_value_slice_from_ptr (struct value *array_ptr, struct type *type,
+                          int low, int high)
 {
   CORE_ADDR base = value_as_address (array_ptr)
     + ((low - TYPE_LOW_BOUND (TYPE_INDEX_TYPE (type)))
@@ -2361,7 +2361,7 @@ ada_value_slice_ptr (struct value *array
                        low, high);
   struct type *slice_type =
     create_array_type (NULL, TYPE_TARGET_TYPE (type), index_type);
-  return value_from_pointer (lookup_reference_type (slice_type), base);
+  return value_at_lazy (slice_type, base);
 }
 
 
@@ -6178,9 +6178,7 @@ ada_index_struct_field_1 (int *index_p, 
 /* Given ARG, a value of type (pointer or reference to a)*
    structure/union, extract the component named NAME from the ultimate
    target structure/union and return it as a value with its
-   appropriate type.  If ARG is a pointer or reference and the field
-   is not packed, returns a reference to the field, otherwise the
-   value of the field (an lvalue if ARG is an lvalue).     
+   appropriate type.
 
    The routine searches for NAME among all members of the structure itself
    and (recursively) among all members of any wrapper members
@@ -6257,8 +6255,7 @@ ada_value_struct_elt (struct value *arg,
                                                   field_type);
             }
           else
-            v = value_from_pointer (lookup_reference_type (field_type),
-                                    address + byte_offset);
+            v = value_at_lazy (field_type, address + byte_offset);
         }
     }
 
@@ -8808,9 +8805,9 @@ ada_evaluate_subexp (struct type *expect
                 struct type *arr_type0 =
                   to_fixed_array_type (TYPE_TARGET_TYPE (value_type (array)),
                                        NULL, 1);
-                return ada_value_slice_ptr (array, arr_type0,
-                                            longest_to_int (low_bound),
-					    longest_to_int (high_bound));
+                return ada_value_slice_from_ptr (array, arr_type0,
+                                                 longest_to_int (low_bound),
+                                                 longest_to_int (high_bound));
               }
           }
         else if (noside == EVAL_AVOID_SIDE_EFFECTS)

Re: [RFA/Ada] remove ref to fields and ref to slices

[Joel Brobecker]

> Here is a new patch, taking the comments into account. I'll work on
> the rest of the simplification on the coming month. In the meantime,
> and if you're OK, I'll commit this first change.

Looks great! Please commit.

Thank you,
-- 
Joel

Re: [RFA/Ada] remove ref to fields and ref to slices

[Jerome Guitton]

Joel Brobecker (brobecker@adacore.com):

> > Here is a new patch, taking the comments into account. I'll work on
> > the rest of the simplification on the coming month. In the meantime,
> > and if you're OK, I'll commit this first change.
> 
> Looks great! Please commit.

Now committed; thank you.