Re: RFC: [PATCH] Add warn_if_not_aligned attribute

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Jun 6, 2017 at 10:34 AM, Martin Sebor <msebor@gmail.com> wrote:
> On 06/06/2017 10:59 AM, H.J. Lu wrote:
>>
>> On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor <msebor@gmail.com> wrote:
>>>
>>> On 06/06/2017 10:07 AM, Martin Sebor wrote:
>>>>
>>>>
>>>> On 06/05/2017 11:45 AM, H.J. Lu wrote:
>>>>>
>>>>>
>>>>> On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers <joseph@codesourcery.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> The new attribute needs documentation.  Should the test be in
>>>>>> c-c++-common
>>>>>
>>>>>
>>>>>
>>>>> This feature does support C++.  But C++ compiler issues a slightly
>>>>> different warning at a different location.
>>>>>
>>>>>> or does this feature not support C++?
>>>>>>
>>>>>
>>>>> Here is the updated patch with documentation and a C++ test.  This
>>>>> patch caused a few testsuite failures:
>>>>>
>>>>> FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile
>>>>>
>>>>>
>>>>>
>>>>> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:
>>>>>
>>>>> warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16
>>>>>
>>>>> FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
>>>>>
>>>>>
>>>>>
>>>>> /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:
>>>>>
>>>>> warning: alignment 1 of 'B' is less than 16
>>>>>
>>>>
>>>> Users often want the ability to control a warning, even when it
>>>> certainly indicates a bug.  I would suggest to add an option to
>>>> make it possible for this warning as well.
>>>>
>>>> Btw., a bug related to some of those this warning is meant to
>>>> detect is assigning the address of an underaligned object to
>>>> a pointer of a natively aligned type.  Clang has an option
>>>> to detect this problem: -Waddress-of-packed-member.  It might
>>>> make a nice follow-on enhancement to add support for the same
>>>> thing.  I mention this because I think it would make sense to
>>>> consider this when choosing the name of the GCC option (i.e.,
>>>> rather than having two distinct but closely related warnings,
>>>> have one that detects both of these alignment type of bugs.
>>>
>>>
>>>
>>> A bug that has some additional context on this is pr 51628.
>>> A possible name for the new option suggested there is -Wpacked.
>>>
>>> Martin
>>
>>
>> Isn't -Waddress-of-packed-member a subset of or the same as
>> -Wpacked?
>
>
> In Clang it's neither.  -Waddress-of-packed-member only triggers
> when the address of a packed member is taken but not for the cases
> in bug 53037 (i.e., reducing the alignment of a member).  It's
> also enabled by default, while -Wpacked needs to be specified
> explicitly (i.e., it's in neither -Wall or -Wextra).
>
> FWIW, I don't really have a strong opinion about the names of
> the options.  My input is that the proliferation of fine-grained
> warning options for closely related problems tends to make it
> tricky to get their interactions right (both in the compiler
> and for users).  Enabling both/all such options can lead to
> multiple warnings for what boils down to essentially the same
> bug in the same expression, overwhelming the user in repetitive
> diagnostics.
>

There is already -Wpacked.  Should I overload it for this?


-- 
H.J.