Re: [PATCH] Compute proper length for dynamic types of TYPE_CODE_TYPEDEF

[Pedro Alves <palves@redhat.com>]

Hi Keith,

On 06/24/2017 01:59 AM, Keith Seitz wrote:
> --- a/gdb/testsuite/gdb.base/vla-datatypes.c
> +++ b/gdb/testsuite/gdb.base/vla-datatypes.c
> @@ -46,11 +46,12 @@ vla_factory (int n)
>    BAR             bar_vla[n];
>    int i;
>  
> -  struct vla_struct
> +  typedef struct vla_struct
>    {
>      int something;
>      int vla_field[n];
> -  } vla_struct_object;
> +  } vla_s;
> +  vla_s vla_struct_object;

This object is now defined with a typedef type, and that confused me a lot.
After staring at this for a while, I think you did intend that.  I just
find the naming very confusing.  Can we improve on that?  Maybe add
some comments too.

>  
>    struct inner_vla_struct
>    {
> @@ -59,14 +60,28 @@ vla_factory (int n)
>      int after;
>    } inner_vla_struct_object;
>  
> +  struct vla_typedef

... because this type is called "typedef", while
the above isn't.

> +  {
> +    int something;
> +    vla_s vla_object;
> +  } vla_typedef_struct_object;

... and this object here is not a a type that is 
defined as a typedef, while it's named "..._typedef_...".
So at first is looked like you have the "typedef vs non-typedef"
cases backwards.  I understand now that you're referring to
the "vla_object" member, but it was totally non-obvious to me.

Can we rename things a bit to avoid this confusion?

Maybe

  struct vla_typedef -> struct vla_s_struct_member
  union vla_typedef_union -> union vla_s_union_member

?

and add some comment about using "vla_s" throughout
because it's a typedef.  And/or rename it
to "vla_struct_typedef".

Thanks,
Pedro Alves