[2/2] Handle indexing Ada arrays with enum indices

Message ID 20200519171808.2637-3-tromey@adacore.com
State New
Headers show
Series
  • Fix some Ada enum bugs
Related show

Commit Message

Tom Tromey May 19, 2020, 5:18 p.m.
In Ada, like C, an enum can assign values to the constants.  However,
unlike C (or any other language supported by gdb), the enum type can
also be used as the range of an array.

In this case, the user's code references the enum constants, but the
compiler translates these to the position of the constant in the enum.
So for example one might write:

   type Enum_With_Gaps is
     (
      LIT0,
      LIT1,
      LIT2,
      LIT3,
      LIT4
     );

   for Enum_With_Gaps use
     (
      LIT0 => 3,
      LIT1 => 5,
      LIT2 => 8,
      LIT3 => 13,
      LIT4 => 21
     );

Then index an array like "array(LIT3)" -- but this will be the 4th
element in an array of 5 elements, not the 13th element in an array of
19 (assuming I did the math right) elements.

gdb supports this to some degree, with the only missing piece being
indexing into such an array.  This patch implements this missing
feature, and also fixes an existing bug, which is that in some
situations I believe gdb would mis-compute the resulting array's
length.

The approach taken here is to try to integrate this feature into the
core of gdb.  My view is that much of the Ada support should be better
integrated with gdb, rather than being "on the side".  This, I think,
would help avoid code duplication at least.  So, I try to take steps
toward this goal when possible.

Because other languages generally don't allow the user to specify the
index type of an array, I simply made the core of gdb unconditionally
apply discrete_position when computing the range of such an array.
This is a no-op for ordinary types, but applies the enum
value-to-position transformation for TYPE_CODE_ENUM.

gdb/ChangeLog
2020-05-19  Tom Tromey  <tromey@adacore.com>

	* ada-lang.c (ada_print_array_index): Change type.  Call val_atr.
	(ada_value_ptr_subscript): Don't call pos_atr on the lower bound.
	(val_atr): New function.
	(value_val_atr): Use it.
	* ada-valprint.c (print_optional_low_bound): Change low bound
	handling for enums.
	(val_print_packed_array_elements): Don't call discrete_position.
	* gdbtypes.c (get_discrete_bounds) <TYPE_CODE_RANGE>: Call
	discrete_position for enum types.
	* language.c (default_print_array_index): Change type.
	* language.h (struct language_defn) <la_print_array_index>: Add
	index_type parameter, change type of index_value.
	(LA_PRINT_ARRAY_INDEX): Add index_type parameter.
	(default_print_array_index): Update.
	* valprint.c (maybe_print_array_index): Don't call
	value_from_longest.  Update.
	(value_print_array_elements): Don't call discrete_position.

gdb/testsuite/ChangeLog
2020-05-19  Tom Tromey  <tromey@adacore.com>

	* gdb.ada/arr_acc_idx_w_gap.exp: Add tests.
---
 gdb/ChangeLog                               | 20 ++++++++++
 gdb/ada-lang.c                              | 40 +++++++++++--------
 gdb/ada-valprint.c                          | 40 +++++--------------
 gdb/gdbtypes.c                              |  6 +++
 gdb/language.c                              |  5 ++-
 gdb/language.h                              | 10 +++--
 gdb/testsuite/ChangeLog                     |  4 ++
 gdb/testsuite/gdb.ada/arr_acc_idx_w_gap.exp |  3 ++
 gdb/valprint.c                              | 43 ++++++---------------
 9 files changed, 86 insertions(+), 85 deletions(-)

-- 
2.21.1

Patch

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index d8489889c85..08d844218f1 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -196,6 +196,8 @@  static LONGEST pos_atr (struct value *);
 
 static struct value *value_pos_atr (struct type *, struct value *);
 
+static struct value *val_atr (struct type *, LONGEST);
+
 static struct value *value_val_atr (struct type *, struct value *);
 
 static struct symbol *standard_lookup (const char *, const struct block *,
@@ -498,9 +500,12 @@  ada_get_gdb_completer_word_break_characters (void)
 /* Print an array element index using the Ada syntax.  */
 
 static void
-ada_print_array_index (struct value *index_value, struct ui_file *stream,
+ada_print_array_index (struct type *index_type, LONGEST index,
+		       struct ui_file *stream,
                        const struct value_print_options *options)
 {
+  struct value *index_value = val_atr (index_type, index);
+
   LA_VALUE_PRINT (index_value, stream, options);
   fprintf_filtered (stream, " => ");
 }
@@ -2780,15 +2785,13 @@  ada_value_ptr_subscript (struct value *arr, int arity, struct value **ind)
   for (k = 0; k < arity; k += 1)
     {
       LONGEST lwb, upb;
-      struct value *lwb_value;
 
       if (type->code () != TYPE_CODE_ARRAY)
         error (_("too many subscripts (%d expected)"), k);
       arr = value_cast (lookup_pointer_type (TYPE_TARGET_TYPE (type)),
                         value_copy (arr));
       get_discrete_bounds (TYPE_INDEX_TYPE (type), &lwb, &upb);
-      lwb_value = value_from_longest (value_type (ind[k]), lwb);
-      arr = value_ptradd (arr, pos_atr (ind[k]) - pos_atr (lwb_value));
+      arr = value_ptradd (arr, pos_atr (ind[k]) - lwb);
       type = TYPE_TARGET_TYPE (type);
     }
 
@@ -9143,26 +9146,29 @@  value_pos_atr (struct type *type, struct value *arg)
 /* Evaluate the TYPE'VAL attribute applied to ARG.  */
 
 static struct value *
-value_val_atr (struct type *type, struct value *arg)
+val_atr (struct type *type, LONGEST val)
 {
-  if (!discrete_type_p (type))
-    error (_("'VAL only defined on discrete types"));
-  if (!integer_type_p (value_type (arg)))
-    error (_("'VAL requires integral argument"));
-
+  gdb_assert (discrete_type_p (type));
   if (type->code () == TYPE_CODE_RANGE)
     type = TYPE_TARGET_TYPE (type);
-
   if (type->code () == TYPE_CODE_ENUM)
     {
-      long pos = value_as_long (arg);
-
-      if (pos < 0 || pos >= TYPE_NFIELDS (type))
+      if (val < 0 || val >= TYPE_NFIELDS (type))
         error (_("argument to 'VAL out of range"));
-      return value_from_longest (type, TYPE_FIELD_ENUMVAL (type, pos));
+      val = TYPE_FIELD_ENUMVAL (type, val);
     }
-  else
-    return value_from_longest (type, value_as_long (arg));
+  return value_from_longest (type, val);
+}
+
+static struct value *
+value_val_atr (struct type *type, struct value *arg)
+{
+  if (!discrete_type_p (type))
+    error (_("'VAL only defined on discrete types"));
+  if (!integer_type_p (value_type (arg)))
+    error (_("'VAL requires integral argument"));
+
+  return val_atr (type, value_as_long (arg));
 }
 
 
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index e11e47ee59e..f25f6e3ddfe 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -92,8 +92,9 @@  print_optional_low_bound (struct ui_file *stream, struct type *type,
 	return 0;
       break;
     case TYPE_CODE_ENUM:
-      if (low_bound == TYPE_FIELD_ENUMVAL (index_type, 0))
+      if (low_bound == 0)
 	return 0;
+      low_bound = TYPE_FIELD_ENUMVAL (index_type, low_bound);
       break;
     case TYPE_CODE_UNDEF:
       index_type = NULL;
@@ -134,47 +135,24 @@  val_print_packed_array_elements (struct type *type, const gdb_byte *valaddr,
 
   {
     LONGEST high;
-    struct type *base_index_type;
 
     if (get_discrete_bounds (index_type, &low, &high) < 0)
       len = 1;
-    else
-      len = high - low + 1;
-
-    if (index_type->code () == TYPE_CODE_RANGE)
-        base_index_type = TYPE_TARGET_TYPE (index_type);
-      else
-        base_index_type = index_type;
-
-    if (base_index_type->code () == TYPE_CODE_ENUM)
+    else if (low > high)
       {
-        LONGEST low_pos, high_pos;
-
-        /* Non-contiguous enumerations types can by used as index types
-           so the array length is computed from the positions of the
-           first and last literal in the enumeration type, and not from
-           the values of these literals.  */
-
-        if (!discrete_position (base_index_type, low, &low_pos)
-          || !discrete_position (base_index_type, high, &high_pos))
-          {
-            warning (_("unable to get positions in array, use bounds instead"));
-            low_pos = low;
-            high_pos = high;
-          }
-
         /* The array length should normally be HIGH_POS - LOW_POS + 1.
            But in Ada we allow LOW_POS to be greater than HIGH_POS for
            empty arrays.  In that situation, the array length is just zero,
            not negative!  */
-
-        if (low_pos > high_pos)
-          len = 0;
-        else
-          len = high_pos - low_pos + 1;
+	len = 0;
       }
+    else
+      len = high - low + 1;
   }
 
+  if (index_type->code () == TYPE_CODE_RANGE)
+    index_type = TYPE_TARGET_TYPE (index_type);
+
   i = 0;
   annotate_array_section_begin (i, elttype);
 
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 83490eaf8ba..af013ad85e2 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1038,6 +1038,12 @@  get_discrete_bounds (struct type *type, LONGEST *lowp, LONGEST *highp)
     case TYPE_CODE_RANGE:
       *lowp = TYPE_LOW_BOUND (type);
       *highp = TYPE_HIGH_BOUND (type);
+      if (TYPE_TARGET_TYPE (type)->code () == TYPE_CODE_ENUM)
+	{
+	  if (!discrete_position (TYPE_TARGET_TYPE (type), *lowp, lowp)
+	      || ! discrete_position (TYPE_TARGET_TYPE (type), *highp, highp))
+	    return 0;
+	}
       return 1;
     case TYPE_CODE_ENUM:
       if (TYPE_NFIELDS (type) > 0)
diff --git a/gdb/language.c b/gdb/language.c
index 732a69721ca..ff76ae7ece6 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -671,9 +671,12 @@  default_word_break_characters (void)
 /* Print the index of array elements using the C99 syntax.  */
 
 void
-default_print_array_index (struct value *index_value, struct ui_file *stream,
+default_print_array_index (struct type *index_type, LONGEST index,
+			   struct ui_file *stream,
 			   const struct value_print_options *options)
 {
+  struct value *index_value = value_from_longest (index_type, index);
+
   fprintf_filtered (stream, "[");
   LA_VALUE_PRINT (index_value, stream, options);
   fprintf_filtered (stream, "] = ");
diff --git a/gdb/language.h b/gdb/language.h
index ea8aae511b0..e112a91ec57 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -376,7 +376,8 @@  struct language_defn
 				   struct language_arch_info *);
 
     /* Print the index of an element of an array.  */
-    void (*la_print_array_index) (struct value *index_value,
+    void (*la_print_array_index) (struct type *index_type,
+				  LONGEST index_value,
                                   struct ui_file *stream,
                                   const struct value_print_options *options);
 
@@ -570,8 +571,9 @@  extern enum language set_language (enum language);
 #define LA_EMIT_CHAR(ch, type, stream, quoter) \
   (current_language->la_emitchar(ch, type, stream, quoter))
 
-#define LA_PRINT_ARRAY_INDEX(index_value, stream, options) \
-  (current_language->la_print_array_index(index_value, stream, options))
+#define LA_PRINT_ARRAY_INDEX(index_type, index_value, stream, options)	\
+  (current_language->la_print_array_index(index_type, index_value, stream, \
+					  options))
 
 #define LA_ITERATE_OVER_SYMBOLS(BLOCK, NAME, DOMAIN, CALLBACK) \
   (current_language->la_iterate_over_symbols (BLOCK, NAME, DOMAIN, CALLBACK))
@@ -634,7 +636,7 @@  extern char *language_class_name_from_physname (const struct language_defn *,
 extern const char *default_word_break_characters (void);
 
 /* Print the index of an array element using the C99 syntax.  */
-extern void default_print_array_index (struct value *index_value,
+extern void default_print_array_index (struct type *index_type, LONGEST index,
                                        struct ui_file *stream,
 				       const struct value_print_options *options);
 
diff --git a/gdb/testsuite/gdb.ada/arr_acc_idx_w_gap.exp b/gdb/testsuite/gdb.ada/arr_acc_idx_w_gap.exp
index c08070ea53c..06eebb07e77 100644
--- a/gdb/testsuite/gdb.ada/arr_acc_idx_w_gap.exp
+++ b/gdb/testsuite/gdb.ada/arr_acc_idx_w_gap.exp
@@ -57,3 +57,6 @@  gdb_test "print s(2..4)" \
 gdb_test "print v" " = lit3"
 gdb_test "print enum_subrange'pos(v)" " = 3"
 gdb_test "print enum_subrange'val(3)" " = lit3"
+
+gdb_test "print indexed_by_enum(lit2)" "43"
+gdb_test "print s(2)" "101 'e'"
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 4bd643e938d..c506f300f1f 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1851,14 +1851,10 @@  maybe_print_array_index (struct type *index_type, LONGEST index,
                          struct ui_file *stream,
 			 const struct value_print_options *options)
 {
-  struct value *index_value;
-
   if (!options->print_array_indexes)
     return; 
     
-  index_value = value_from_longest (index_type, index);
-
-  LA_PRINT_ARRAY_INDEX (index_value, stream, options);
+  LA_PRINT_ARRAY_INDEX (index_type, index, stream, options);
 }
 
 /* See valprint.h.  */
@@ -1871,7 +1867,7 @@  value_print_array_elements (struct value *val, struct ui_file *stream,
 {
   unsigned int things_printed = 0;
   unsigned len;
-  struct type *elttype, *index_type, *base_index_type;
+  struct type *elttype, *index_type;
   unsigned eltlen;
   /* Position of the array element we are examining to see
      whether it is repeated.  */
@@ -1879,43 +1875,26 @@  value_print_array_elements (struct value *val, struct ui_file *stream,
   /* Number of repetitions we have detected so far.  */
   unsigned int reps;
   LONGEST low_bound, high_bound;
-  LONGEST low_pos, high_pos;
 
   struct type *type = check_typedef (value_type (val));
 
   elttype = TYPE_TARGET_TYPE (type);
   eltlen = type_length_units (check_typedef (elttype));
   index_type = TYPE_INDEX_TYPE (type);
+  if (index_type->code () == TYPE_CODE_RANGE)
+    index_type = TYPE_TARGET_TYPE (index_type);
 
   if (get_array_bounds (type, &low_bound, &high_bound))
     {
-      if (index_type->code () == TYPE_CODE_RANGE)
-	base_index_type = TYPE_TARGET_TYPE (index_type);
-      else
-	base_index_type = index_type;
-
-      /* Non-contiguous enumerations types can by used as index types
-	 in some languages (e.g. Ada).  In this case, the array length
-	 shall be computed from the positions of the first and last
-	 literal in the enumeration type, and not from the values
-	 of these literals.  */
-      if (!discrete_position (base_index_type, low_bound, &low_pos)
-	  || !discrete_position (base_index_type, high_bound, &high_pos))
-	{
-	  warning (_("unable to get positions in array, use bounds instead"));
-	  low_pos = low_bound;
-	  high_pos = high_bound;
-	}
-
-      /* The array length should normally be HIGH_POS - LOW_POS + 1.
-         But we have to be a little extra careful, because some languages
-	 such as Ada allow LOW_POS to be greater than HIGH_POS for
-	 empty arrays.  In that situation, the array length is just zero,
-	 not negative!  */
-      if (low_pos > high_pos)
+      /* The array length should normally be HIGH_BOUND - LOW_BOUND +
+         1.  But we have to be a little extra careful, because some
+         languages such as Ada allow LOW_BOUND to be greater than
+         HIGH_BOUND for empty arrays.  In that situation, the array
+         length is just zero, not negative!  */
+      if (low_bound > high_bound)
 	len = 0;
       else
-	len = high_pos - low_pos + 1;
+	len = high_bound - low_bound + 1;
     }
   else
     {