This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RE: [PATCH 01/10] vla: introduce new bound type abstraction adapt uses
- From: "Agovic, Sanimir" <sanimir dot agovic at intel dot com>
- To: 'Tom Tromey' <tromey at redhat dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Mon, 18 Nov 2013 09:36:45 +0000
- Subject: RE: [PATCH 01/10] vla: introduce new bound type abstraction adapt uses
- Authentication-results: sourceware.org; auth=none
- References: <1382366424-21010-1-git-send-email-sanimir dot agovic at intel dot com> <1382366424-21010-2-git-send-email-sanimir dot agovic at intel dot com> <87iow4um4k dot fsf at fleche dot redhat dot com>
Thanks for your review.
All your comments being addressed and part of v2, see below.
-Sanimir
> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Thursday, November 07, 2013 07:02 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 01/10] vla: introduce new bound type abstraction adapt uses
>
> Sanimir> + low.data.const_val = dwarf2_get_attr_constant_value (attr,
> low.data.const_val);
>
> This line is too long and should be wrapped.
>
Done
>
> Sanimir> +struct type *
> Sanimir> +create_range_type_1 (struct type *result_type, struct type *index_type,
> Sanimir> + const struct dwarf2_prop *low_bound,
> Sanimir> + const struct dwarf2_prop *high_bound)
> Sanimir> +{
>
> This needs an introductory comment.
>
Done.
> Sanimir> +/* Used to store bound information for a type. */
> Sanimir> +
> Sanimir> +struct dwarf2_prop
> Sanimir> +{
> Sanimir> + /* Determine which field of the union dwarf2_prop.data is used. */
> Sanimir> + enum
> Sanimir> + {
> Sanimir> + DWARF_CONST,
> Sanimir> + DWARF_LOCEXPR,
> Sanimir> + DWARF_LOCLIST
> Sanimir> + } kind;
> Sanimir> +
> Sanimir> + /* Stores information as location expression, location list,
> Sanimir> + or constant value. */
> Sanimir> + union data
> Sanimir> + {
> Sanimir> + LONGEST const_val;
> Sanimir> + struct dwarf2_locexpr_baton *locexpr;
> Sanimir> + struct dwarf2_loclist_baton *loclist;
> Sanimir> + } data;
> Sanimir> +};
>
> Sanimir> @@ -589,11 +612,11 @@ struct main_type
> Sanimir> {
> Sanimir> /* Low bound of range. */
>
> Sanimir> - LONGEST low;
> Sanimir> + struct dwarf2_prop low;
>
> Sanimir> /* High bound of range. */
>
> Sanimir> - LONGEST high;
> Sanimir> + struct dwarf2_prop high;
>
>
> Just after this hunk of "struct range_bounds" is this code:
>
> /* Flags indicating whether the values of low and high are
> valid. When true, the respective range value is
> undefined. Currently used only for FORTRAN arrays. */
>
> char low_undefined;
> char high_undefined;
>
> It seems to me that it would be cleanest to make this a new value of the
> enum you introduced, like "DWARF_UNDEFINED", and update a few macros to
> follow. What do you think?
>
Agreed. Added DWARF_UNDEFINED and updated macro & uses.
-Sanimir
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052