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

[Yaron Keren]

Well, building for Visual C++ 2013 *Win 64* there is a flood of:

 tools\clang\include\clang/AST/Stmt.h(437): warning C4324: 'clang::Stmt' :
structure was padded due to __declspec(align())

and also

 lib\Support\CrashRecoveryContext.cpp(29): warning C4324:
'`anonymous-namespace'::CrashRecoveryContextImpl' : structure was padded
due to __declspec(align())

We could continue fixing these or just disable this warning.



2015-04-17 23:26 GMT+03:00 Yaron Keren <yaron.keren at gmail.com>:

> Thanks!
>
> 2015-04-17 22:40 GMT+03:00 Aaron Ballman <aaron at aaronballman.com>:
>
>> 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
>> >>> >
>> >>> >
>> >>
>> >>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150421/2a6db169/attachment.html>