This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
> 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
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |