[PATCH] Remove ada_value_slice

[PATCH] Remove ada_value_slice

[Tom Tromey]

ada_value_slice and ada_value_slice_from_ptr are nearly identical.
From what I can tell, the former can be removed without any problem.
---
 gdb/ada-lang.c | 46 ++--------------------------------------------
 1 file changed, 2 insertions(+), 44 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index bb044e2c5a0..153debc6569 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -3183,49 +3183,6 @@ ada_value_slice_from_ptr (struct value *array_ptr, struct type *type,
 }
 
 
-static struct value *
-ada_value_slice (struct value *array, LONGEST low, LONGEST high)
-{
-  struct type *type = ada_check_typedef (array->type ());
-  struct type *base_index_type = type->index_type ()->target_type ();
-  type_allocator alloc (type->index_type ());
-  struct type *index_type
-    = create_static_range_type (alloc, type->index_type (), low, high);
-
-  dynamic_prop prop_storage;
-  dynamic_prop *prop = type->dyn_prop (DYN_PROP_BYTE_STRIDE);
-  bool is_byte_stride = true;
-  if (prop == nullptr)
-    {
-      prop = type->dyn_prop (DYN_PROP_BIT_STRIDE);
-      is_byte_stride = false;
-      if (prop == nullptr)
-	{
-	  prop = &prop_storage;
-	  prop->set_const_val (type->field (0).bitsize ());
-	}
-    }
-
-  struct type *slice_type = create_array_type_with_stride
-			      (alloc, type->target_type (), index_type,
-			       prop, is_byte_stride);
-  std::optional<LONGEST> low_pos, high_pos;
-
-
-  low_pos = discrete_position (base_index_type, low);
-  high_pos = discrete_position (base_index_type, high);
-
-  if (!low_pos.has_value () || !high_pos.has_value ())
-    {
-      warning (_("unable to get positions in slice, use bounds instead"));
-      low_pos = low;
-      high_pos = high;
-    }
-
-  return value_cast (slice_type,
-		     value_slice (array, low, *high_pos - *low_pos + 1));
-}
-
 /* If type is a record type in the form of a standard GNAT array
    descriptor, returns the number of dimensions for type.  If arr is a
    simple array, returns the number of "array of"s that prefix its
@@ -10306,7 +10263,8 @@ ada_ternop_slice_operation::evaluate (struct type *expect_type,
   else if (high_bound < low_bound)
     return empty_array (array->type (), low_bound, high_bound);
   else
-    return ada_value_slice (array, low_bound, high_bound);
+    return ada_value_slice_from_ptr (array, array->type (),
+				     low_bound, high_bound);
 }
 
 /* Implement BINOP_IN_BOUNDS.  */

base-commit: 18ad8c1a54d5d0e3fe3ff6c13f217e0dc840f991
-- 
2.54.0

Re: [PATCH] Remove ada_value_slice

[Andrew Burgess]

Tom Tromey <tromey@adacore.com> writes:

> ada_value_slice and ada_value_slice_from_ptr are nearly identical.
> From what I can tell, the former can be removed without any problem.

I tried running the testsuite adding a print out at the call site where
ada_value_slice used to be called.  I ran gdb.ada/*.exp and
gdb.dwarf2/*.exp and didn't see the call site being hit once.  Which is
not great.

I looked at the history of these two functions, and they are both added
in commit 0b5d8877912034b2af0c548afbb794e4dcd07fbd, so clearly at one
point there was an expectation that these would do different things.  If
they do the same now, then I wonder if that's intentional, or a mistake
because one code path was never used?

However, I'm curious about some of the differences between these two
functions ....

> -
> -  return value_cast (slice_type,
> -		     value_slice (array, low, *high_pos - *low_pos + 1));

In ada_value_slice_from_ptr  the final return is:

  base = value_as_address (array_ptr) + (*low_pos - *base_low_pos) * stride;
  return value_at_lazy (slice_type, base);

The value_as_address call, for an array, will end up calling
coerce_array, which is:

  struct value *
  coerce_array (struct value *arg)
  {
    struct type *type;
  
    arg = coerce_ref (arg);
    type = check_typedef (arg->type ());
  
    switch (type->code ())
      {
      case TYPE_CODE_ARRAY:
        if (!type->is_vector () && current_language->c_style_arrays_p ())
  	arg = value_coerce_array (arg);
        break;
      case TYPE_CODE_FUNC:
        arg = value_coerce_function (arg);
        break;
      }
    return arg;
  }

Notice that for TYPE_CODE_ARRAY we only actually do anything when
c_style_arrays_p() is true, which it isn't for Ada.  So we're not going
to convert the array into a pointer, which means (I think) that the
value_at_lazy isn't going to do what you want.  I suspect this is why
there are two, very similar functions.

What are your thoughts?

Thanks,
Andrew