<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div><br><br>Sent from my iPhone</div><div><br>On Jul 10, 2015, at 4:13 PM, David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br><br></div><blockquote type="cite"><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 10, 2015 at 4:01 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class=""><br>
> On 2015-Jul-10, at 15:46, David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br>
><br>
> Author: majnemer<br>
> Date: Fri Jul 10 17:46:02 2015<br>
> New Revision: 241958<br>
><br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D241958-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=vlpPDUd2DT1f1slZ2uCdvrX-x01vntSI1jH0dBr988U&s=cy8fzRd2Zp1Qomp2F0QBEE7Ju5Nz--BONaHtoaG1WJ8&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=241958&view=rev</a><br>
> Log:<br>
> [IR] Switch static const to an enum to silence MSVC linker warnings<br>
><br>
> Integral class statics are handled oddly in MSVC, we don't need them<br>
> in this case, use an enum instead.<br>
<br>
</span>Oddly, how? </blockquote><div><br></div><div>say we have foo.h with:</div><div>struct S {<br></div><div> static const int x = 3;</div><div>};</div><div><br></div><div>and we have foo.cpp with:</div><div>#include "foo.h"</div><div>const int S::x;</div><div><br></div><div><br></div><div>all files which include foo.h, except for foo.cpp, will get a definition of S::x. In order for them to coexist, the definition lives in a COMDAT.</div><div><br></div><div>however, foo.cpp will get a definition of S::x which is not in a COMDAT. Instead, it will get a strong definition. This will result in link errors because we will have two definitions: one in a COMDAT and one not in a COMDAT.</div><div><br></div><div>This is documented, intentional, behavior of MSVC: <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_34h23df8-28v-3Dvs.100-29.aspx-23sectionToggle0&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=vlpPDUd2DT1f1slZ2uCdvrX-x01vntSI1jH0dBr988U&s=AL-kdxOIygetnHD5yUD2hnMVzQuQ55hbo8MFOszw9eo&e=">https://msdn.microsoft.com/en-us/library/34h23df8(v=vs.100).aspx#sectionToggle0</a></div></div></div></div></div></blockquote>Thanks for the explanation. That is pretty odd behaviour IMO.<div><br></div><div>Any thoughts on adding this as a compatibility warning to clang so that it tells us when something isn't going to work on MSVC?</div><div><br></div><div>Pete<br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">What would the fix be if we "needed" them here?<br></blockquote><div><br></div><div>We would have to make sure we don't provide a definition for NumUserOperandsBits in Value.cpp if we are compiling with MSVC or we annotate the definition with __declspec(selectany). I found the enum more appealing.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5"><br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/IR/Value.h<br>
> llvm/trunk/lib/IR/Value.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/IR/Value.h<br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_IR_Value.h-3Frev-3D241958-26r1-3D241957-26r2-3D241958-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=vlpPDUd2DT1f1slZ2uCdvrX-x01vntSI1jH0dBr988U&s=C4zUCYNdRK3aS4U3QFmPrGDYkXRvbeSjcqWqkSlWpO8&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Value.h?rev=241958&r1=241957&r2=241958&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/IR/Value.h (original)<br>
> +++ llvm/trunk/include/llvm/IR/Value.h Fri Jul 10 17:46:02 2015<br>
> @@ -104,8 +104,8 @@ protected:<br>
> ///<br>
> /// Note, this should *NOT* be used directly by any class other than User.<br>
> /// User uses this value to find the Use list.<br>
> - static const unsigned NumUserOperandsBits = 29;<br>
> - unsigned NumUserOperands : 29;<br>
> + enum : unsigned { NumUserOperandsBits = 29 };<br>
> + unsigned NumUserOperands : NumUserOperandsBits;<br>
><br>
> bool IsUsedByMD : 1;<br>
> bool HasName : 1;<br>
><br>
> Modified: llvm/trunk/lib/IR/Value.cpp<br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_IR_Value.cpp-3Frev-3D241958-26r1-3D241957-26r2-3D241958-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=vlpPDUd2DT1f1slZ2uCdvrX-x01vntSI1jH0dBr988U&s=xGnX0AAjYOHYfl037oJV7oumOeFGpPDP3YTq3Hv97qE&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Value.cpp?rev=241958&r1=241957&r2=241958&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/Value.cpp (original)<br>
> +++ llvm/trunk/lib/IR/Value.cpp Fri Jul 10 17:46:02 2015<br>
> @@ -39,8 +39,6 @@ using namespace llvm;<br>
> //===----------------------------------------------------------------------===//<br>
> // Value Class<br>
> //===----------------------------------------------------------------------===//<br>
> -const unsigned Value::NumUserOperandsBits;<br>
> -<br>
> static inline Type *checkType(Type *Ty) {<br>
> assert(Ty && "Value defined with a null type: Error!");<br>
> return Ty;<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>
</div></blockquote></div></body></html>