[llvm] r234343 - Enable W4 warnings by default for MSVC builds

[Aaron Ballman]

On Thu, Apr 16, 2015 at 11:47 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Thu, Apr 16, 2015 at 11:43 AM, Yaron Keren <yaron.keren at gmail.com> wrote:
>> This LGTM, with #ifdef _MSC_VER macro of course, maybe in Compiler.h ?
>
> That was what I was thinking, yes.
>
>> I wouldn't mind disabling the warning completely either, I don't find it
>> helpful.
>
> Since this is the only instance of that warning, and we may want to
> fix it to keep the published support headers compiling at /W4 cleanly,
> I don't see a reason to disable the warning. This warning *could* give
> us a heads up on behavioral differences for binary formats due to
> __declspec(align), but it's just as likely to trigger false positives,
> so I wouldn't argue too hard against disabling the warning, either.

I have disabled this warning in AlignOf.h in r235219. I did not add a
macro for it because MSVC doesn't support _Pragma from my testing.

~Aaron

>
> ~Aaron
>
>>
>>
>> 2015-04-16 18:12 GMT+03:00 Aaron Ballman <aaron at aaronballman.com>:
>>>
>>> Since AlignOf is part of our support headers, I would slightly prefer
>>> if we could fix it there. As an experiment, I put in the proper
>>> #pragma incantation around the declaration of AlignmentCalcImpl<T> in
>>> AlignOf.h, and it resolved *all* of the warnings.
>>>
>>> #pragma warning(push)
>>> #pragma warning(disable : 4324)
>>> template <typename T>
>>> struct AlignmentCalcImpl {
>>>   char x;
>>>   T t;
>>> private:
>>>   AlignmentCalcImpl() {} // Never instantiate.
>>> };
>>> #pragma warning(pop)
>>>
>>>
>>> While the #pragma dance is ugly (and would need to be abstracted away
>>> via macros), it may be worth it since this is a support header (and we
>>> cannot fix the padding issue because that's the whole point to that
>>> structure). How do others feel?
>>>
>>> ~Aaron
>>>
>>> On Thu, Apr 16, 2015 at 10:49 AM, Yaron Keren <yaron.keren at gmail.com>
>>> wrote:
>>> > /W3 was probably cached from the previous build.
>>> >
>>> > So should we disable C4324 or fix (how?) the AlignOf?
>>> >
>>> >
>>> > 2015-04-16 17:34 GMT+03:00 Aaron Ballman <aaron at aaronballman.com>:
>>> >>
>>> >> It seems that in order to trigger this, you have to have done a clean
>>> >> cmake build, as I am now seeing the same warnings you are.
>>> >>
>>> >> ~Aaron
>>> >>
>>> >> On Thu, Apr 16, 2015 at 10:16 AM, Aaron Ballman
>>> >> <aaron at aaronballman.com>
>>> >> wrote:
>>> >> > On Thu, Apr 16, 2015 at 10:03 AM, Yaron Keren <yaron.keren at gmail.com>
>>> >> > wrote:
>>> >> >> It is same in the IDE and command line, Release or Debug
>>> >> >> (unthreaded),
>>> >> >> 32
>>> >> >> bit.
>>> >> >> The warning is:
>>> >> >>
>>> >> >>   PPMacroExpansion.cpp
>>> >> >> c:\llvm\include\llvm/Support/AlignOf.h(25): warning C4324:
>>> >> >> 'llvm::AlignmentCalcImpl<T>' : structure was padded due to
>>> >> >> __declspec(align())
>>> >> >> [c:\llvm\msvc\tools\clang\lib\Lex\clangLex.vcxproj]
>>> >> >>           with
>>> >> >>           [
>>> >> >>               T=clang::SrcMgr::ContentCache
>>> >> >>           ] (c:\llvm\tools\clang\lib\Lex\PPCaching.cpp)
>>> >> >>           c:\llvm\include\llvm/Support/AlignOf.h(40) : see reference
>>> >> >> to
>>> >> >> class template instantiation 'llvm::AlignmentC
>>> >> >>   alcImpl<T>' being compiled
>>> >> >>           with
>>> >> >>           [
>>> >> >>               T=clang::SrcMgr::ContentCache
>>> >> >>           ]
>>> >> >>
>>> >> >> c:\llvm\tools\clang\include\clang/Basic/SourceManager.h(225)
>>> >> >> : see
>>> >> >> reference to class template instantiation
>>> >> >>    'llvm::AlignOf<clang::SrcMgr::ContentCache>' being compiled
>>> >> >>
>>> >> >> referring to SourceManager.h(225):
>>> >> >>
>>> >> >>   static_assert(llvm::AlignOf<ContentCache>::Alignment >= 8,
>>> >> >>                 "ContentCache must be 8-byte aligned.");
>>> >> >>
>>> >> >> And appears whenever SourceManager.h is included, a lot.
>>> >> >>
>>> >> >> The compiler is Microsoft Visual Studio Ultimate 2013,  Version
>>> >> >> 12.0.31101.00 Update 4.
>>> >> >>
>>> >> >> I'll try threaded build now.
>>> >> >
>>> >> > I will do a clean rebuild from scratch; I wonder if the reason I'm
>>> >> > not
>>> >> > seeing anything is because of some caching somewhere. Thank you!
>>> >> >
>>> >> > ~Aaron
>>> >
>>> >
>>
>>