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

[Yaron Keren]

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/20150417/8102404c/attachment.html>