r309386 - Recommit r308327 3rd time: Add a warning for missing

[Alex L via cfe-commits]

Add a note with a fixit in r309559.

On 31 July 2017 at 10:50, Alex L <arphaman at gmail.com> wrote:

> That's an interesting case. I'll tweak the warning to diagnose this
> scenario better today.
>
> On 29 July 2017 at 00:56, Hans Wennborg <hans at chromium.org> wrote:
>
>> On Fri, Jul 28, 2017 at 7:41 AM, Alex Lorenz via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>> > Author: arphaman
>> > Date: Fri Jul 28 07:41:21 2017
>> > New Revision: 309386
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=309386&view=rev
>> > Log:
>> > Recommit r308327 3rd time: Add a warning for missing
>> > '#pragma pack (pop)' and suspicious uses of '#pragma pack' in included
>> files
>> >
>> > The second recommit (r309106) was reverted because the "non-default
>> #pragma
>> > pack value chages the alignment of struct or union members in the
>> included file"
>> > warning proved to be too aggressive for external projects like Chromium
>> > (https://bugs.chromium.org/p/chromium/issues/detail?id=749197). This
>> recommit
>> > makes the problematic warning a non-default one, and gives it the
>> > -Wpragma-pack-suspicious-include warning option.
>> >
>> > The first recommit (r308441) caused a "non-default #pragma pack value
>> might
>> > change the alignment of struct or union members in the included file"
>> warning
>> > in LLVM itself. This recommit tweaks the added warning to avoid
>> warnings for
>> > #includes that don't have any records that are affected by the
>> non-default
>> > alignment. This tweak avoids the previously emitted warning in LLVM.
>> >
>> > Original message:
>> >
>> > This commit adds a new -Wpragma-pack warning. It warns in the following
>> cases:
>> >
>> > - When a translation unit is missing terminating #pragma pack (pop)
>> directives.
>> > - When entering an included file if the current alignment value as
>> determined
>> >   by '#pragma pack' directives is different from the default alignment
>> value.
>> > - When leaving an included file that changed the state of the current
>> alignment
>> >   value.
>> >
>> > rdar://10184173
>> >
>> > Differential Revision: https://reviews.llvm.org/D35484
>>
>> We did get warnings with this version too, but this time it looks like
>> a real bug :-)
>>
>> C:\b\c\builder\ClangToTWin_dll_\src\third_party\usrsctp\usrs
>> ctplib\usrsctplib\netinet\sctp.h(51,9):
>>  error: unterminated '#pragma pack (push, ...)' at end of file
>> [-Werror,-Wpragma-pack]
>>
>> The interesting thing is that the file does reset the alignment, but
>> it does it with a "#pragma pack()" which doesn't pop the stack, but
>> just resets to the default value.
>>
>> It might be worth tweaking the warning to give a better message for
>> this. A "#pragma pack(push, ..)" followed by "#pragma pack()" seems
>> pretty dodgy since there's no point in pushing if the code isn't going
>> to pop.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170731/52fb760a/attachment.html>