Re: [PATCH] Fix Ada val_print removal regression

[Simon Marchi <simark@simark.ca>]

On 2020-03-19 3:42 p.m., Tom Tromey wrote:
> @@ -553,39 +551,34 @@ ada_printstr (struct ui_file *stream, struct type *type,
>  }
>  
>  static int
> -print_variant_part (struct type *type, int field_num,
> -		    const gdb_byte *valaddr, int offset,
> +print_variant_part (struct value *value, int field_num,
> +		    struct value *outer_value,
>  		    struct ui_file *stream, int recurse,
> -		    struct value *val,
>  		    const struct value_print_options *options,
>  		    int comma_needed,
> -		    struct type *outer_type, int outer_offset,
>  		    const struct language_defn *language)
>  {
> +  struct type *type = value_type (value);
>    struct type *var_type = TYPE_FIELD_TYPE (type, field_num);
> -  int which = ada_which_variant_applies (var_type, outer_type,
> -					 valaddr + outer_offset);
> +  int which = ada_which_variant_applies (var_type,
> +					 value_type (outer_value),
> +					 value_contents (outer_value));z
>  
>    if (which < 0)
>      return 0;
> -  else
> -    return print_field_values
> -      (TYPE_FIELD_TYPE (var_type, which),
> -       valaddr,
> -       offset + TYPE_FIELD_BITPOS (type, field_num) / HOST_CHAR_BIT
> -       + TYPE_FIELD_BITPOS (var_type, which) / HOST_CHAR_BIT,
> -       stream, recurse, val, options,
> -       comma_needed, outer_type, outer_offset, language);
> +
> +  struct value *variant = value_field (value, which);

A bit of a nit, but it's to make sure I understand what's happening.  From what
I understand, the variant is the enclosing type, from which only one component
is active at a given time.  This value variable represents the active component,
right?  If so, I'd suggest naming it active_component or something like that.

> @@ -923,17 +907,16 @@ ada_val_print_enum (struct value *value, struct ui_file *stream, int recurse,
>      print_longest (stream, 'd', 0, val);
>  }
>  
> -/* Implement Ada val_print'ing for the case where TYPE is
> -   a TYPE_CODE_STRUCT or TYPE_CODE_UNION.  */
> +/* Implement Ada val_print'ing for the case where the type is
> +   TYPE_CODE_STRUCT or TYPE_CODE_UNION.  */
>  
>  static void
>  ada_val_print_struct_union
> -  (struct type *type, const gdb_byte *valaddr, int offset,
> -   int offset_aligned, CORE_ADDR address, struct ui_file *stream,
> +  (struct value *value, struct ui_file *stream,
>     int recurse, struct value *original_value,
>     const struct value_print_options *options)
>  {

The sole caller of this function passes the same `val` twice, so I suppose you could remove `original_value`.

Simon