Re: [rfc][19/37] Eliminate builtin_type_ macros: Ada range type handling

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Joel Brobecker wrote

> > two more small changes to ada-lang.c:  ada_array_length would simply use
> > builtin_type_int as type of its return value, even though it should probably
> > better use the type of the array bounds.
> 
> I don't think this part is correct, since the bounds can be any discrete
> type, which includes enumeration types. The 'Length attribute is defined
> as returning a universal_integer. The definition of universal_integer
> is a little complicated for me, but it is an integer.

I see; it seems I have misinterpreted range types -- I though they were always
based on an integer type.

> For this hunk:
> 
> > @@ -2633,12 +2633,13 @@ ada_array_length (struct value *arr, int
> >        return value_from_longest (type, v);
> >      }
> >    else
> > -    return
> > -      value_from_longest (builtin_type_int,
> > -                          value_as_long (desc_one_bound (desc_bounds (arr),
> > -                                                         n, 1))
> > -                          - value_as_long (desc_one_bound (desc_bounds (arr),
> > -                                                           n, 0)) + 1);
> > +    {
> > +      struct value *high = desc_one_bound (desc_bounds (arr), n, 1);
> > +      struct value *low = desc_one_bound (desc_bounds (arr), n, 0);
> > +      return value_from_longest (value_type (high),
> > +				 value_as_long (high)
> > +				 - value_as_long (low) + 1);
> 
> In this case, I propose we use builtin_type_int32 as you have done
> in an earlier patch, or maybe builtin_type_uint32 since the length
> is always positive. Not ideal, but I don't think it's worse than
> what we have been doing so far (there might be some 64-bit architectures
> where Integer'Length is 64-bit, so technically we might be doing worse,
> but again, i don't think we're going to see arrays that large - hopefully
> I won't stand corrected, but I propose to worry about it then).
> 
> Another option is to pass the int type as a parameter, since I believe
> it is indirectly accessible form the caller (ada_evaluate_subexp).
> We can extract the gdbarch from the expression, and then get the
> int type from there. For some reason, I find it annoying to have
> to pass the type, and this approach annoys me as much as the current
> approach. Since this is also more work, I would simply leave it as
> a builtin_type_int32 for now.

OK, fine with me.

> >          case TYPE_CODE_RANGE:
> > -          arg2 = value_from_longest (builtin_type_int, TYPE_LOW_BOUND (type));
> > -          arg3 = value_from_longest (builtin_type_int,
> > -                                     TYPE_HIGH_BOUND (type));
> > +	  arg2 = value_from_longest (TYPE_TARGET_TYPE (type),
> > +				     TYPE_LOW_BOUND (type));
> > +	  arg3 = value_from_longest (TYPE_TARGET_TYPE (type),
> > +				     TYPE_HIGH_BOUND (type));
> >  	  binop_promote (exp->language_defn, exp->gdbarch, &arg1, &arg2);
> >  	  binop_promote (exp->language_defn, exp->gdbarch, &arg1, &arg3);
> >            type = language_bool_type (exp->language_defn, exp->gdbarch);
> 
> I don't really understand why the TYPE_TARGET_TYPE is necessarily
> an integer type. I don't even think that the TYPE_TARGET_TYPE
> is necessarily set, particularly in the case of enumerated types
> or base types.
> 
> We have access to the int type through the expression in this case.
> Can we use that?

I guess so, but it would appear this doesn't really help.  There isn't
really anything in the language that says the gdbarch's int type is the
"correct" type to use here, or is there?

If not, and this is just about using some "random" integer type to perform
the comparison in, it seems to me using builtin_type_int32 here as well
would be better ...

Bye,
Ulrich

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