r309106 - Recommit r308327 2nd time: Add a warning for missing

[Hans Wennborg via cfe-commits]

On Wed, Jul 26, 2017 at 11:27 AM, Hans Wennborg <hans at chromium.org> wrote:
> On Wed, Jul 26, 2017 at 5:20 AM, Alex Lorenz via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>> Author: arphaman
>> Date: Wed Jul 26 05:20:57 2017
>> New Revision: 309106
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=309106&view=rev
>> Log:
>> Recommit r308327 2nd time: Add a warning for missing
>> '#pragma pack (pop)' and suspicious uses of '#pragma pack' in included files
>>
>> 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 have code in Chromium that does exactly this:
>
> gles2_cmd_format.h does #pragma pack(push, 4) and then #includes a
> file with some generated structs, with the intention that the pragma
> applies to them.
>
> What's the best way to pacify the warning in this case?
>
> (We're tracking this in
> https://bugs.chromium.org/p/chromium/issues/detail?id=749197)

I agree that cases 1) and 3) from your patch description make sense to
warn for, but I'm not sure that's the case for 2). Do you have
examples where this catches any bugs? In our case #pragma packing an
included file is intentional, and I suspect it might be a bit of a
pattern.

Wouldn't cases 1) and 3) catch most situations where this happens
unintentionally? E.g. when one #includes a file that forgets to
#pragma pop, and then includes a new file afterwards?

I've reverted in r309186 in the meantime.