* [rfa/Ada] Restructure array bounds routines to remove builtin types
@ 2009-06-26 15:51 Ulrich Weigand
2009-06-29 15:37 ` Joel Brobecker
0 siblings, 1 reply; 3+ messages in thread
From: Ulrich Weigand @ 2009-06-26 15:51 UTC (permalink / raw)
To: brobecker, gdb-patches
Hello Joel,
when attempting to remove references to the remaining global builtin
types, I ran across a couple of instances in Ada language support code.
This patch removes some references to global builtin types from Ada code.
In particular, routines used to determine array bounds and length in
order to implement the 'range, 'first, 'last, and 'length attribute
would use builtin_type_int32 as fallback when unable to determine
the actual type.
The patch attempts to simplify that set of routines:
- There is now a clean separation between routines that compute
index *types* and those that compute index bound *values*.
- The ada_array_bound, ada_array_bound_from_type, and ada_array_length
now solely return numerical values (as LONGEST); they do not attempt
to do anything with types.
- The ada_index_type is now the only routine that determines index
types; it is consistently used in all cases by ada_evaluate_subexp
to determine the result types of the attribute expressions.
- ada_index_type will return NULL if unable to determine a type. The
caller, ada_evaluate_subexp, is in the best position to determine
an appropriate fall-back type to use.
- As the NULL return value now indicates the use of a fall-back type,
it can no longer be used to indicate errors. As ada_index_type is
only used from within ada_evaluate_subexp, I've changed it to
throw errors itself when detecting invalid arguments (and removed
now redundant checked from ada_evaluate_subexp).
Tested on amd64-linux. OK for mainline?
Bye,
Ulrich
ChangeLog:
* ada-lang.c (ada_index_type): Make static. Add NAME argument.
Throw error on invalid input arguments. Return NULL if unable
to determine index type.
(ada_array_bound_from_type): Remove TYPEP argument.
(ada_array_bound): Make static. Return LONGEST instead of value.
Update for ada_array_bound_from_type change.
(ada_array_length): Return LONGEST instead of value.
Update for ada_array_bound_from_type change.
(make_array_descriptor): Update for ada_array_bound change.
(ada_evaluate_subexp) [OP_ATR_RANGE, OP_ATR_FIRST, OP_ATR_LAST,
OP_ATR_LENGTH]: Update for ada_array_bound_from_type,
ada_array_bound, ada_array_length, ada_index_type changes.
Always use ada_index_type to compute result type; fall back
to architecture-specific integer type if ada_index_type fails.
* ada-lang.h (ada_index_type): Remove prototype.
(ada_array_bound): Likewise.
Index: gdb-head/gdb/ada-lang.c
===================================================================
--- gdb-head.orig/gdb/ada-lang.c
+++ gdb-head/gdb/ada-lang.c
@@ -2410,15 +2410,15 @@ ada_array_element_type (struct type *typ
/* The type of nth index in arrays of given type (n numbering from 1).
Does not examine memory. */
-struct type *
-ada_index_type (struct type *type, int n)
+static struct type *
+ada_index_type (struct type *type, int n, const char *name)
{
struct type *result_type;
type = desc_base_type (type);
- if (n > ada_array_arity (type))
- return NULL;
+ if (n < 0 || n > ada_array_arity (type))
+ error (_("invalid dimension number to '%s"), name);
if (ada_is_simple_array_type (type))
{
@@ -2430,25 +2430,27 @@ ada_index_type (struct type *type, int n
/* FIXME: The stabs type r(0,0);bound;bound in an array type
has a target type of TYPE_CODE_UNDEF. We compensate here, but
perhaps stabsread.c would make more sense. */
- if (result_type == NULL || TYPE_CODE (result_type) == TYPE_CODE_UNDEF)
- result_type = builtin_type_int32;
-
- return result_type;
+ if (result_type && TYPE_CODE (result_type) == TYPE_CODE_UNDEF)
+ result_type = NULL;
}
else
- return desc_index_type (desc_bounds_type (type), n);
+ {
+ result_type = desc_index_type (desc_bounds_type (type), n);
+ if (result_type == NULL)
+ error (_("attempt to take bound of something that is not an array"));
+ }
+
+ return result_type;
}
/* Given that arr is an array type, returns the lower bound of the
Nth index (numbering from 1) if WHICH is 0, and the upper bound if
WHICH is 1. This returns bounds 0 .. -1 if ARR_TYPE is an
- array-descriptor type. If TYPEP is non-null, *TYPEP is set to the
- bounds type. It works for other arrays with bounds supplied by
- run-time quantities other than discriminants. */
+ array-descriptor type. It works for other arrays with bounds supplied
+ by run-time quantities other than discriminants. */
static LONGEST
-ada_array_bound_from_type (struct type * arr_type, int n, int which,
- struct type ** typep)
+ada_array_bound_from_type (struct type * arr_type, int n, int which)
{
struct type *type, *index_type_desc, *index_type;
LONGEST retval;
@@ -2459,11 +2461,7 @@ ada_array_bound_from_type (struct type *
arr_type = decode_packed_array_type (arr_type);
if (arr_type == NULL || !ada_is_simple_array_type (arr_type))
- {
- if (typep != NULL)
- *typep = builtin_type_int32;
- return (LONGEST) - which;
- }
+ return (LONGEST) - which;
if (TYPE_CODE (arr_type) == TYPE_CODE_PTR)
type = TYPE_TARGET_TYPE (arr_type);
@@ -2500,9 +2498,6 @@ ada_array_bound_from_type (struct type *
internal_error (__FILE__, __LINE__, _("invalid type code of index type"));
}
- if (typep != NULL)
- *typep = index_type;
-
return retval;
}
@@ -2511,7 +2506,7 @@ ada_array_bound_from_type (struct type *
WHICH is 1. This routine will also work for arrays with bounds
supplied by run-time quantities other than discriminants. */
-struct value *
+static LONGEST
ada_array_bound (struct value *arr, int n, int which)
{
struct type *arr_type = value_type (arr);
@@ -2519,13 +2514,9 @@ ada_array_bound (struct value *arr, int
if (ada_is_packed_array_type (arr_type))
return ada_array_bound (decode_packed_array (arr), n, which);
else if (ada_is_simple_array_type (arr_type))
- {
- struct type *type;
- LONGEST v = ada_array_bound_from_type (arr_type, n, which, &type);
- return value_from_longest (type, v);
- }
+ return ada_array_bound_from_type (arr_type, n, which);
else
- return desc_one_bound (desc_bounds (arr), n, which);
+ return value_as_long (desc_one_bound (desc_bounds (arr), n, which));
}
/* Given that arr is an array value, returns the length of the
@@ -2534,7 +2525,7 @@ ada_array_bound (struct value *arr, int
Does not work for arrays indexed by enumeration types with representation
clauses at the moment. */
-static struct value *
+static LONGEST
ada_array_length (struct value *arr, int n)
{
struct type *arr_type = ada_check_typedef (value_type (arr));
@@ -2543,20 +2534,11 @@ ada_array_length (struct value *arr, int
return ada_array_length (decode_packed_array (arr), n);
if (ada_is_simple_array_type (arr_type))
- {
- struct type *type;
- LONGEST v =
- ada_array_bound_from_type (arr_type, n, 1, &type) -
- ada_array_bound_from_type (arr_type, n, 0, NULL) + 1;
- return value_from_longest (type, v);
- }
+ return (ada_array_bound_from_type (arr_type, n, 1)
+ - ada_array_bound_from_type (arr_type, n, 0) + 1);
else
- return
- 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),
- n, 0)) + 1);
+ return (value_as_long (desc_one_bound (desc_bounds (arr), n, 1))
+ - value_as_long (desc_one_bound (desc_bounds (arr), n, 0)) + 1);
}
/* An empty array whose type is that of ARR_TYPE (an array type),
@@ -3859,11 +3841,11 @@ make_array_descriptor (struct type *type
for (i = ada_array_arity (ada_check_typedef (value_type (arr))); i > 0; i -= 1)
{
modify_general_field (value_contents_writeable (bounds),
- value_as_long (ada_array_bound (arr, i, 0)),
+ ada_array_bound (arr, i, 0),
desc_bound_bitpos (bounds_type, i, 0),
desc_bound_bitsize (bounds_type, i, 0));
modify_general_field (value_contents_writeable (bounds),
- value_as_long (ada_array_bound (arr, i, 1)),
+ ada_array_bound (arr, i, 1),
desc_bound_bitpos (bounds_type, i, 1),
desc_bound_bitsize (bounds_type, i, 1));
}
@@ -9046,11 +9028,12 @@ ada_evaluate_subexp (struct type *expect
tem = longest_to_int (exp->elts[pc + 1].longconst);
- if (tem < 1 || tem > ada_array_arity (value_type (arg2)))
- error (_("invalid dimension number to 'range"));
+ type = ada_index_type (value_type (arg2), tem, "range");
+ if (!type)
+ type = value_type (arg1);
- arg3 = ada_array_bound (arg2, tem, 1);
- arg2 = ada_array_bound (arg2, tem, 0);
+ arg3 = value_from_longest (type, ada_array_bound (arg2, tem, 1));
+ arg2 = value_from_longest (type, ada_array_bound (arg2, tem, 0));
binop_promote (exp->language_defn, exp->gdbarch, &arg1, &arg2);
binop_promote (exp->language_defn, exp->gdbarch, &arg1, &arg3);
@@ -9112,29 +9095,27 @@ ada_evaluate_subexp (struct type *expect
if (ada_is_packed_array_type (value_type (arg1)))
arg1 = ada_coerce_to_simple_array (arg1);
- if (tem < 1 || tem > ada_array_arity (value_type (arg1)))
- error (_("invalid dimension number to '%s"),
- ada_attribute_name (op));
+ type = ada_index_type (value_type (arg1), tem,
+ ada_attribute_name (op));
+ if (type == NULL)
+ type = builtin_type (exp->gdbarch)->builtin_int;
if (noside == EVAL_AVOID_SIDE_EFFECTS)
- {
- type = ada_index_type (value_type (arg1), tem);
- if (type == NULL)
- error
- (_("attempt to take bound of something that is not an array"));
- return allocate_value (type);
- }
+ return allocate_value (type);
switch (op)
{
default: /* Should never happen. */
error (_("unexpected attribute encountered"));
case OP_ATR_FIRST:
- return ada_array_bound (arg1, tem, 0);
+ return value_from_longest
+ (type, ada_array_bound (arg1, tem, 0));
case OP_ATR_LAST:
- return ada_array_bound (arg1, tem, 1);
+ return value_from_longest
+ (type, ada_array_bound (arg1, tem, 1));
case OP_ATR_LENGTH:
- return ada_array_length (arg1, tem);
+ return value_from_longest
+ (type, ada_array_length (arg1, tem));
}
}
else if (discrete_type_p (type_arg))
@@ -9170,14 +9151,10 @@ ada_evaluate_subexp (struct type *expect
if (ada_is_packed_array_type (type_arg))
type_arg = decode_packed_array_type (type_arg);
- if (tem < 1 || tem > ada_array_arity (type_arg))
- error (_("invalid dimension number to '%s"),
- ada_attribute_name (op));
-
- type = ada_index_type (type_arg, tem);
+ type = ada_index_type (type_arg, tem, ada_attribute_name (op));
if (type == NULL)
- error
- (_("attempt to take bound of something that is not an array"));
+ type = builtin_type (exp->gdbarch)->builtin_int;
+
if (noside == EVAL_AVOID_SIDE_EFFECTS)
return allocate_value (type);
@@ -9186,14 +9163,14 @@ ada_evaluate_subexp (struct type *expect
default:
error (_("unexpected attribute encountered"));
case OP_ATR_FIRST:
- low = ada_array_bound_from_type (type_arg, tem, 0, &type);
+ low = ada_array_bound_from_type (type_arg, tem, 0);
return value_from_longest (type, low);
case OP_ATR_LAST:
- high = ada_array_bound_from_type (type_arg, tem, 1, &type);
+ high = ada_array_bound_from_type (type_arg, tem, 1);
return value_from_longest (type, high);
case OP_ATR_LENGTH:
- low = ada_array_bound_from_type (type_arg, tem, 0, &type);
- high = ada_array_bound_from_type (type_arg, tem, 1, NULL);
+ low = ada_array_bound_from_type (type_arg, tem, 0);
+ high = ada_array_bound_from_type (type_arg, tem, 1);
return value_from_longest (type, high - low + 1);
}
}
Index: gdb-head/gdb/ada-lang.h
===================================================================
--- gdb-head.orig/gdb/ada-lang.h
+++ gdb-head/gdb/ada-lang.h
@@ -284,10 +284,6 @@ extern int ada_is_array_descriptor_type
extern int ada_is_bogus_array_descriptor (struct type *);
-extern struct type *ada_index_type (struct type *, int);
-
-extern struct value *ada_array_bound (struct value *, int, int);
-
extern char *ada_decode_symbol (const struct general_symbol_info*);
extern const char *ada_decode (const char*);
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [rfa/Ada] Restructure array bounds routines to remove builtin types
2009-06-26 15:51 [rfa/Ada] Restructure array bounds routines to remove builtin types Ulrich Weigand
@ 2009-06-29 15:37 ` Joel Brobecker
2009-06-29 17:29 ` Ulrich Weigand
0 siblings, 1 reply; 3+ messages in thread
From: Joel Brobecker @ 2009-06-29 15:37 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
Hi Ulrich,
First of all, thank you for doing this. I know it's painful to change
someone else's code, sometime, especially when it's tough to test some
of them.
> - The ada_array_bound, ada_array_bound_from_type, and ada_array_length
> now solely return numerical values (as LONGEST); they do not attempt
> to do anything with types.
I'm slightly concerned at this part, but I don't think your patch makes
things any worse. In particular, I'm worried about arrays whose index
type is an unsigned 64bit type. If I understand things correctly, I
don't think it works well just yet, and PaulH was working on that.
> * ada-lang.c (ada_index_type): Make static. Add NAME argument.
> Throw error on invalid input arguments. Return NULL if unable
> to determine index type.
>
> (ada_array_bound_from_type): Remove TYPEP argument.
> (ada_array_bound): Make static. Return LONGEST instead of value.
> Update for ada_array_bound_from_type change.
> (ada_array_length): Return LONGEST instead of value.
> Update for ada_array_bound_from_type change.
> (make_array_descriptor): Update for ada_array_bound change.
>
> (ada_evaluate_subexp) [OP_ATR_RANGE, OP_ATR_FIRST, OP_ATR_LAST,
> OP_ATR_LENGTH]: Update for ada_array_bound_from_type,
> ada_array_bound, ada_array_length, ada_index_type changes.
> Always use ada_index_type to compute result type; fall back
> to architecture-specific integer type if ada_index_type fails.
>
> * ada-lang.h (ada_index_type): Remove prototype.
> (ada_array_bound): Likewise.
Looks OK to me, and verified also against AdaCore's testsuite
on x86-linux (DWARF and stabs).
Just one tiny request:
> -struct type *
> -ada_index_type (struct type *type, int n)
> +static struct type *
> +ada_index_type (struct type *type, int n, const char *name)
Would you mind documenting what this new NAME parameter is supposed
to be?
Thanks,
--
Joel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [rfa/Ada] Restructure array bounds routines to remove builtin types
2009-06-29 15:37 ` Joel Brobecker
@ 2009-06-29 17:29 ` Ulrich Weigand
0 siblings, 0 replies; 3+ messages in thread
From: Ulrich Weigand @ 2009-06-29 17:29 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
Joel Brobecker wrote:
> Looks OK to me, and verified also against AdaCore's testsuite
> on x86-linux (DWARF and stabs).
Thanks for the quick review! I've committed the patch now.
> Just one tiny request:
>
> > -struct type *
> > -ada_index_type (struct type *type, int n)
> > +static struct type *
> > +ada_index_type (struct type *type, int n, const char *name)
>
> Would you mind documenting what this new NAME parameter is supposed
> to be?
Here's what I committed on top of the original patch:
ChangeLog:
* ada-lang.c (ada_index_type): Update comment.
Index: gdb/ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.217
diff -u -r1.217 ada-lang.c
--- gdb/ada-lang.c 29 Jun 2009 17:22:10 -0000 1.217
+++ gdb/ada-lang.c 29 Jun 2009 17:24:55 -0000
@@ -2408,7 +2408,10 @@
}
/* The type of nth index in arrays of given type (n numbering from 1).
- Does not examine memory. */
+ Does not examine memory. Throws an error if N is invalid or TYPE
+ is not an array type. NAME is the name of the Ada attribute being
+ evaluated ('range, 'first, 'last, or 'length); it is used in building
+ the error message. */
static struct type *
ada_index_type (struct type *type, int n, const char *name)
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-06-29 17:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-26 15:51 [rfa/Ada] Restructure array bounds routines to remove builtin types Ulrich Weigand
2009-06-29 15:37 ` Joel Brobecker
2009-06-29 17:29 ` Ulrich Weigand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox