Re: [RFA 2/6] Handle alignof and _Alignof

[Pedro Alves <palves@redhat.com>]

Hi Tom,

What's in the patch looks good to me.  I have comments on the
tests -- I think it'd be good to extend them a bit more.

On 04/24/2018 04:22 PM, Tom Tromey wrote:

> +
> +# The types we're going to test.
> +
> +set typelist {
> +    char {unsigned char}
> +    short {unsigned short}
> +    int {unsigned int}
> +    long {unsigned long}
> +    {long long} {unsigned long long}
> +    float
> +    double

Shouldn't we test "long double"?  Patch #1 handles it.
Not sure all GCC ports support it, may require separate compilation.

Also, I'm wondering about "__int128" if the target
supports it.

In C++, do we get the alignment of non-standard layout classes right?
E.g., structs with references.  And structs with virtual methods, like:

 struct S
 {
   virtual ~S ();
   char c;
 };

This should print 8 instead of 1 on x86-64, due to the vtable pointer.

I think it'd be good to cover those things in the tests too.

Likewise arrays, bitfields and typedefs?

I didn't spot any test for the
 "could not determine alignment of type"
case to make that that works gracefully without crashing.  

What do we do with _Alignof(void)?  We treat sizeof(void) == 1,
like gcc, so I assume the _Alignof will return 1 too instead
of erroring out.

Finally, for completeness, GCC allows _Alignof applied to
expressions, so I guess we should to.  Does the series allow that?
I.e., can we do _Alignof(1 + 1)?  Does the parser handle that?

Shouldn't we test _Alignof applied to the structure fields too?

There was a snippet in the patch that made me wonder if the patch
handles alignof of a no-debug-info variable and and the return-type
of a no-debug-info function correctly (instead of e.g., crashing).
I'd be nice to add a couples test to gdb.base/nodebug.exp
to make sure.  E.g.:
   p _Alignof (dataglobal64_1)
   p _Alignof (middle())"

Also, please add intro comments to the testcase .exp files, so
that later on people can find out what the testcase is
about easily.

Thanks,
Pedro Alves