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

[Pedro Alves <palves@redhat.com>]

On 04/27/2018 03:01 PM, Tom Tromey wrote:
> This adds alignof and _Alignof to the C/C++ expression parser, and
> adds new tests to test the features.  The tests are written to try to
> ensure that gdb's knowledge of alignment rules stays in sync with the
> compiler's.

This looks good to me.  A few comments on additional tests below,
but there's no need for another round of review for those.

> +++ b/gdb/testsuite/gdb.cp/align.exp
> @@ -0,0 +1,145 @@

...

> +
> +# Prologue.
> +puts $outfile {
> +    template<typename T, typename U>
> +    struct align_pair
> +    {
> +	T one;
> +	U two;
> +    };
> +
> +    template<typename T, typename U>
> +    struct align_union
> +    {
> +	T one;
> +	U two;
> +    };
> +
> +    enum bigenum { VALUE = 0xffffffffull };
> +
> +    struct empty { };
> +
> +    struct vstruct { virtual ~vstruct() {}  char c; };
> +
> +    struct bfstruct { unsigned b : 3; };
> +
> +    struct arrstruct { short fld[7]; };

Sorry to be a drag, but I think it'd be good to add a few
more tests here:

 - alignof(typedef)

   (to make sure we're not missing a check_typedef
   call somewhere.  e.g., add "typedef arrstruct arrstruct_t",
   then add arrstruct_t to the list of tested types.

 - alignof(a class with a base class).

   E.g.,:

    struct A { int i;};
    struct B : A { char c; };

   and then check alignof(B).

   multiple and virtual inheritance might be worth
   testing too, not sure whether the code paths are
   (and will always be) the same.

 - alignof(array-type)

   This is what I actually meant by arrays in v1.  E.g.:

     alignof (int[3])

   It might be we don't parse that.  Not sure.  If it's hard,
   it's ok to punt, but then it'd be good to still add the test
   and kfail it.

> +proc maybe_xfail {type} {
> +    # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69560
> +    # The g++ implementation of alignof is changing to match C11.
> +    if {[is_x86_like_target]
> +	&& ($type == "double" || $type == "long long"
> +	    || $type == "unsigned long long")} {
> +	setup_xfail *-*-*

It seems like we can check for gcc version with test_compiler_info.
Maybe go ahead and limit the xfail to:

  [test_compiler_info {gcc-[0-8]-*}]

?

Thanks,
Pedro Alves