Re: RFC: [PATCH] Add warn_if_not_aligned attribute

[Joseph Myers <joseph@codesourcery.com>]

On Fri, 16 Jun 2017, H.J. Lu wrote:

> +@code{warning: alignment 8 of 'struct foo' is less than 16}.

I think @samp is better than @code for warnings, throughout, since they 
aren't pieces of program code.

> +This warning can be disabled by @option{-Wno-if-not-aligned}.
> +The @code{warn_if_not_aligned } attribute can also be used for types

Stray space before }.

> +static void
> +handle_warn_if_not_align (tree field, unsigned int record_align)

Missing comment above this function explaining its semantics and those of 
its arguments.

> +  if ((record_align % warn_if_not_align) != 0)
> +    warning (opt_w, "alignment %d of %qT is less than %d",
> +	     record_align, context, warn_if_not_align);

I'd expect %u for unsigned int alignments, instead of %d.

> +  unsigned int off
> +    = (tree_to_uhwi (DECL_FIELD_OFFSET (field))
> +       + tree_to_uhwi (DECL_FIELD_BIT_OFFSET (field)) / BITS_PER_UNIT);
> +  if ((off % warn_if_not_align) != 0)
> +    warning (opt_w, "%q+D offset %d in %qT isn't aligned to %d",
> +	     field, off, context, warn_if_not_align);

And you can have struct offsets that don't fit in unsigned int (i.e. 
structures over 4 GB), so should be using unsigned HOST_WIDE_INT to store 
the offset and %wu to print it.  (Whereas various places in GCC restrict 
alignments to unsigned int.)

What happens if you specify the attribute on a bit-field, or on a type 
used to declare a bit-field?  I don't think either of those particularly 
makes sense, but I don't see tests for it either.

-- 
Joseph S. Myers
joseph@codesourcery.com