Re: [RFA] print arrays with indexes

[Joel Brobecker <brobecker at adacore dot com>]

> This looks OK to me, with documentation and tests.  Minor comments:

Thanks for the review. The documentation has been submitted under:
        http://sources.redhat.com/ml/gdb-patches/2005-09/msg00240.html
And the testcase has been submitted under:
        http://sources.redhat.com/ml/gdb-patches/2005-09/msg00238.html

> - Reading the diff it's apparent that there is some space vs tabs
> confusion in the Ada changes.  I prefer tabs, but not violently so;
> however, I mostly prefer that code match its surroundings.  Otherwise
> indentation in diffs looks very odd.

Ok. There is some confusion, probably because I very much prefer spaces
(I am always jerked off by the cursor suddenly jumping because of
invisible tabs, it makes editing a bit more difficult, etc), but the
fact is that this is currently the policy. So I have forced tabs where
appropriate.

BTW: I think you are using vim, IIRC. Do you have some magic settings
that handles that handles the tabs for you. If I were able to have tabs
automatically created after I create spaces, and also transformed into
spaces when I backspace into them, as well as allowing me to go through
them as if they were spaces, I wouldn't mind the tabs so much.

> >  /*  Called by various <lang>_val_print routines to print elements of an
> >     array in the form "<elem1>, <elem2>, <elem3>, ...".
> >  
> > +   Some languages such as Ada allow the user to specify arrays where
> > +   the index of the first element is not zero. REAL_INDEX_OFFSET is
> > +   the user-level index of the first element of the array. For many
> > +   languages such as C or C++, it is always zero.
> > +
> 
> Ada is by no means the only language with this feature - namely,
> Fortran.  Can't you compute this value from the array type, instead
> of passing it in from Ada-specific code?  I couldn't see any obvious
> reasons why not.

I cannot either. So what I did was move that function to valprint.
I also removed the part computing the index type, it should be as
simple as using the TYPE_INDEX_TYPE macro, so no need to complexify
this function for that.

Note that this had the effect of losing the following code:

+  if (TYPE_CODE (index) == TYPE_CODE_RANGE)
+    index = TYPE_TARGET_TYPE (index);

which the function used to have. I couldn't understand why this
was necessary, and a look at where the type was used showed that
this was not necessary, as ada_print_scalar, the only eventual consumer
of that type, knew how to handle range types. I would think the same is
true of all other languages, right?

> > +/* Assuming TYPE is a simple, non-empty array type, compute the lower
> > +   bound and the array index type.  Save the low bound into LOW_BOUND
> > +   if not NULL.  Save the index type in INDEX_TYPE if not NULL.
> > +
> > +   Return 1 if the operation was successful. Return zero otherwise,
> > +   in which case the value of LOW_BOUND and INDEX_TYPE is undefined.  */
> 
> s/undefined/unmodified/, since you rely on it below.

Thanks for the recommendation. I really like this term better.

Here is a new patch, hopefully implementing the suggestions you made.

2005-09-22  Joel Brobecker  <brobecker@adacore.com>

        * language.h (language_defn): New field la_print_array_index.
        (LA_PRINT_ARRAY_INDEX): New macro.
        (default_print_array_index): Add declaration.
        * language.c (default_print_array_index): new function.
        (unknown_language): Add value for new field.
        (auto_language): Likewise.
        (local_language): Likewise.
        * ada-lang.c (ada_print_array_index): New function.
        (ada_language_defn): Add value for new field.
        * c-lang.c (c_language_defn): Likewise.
        (cpluc_language_defn): Likewise.
        (asm_language_defn): Likewise.
        (minimal_language_defn): Likewise.
        * f-lang.c (f_language_defn): Likewise.
        * jv-lang.c (java_language_defn): Likewise.
        * m2-lang.c (m2_language_defn): Likewise.
        * objc-lang.c (objc_language_defn): Likewise.
        * p-lang.c (pascal_language_defn): Likewise.
        * valprint.h (print_array_indexes_p): Add declaration.
        (get_array_low_bound): Add declaration.
        (maybe_print_array_index): Add declaration.
        * valprint.c (print_array_indexes): New static variable.
        (show_print_array_indexes): New function.
        (print_array_indexes_p): New function.
        (get_array_low_bound): New function.
        (maybe_print_array_index): New function.
        (val_print_array_elements): Print the index of each element if
        requested by the user.
        (_initialize_valprint): Add new array-indexes "set/show print" command.
        * ada-valprint.c (print_optional_low_bound): Replace extracted code
        by call to ada_get_array_low_bound_and_type(). Stop printing the low
        bound if indexes will be printed for all elements of the array.
        (val_print_packed_array_elements): Print the index of each element
        of the array if necessary.

Tested on x86-linux, no regression.

Thanks,
-- 
Joel

Attachment: arridx.diff
Description: Text document