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

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

Joel Brobecker wrote:

> So, for ada_array_length, shall use use a type of builtin_type_int32?
> That's what I did for my testing (more on this in a separate message)

OK, I've changed the patch accordingly.

> > > >          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?
> 
> No, I don't think so. I had a look at the Ada Reference Manual,
> and it doesn't say anything about that.
> 
> > 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 ...
> 
> It just occured to me that, since this is a TYPE_CODE_RANGE, can't
> we just use its type? A TYPE_CODE_RANGE should always be some kind
> of "integer" type, so why not use that when calling value_from_longest?
> I have this awful feeling that we're missing something, but I don't
> see what. Do you see anything wrong with that?

Well, the only effect going through GDB values to perform the comparison has
is to do the appropriate type promotions.  However, if this doesn't actually
apply for Ada, I'm wondering why we don't simply do the comparison on integral
LONGEST values directly, like in the patch below?

Does this make sense?

Bye,
Ulrich

ChangeLog:

	* ada-lang.c (ada_array_length): Use builtin_type_int32 instead
	of builtin_type_int.
	(ada_evaluate_subexp): Perform range check on LONGEST values
	instead of on GDB values.


Index: gdb-head/gdb/ada-lang.c
===================================================================
--- gdb-head.orig/gdb/ada-lang.c
+++ gdb-head/gdb/ada-lang.c
@@ -2623,7 +2623,7 @@ ada_array_length (struct value *arr, int
     }
   else
     return
-      value_from_longest (builtin_type_int,
+      value_from_longest (builtin_type_int32,
                           value_as_long (desc_one_bound (desc_bounds (arr),
                                                          n, 1))
                           - value_as_long (desc_one_bound (desc_bounds (arr),
@@ -8860,18 +8860,13 @@ ada_evaluate_subexp (struct type *expect
           return value_from_longest (type, (LONGEST) 1);
 
         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));
-	  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);
-          return
-            value_from_longest (type,
-                                (value_less (arg1, arg3)
-                                 || value_equal (arg1, arg3))
-                                && (value_less (arg2, arg1)
-                                    || value_equal (arg2, arg1)));
+	  {
+	    LONGEST val = value_as_long (arg1);
+	    LONGEST low = TYPE_LOW_BOUND (type);
+	    LONGEST high = TYPE_HIGH_BOUND (type);
+	    type = language_bool_type (exp->language_defn, exp->gdbarch);
+	    return value_from_longest (type, low <= val && val <= high);
+	  }
         }
 
     case BINOP_IN_BOUNDS:



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