<div dir="rtl"><div dir="ltr">In general I'm pro-warnings, but <span style="font-size:12.8000001907349px">C4324 </span>warning is not helpful...the compiler says it aligned what you specifically asked to align. Surprise.</div><div dir="ltr"><br></div><div dir="ltr">Go ahead and disable it, you can remove <span style="font-size:12.8000001907349px">r235219 which will become superfluous.</span></div><div dir="ltr"><span style="font-size:12.8000001907349px"><br></span></div></div><div class="gmail_extra"><br><div class="gmail_quote"><div dir="ltr">2015-04-22 0:57 GMT+03:00 Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span>:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Tue, Apr 21, 2015 at 4:58 PM, Yaron Keren <<a href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>> wrote:<br>
> Well, building for Visual C++ 2013 Win 64 there is a flood of:<br>
<br>
</span>Ah, I was building for x86, so this makes sense.<br>
<span class=""><br>
> tools\clang\include\clang/AST/Stmt.h(437): warning C4324: 'clang::Stmt' :<br>
> structure was padded due to __declspec(align())<br>
><br>
> and also<br>
><br>
> lib\Support\CrashRecoveryContext.cpp(29): warning C4324:<br>
> '`anonymous-namespace'::CrashRecoveryContextImpl' : structure was padded due<br>
> to __declspec(align())<br>
><br>
> We could continue fixing these or just disable this warning.<br>
<br>
</span>These I am far less concerned about fixing. I think we're fine<br>
disabling the warning (for all MSVC targets, not just x64). What do<br>
you think?<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
><br>
><br>
> 2015-04-17 23:26 GMT+03:00 Yaron Keren <<a href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>>:<br>
>><br>
>> Thanks!<br>
>><br>
>> 2015-04-17 22:40 GMT+03:00 Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>>:<br>
>>><br>
>>> On Thu, Apr 16, 2015 at 11:47 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>>> wrote:<br>
>>> > On Thu, Apr 16, 2015 at 11:43 AM, Yaron Keren <<a href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>><br>
>>> > wrote:<br>
>>> >> This LGTM, with #ifdef _MSC_VER macro of course, maybe in Compiler.h ?<br>
>>> ><br>
>>> > That was what I was thinking, yes.<br>
>>> ><br>
>>> >> I wouldn't mind disabling the warning completely either, I don't find<br>
>>> >> it<br>
>>> >> helpful.<br>
>>> ><br>
>>> > Since this is the only instance of that warning, and we may want to<br>
>>> > fix it to keep the published support headers compiling at /W4 cleanly,<br>
>>> > I don't see a reason to disable the warning. This warning *could* give<br>
>>> > us a heads up on behavioral differences for binary formats due to<br>
>>> > __declspec(align), but it's just as likely to trigger false positives,<br>
>>> > so I wouldn't argue too hard against disabling the warning, either.<br>
>>><br>
>>> I have disabled this warning in AlignOf.h in r235219. I did not add a<br>
>>> macro for it because MSVC doesn't support _Pragma from my testing.<br>
>>><br>
>>> ~Aaron<br>
>>><br>
>>> ><br>
>>> > ~Aaron<br>
>>> ><br>
>>> >><br>
>>> >><br>
>>> >> 2015-04-16 18:12 GMT+03:00 Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>>:<br>
>>> >>><br>
>>> >>> Since AlignOf is part of our support headers, I would slightly prefer<br>
>>> >>> if we could fix it there. As an experiment, I put in the proper<br>
>>> >>> #pragma incantation around the declaration of AlignmentCalcImpl<T> in<br>
>>> >>> AlignOf.h, and it resolved *all* of the warnings.<br>
>>> >>><br>
>>> >>> #pragma warning(push)<br>
>>> >>> #pragma warning(disable : 4324)<br>
>>> >>> template <typename T><br>
>>> >>> struct AlignmentCalcImpl {<br>
>>> >>> char x;<br>
>>> >>> T t;<br>
>>> >>> private:<br>
>>> >>> AlignmentCalcImpl() {} // Never instantiate.<br>
>>> >>> };<br>
>>> >>> #pragma warning(pop)<br>
>>> >>><br>
>>> >>><br>
>>> >>> While the #pragma dance is ugly (and would need to be abstracted away<br>
>>> >>> via macros), it may be worth it since this is a support header (and<br>
>>> >>> we<br>
>>> >>> cannot fix the padding issue because that's the whole point to that<br>
>>> >>> structure). How do others feel?<br>
>>> >>><br>
>>> >>> ~Aaron<br>
>>> >>><br>
>>> >>> On Thu, Apr 16, 2015 at 10:49 AM, Yaron Keren <<a href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>><br>
>>> >>> wrote:<br>
>>> >>> > /W3 was probably cached from the previous build.<br>
>>> >>> ><br>
>>> >>> > So should we disable C4324 or fix (how?) the AlignOf?<br>
>>> >>> ><br>
>>> >>> ><br>
>>> >>> > 2015-04-16 17:34 GMT+03:00 Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>>:<br>
>>> >>> >><br>
>>> >>> >> It seems that in order to trigger this, you have to have done a<br>
>>> >>> >> clean<br>
>>> >>> >> cmake build, as I am now seeing the same warnings you are.<br>
>>> >>> >><br>
>>> >>> >> ~Aaron<br>
>>> >>> >><br>
>>> >>> >> On Thu, Apr 16, 2015 at 10:16 AM, Aaron Ballman<br>
>>> >>> >> <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>>> >>> >> wrote:<br>
>>> >>> >> > On Thu, Apr 16, 2015 at 10:03 AM, Yaron Keren<br>
>>> >>> >> > <<a href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>><br>
>>> >>> >> > wrote:<br>
>>> >>> >> >> It is same in the IDE and command line, Release or Debug<br>
>>> >>> >> >> (unthreaded),<br>
>>> >>> >> >> 32<br>
>>> >>> >> >> bit.<br>
>>> >>> >> >> The warning is:<br>
>>> >>> >> >><br>
>>> >>> >> >> PPMacroExpansion.cpp<br>
>>> >>> >> >> c:\llvm\include\llvm/Support/AlignOf.h(25): warning C4324:<br>
>>> >>> >> >> 'llvm::AlignmentCalcImpl<T>' : structure was padded due to<br>
>>> >>> >> >> __declspec(align())<br>
>>> >>> >> >> [c:\llvm\msvc\tools\clang\lib\Lex\clangLex.vcxproj]<br>
>>> >>> >> >> with<br>
>>> >>> >> >> [<br>
>>> >>> >> >> T=clang::SrcMgr::ContentCache<br>
>>> >>> >> >> ] (c:\llvm\tools\clang\lib\Lex\PPCaching.cpp)<br>
>>> >>> >> >> c:\llvm\include\llvm/Support/AlignOf.h(40) : see<br>
>>> >>> >> >> reference<br>
>>> >>> >> >> to<br>
>>> >>> >> >> class template instantiation 'llvm::AlignmentC<br>
>>> >>> >> >> alcImpl<T>' being compiled<br>
>>> >>> >> >> with<br>
>>> >>> >> >> [<br>
>>> >>> >> >> T=clang::SrcMgr::ContentCache<br>
>>> >>> >> >> ]<br>
>>> >>> >> >><br>
>>> >>> >> >> c:\llvm\tools\clang\include\clang/Basic/SourceManager.h(225)<br>
>>> >>> >> >> : see<br>
>>> >>> >> >> reference to class template instantiation<br>
>>> >>> >> >> 'llvm::AlignOf<clang::SrcMgr::ContentCache>' being compiled<br>
>>> >>> >> >><br>
>>> >>> >> >> referring to SourceManager.h(225):<br>
>>> >>> >> >><br>
>>> >>> >> >> static_assert(llvm::AlignOf<ContentCache>::Alignment >= 8,<br>
>>> >>> >> >> "ContentCache must be 8-byte aligned.");<br>
>>> >>> >> >><br>
>>> >>> >> >> And appears whenever SourceManager.h is included, a lot.<br>
>>> >>> >> >><br>
>>> >>> >> >> The compiler is Microsoft Visual Studio Ultimate 2013, Version<br>
>>> >>> >> >> 12.0.31101.00 Update 4.<br>
>>> >>> >> >><br>
>>> >>> >> >> I'll try threaded build now.<br>
>>> >>> >> ><br>
>>> >>> >> > I will do a clean rebuild from scratch; I wonder if the reason<br>
>>> >>> >> > I'm<br>
>>> >>> >> > not<br>
>>> >>> >> > seeing anything is because of some caching somewhere. Thank you!<br>
>>> >>> >> ><br>
>>> >>> >> > ~Aaron<br>
>>> >>> ><br>
>>> >>> ><br>
>>> >><br>
>>> >><br>
>><br>
>><br>
><br>
</div></div></blockquote></div><br></div>