Re: [RFA 3/3] Handle DW_TAG_variant_part and DW_TAG_variant

[Joel Brobecker <brobecker at adacore dot com>]

> 2018-02-19  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2read.c (struct nextfield) <discriminant_value,
> 	default_branch, is_discriminant>: New fields.
> 	(dwarf2_add_field): Handle DW_TAG_variant_part.
> 	(dwarf2_attach_fields_to_type): Attach a discriminant_info to a
> 	discriminated union.
> 	(read_structure_type): Handle DW_TAG_variant_part.
> 	(handle_struct_member_die): New function, extracted from
> 	process_structure_scope.  Handle DW_TAG_variant.
> 	(process_structure_scope): Handle discriminated unions.  Call
> 	handle_struct_member_die.
> 
> 2018-02-20  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.dwarf2/variant.c: New file.
> 	* gdb.dwarf2/variant.exp: New file.

Looks good to me as well. One suggestion below.

Also, thanks for the gdb.dwarf2 testcase!

> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 12ed4ae33c..db23b1a421 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1659,6 +1659,15 @@ struct nextfield
>    int accessibility;
>    int virtuality;
>    struct field field;
> +  /* If we see a DW_TAG_variant, then this will be the discriminant
> +     value.  */
> +  ULONGEST discriminant_value;
> +  /* If we see a DW_TAG_variant, then this will be set if this is the
> +     default branch.  */
> +  bool default_branch;
> +  /* While reading a DW_TAG_variant_part, this will be set if this
> +     field is the discriminant.  */
> +  bool is_discriminant;

Since those 3 new fields are related, what do you think of putting all 3
fields into a new struct, and then add one field in this struct?

> +static void
> +handle_struct_member_die (struct die_info *child_die, struct type *type,
> +			  struct field_info *fi,
> +			  std::vector<struct symbol *> *template_args,
> +			  struct dwarf2_cu *cu)

Thanks for extracting this code out and making it a function! ;-)


-- 
Joel