Re: [PATCH 1/5] stdalign: new module

[Bruno Haible]

Hi Paul,

Thanks for getting back to this.

> A while ago Bruno drafted a Gnulib module that is a replacement for C1x.

Reference:
<https://lists.gnu.org/archive/html/bug-gnulib/2011-07/msg00226.html>.

> address@hidden
> address@hidden and @code{alignas} are not always supported;
> +on platforms lacking support, the
> +macro @code{__alignas_is_defined} is not defined.
> +Supported platforms includes GCC 2 and later, Sun C 5.11 and later,
> +and MSVC 7.0 or later.

Maybe say "Supported compilers" instead of "Supported platforms"?
A platform is typically a compilation of OS and compiler.

> +/* Return the alignment of a structure slot (field) of TYPE,

It's not clear here whether this text talks about _Alignof (TYPE),
or alignof (TYPE), or both. Maybe say it explicitly?

> +   This is not the same as GCC's __alignof__ operator; for example, on
> +   x86 with GCC, _Alignof (long long) is typically 4 whereas
> +   __alignof__ (long long) is 8.  */

And 'double' as well. And the option -malign-double changes it from 4 to 8
(yes, also for 'long long'!). So I would write it like this:

  This is not the same as GCC's __alignof__ operator.  The latter returns
  the optimal alignment for an object of that type.  For example, on x86
  with GCC, __alignof__ (double) and __alignof__ (long long) are 8,
  whereas _Alignof (double) and _Alignof (long long) are usually 4 (unless
  the option '-malign-double' is used).

> +/* Align a type or variable to the alignment A.  */

Here too: Is this a specification of _Alignas(A), or alignas(A), or both?

It would also be good to specify where this attribute has to be used
in case of a variable: before the type, after the type, before the variable,
or after the variable.

> +#if @HAVE_ATTRIBUTE_ALIGNED@ && !defined __cplusplus

Gnulib generated .h files ought to be usable with a different compiler
on the same platform. For example, build it with Solaris cc then use it
with gcc, or vice versa. Therefore IMO @HAVE_ATTRIBUTE_ALIGNED@ should
be replaced with a preprocessor conditional that tests the compiler
brand and version. I think
  __GNUC__ >= 2
is a good approximation. gcc 2.95.3 already had it (
<http://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_4.html#SEC90>,
<http://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_4.html#SEC91>),
and I think it goes back to gcc 2.7 or older.

> +#elif 1300 <= _MSC_VER
> +# define _Alignas(a) __declspec ((align (a)))

Thanks!

> +    AC_CACHE_CHECK([for  __attribute__ ((__aligned__ (expr)))],
> +      [gl_cv_attribute_aligned],
> +      [AC_COMPILE_IFELSE(
> +         [AC_LANG_PROGRAM(
> +            [[char __attribute__ ((__aligned__ (1 << 3))) c;]],
> +            [[]])],
> +         [gl_cv_attribute_aligned=yes],
> +         [gl_cv_attribute_aligned=no])])
> +    if test $gl_cv_attribute_aligned = yes; then
> +      HAVE_ATTRIBUTE_ALIGNED=1
> +    else
> +      HAVE_ATTRIBUTE_ALIGNED=0
> +    fi

I would either remove this, or augment the test suite to emit a warning
if HAVE_ATTRIBUTE_ALIGNED turns out to be 1 but the preprocessor condition
(__GNUC__ >= 2) is false.

Again, thanks for following through on this!

Bruno
-- 
In memoriam The victims of the French police 
<http://en.wikipedia.org/wiki/Paris_massacre_of_1961>