[llvm] r241958 - [IR] Switch static const to an enum to silence MSVC linker warnings

[Pete Cooper]

Sent from my iPhone

> On Jul 10, 2015, at 4:13 PM, David Majnemer <david.majnemer at gmail.com> wrote:
> 
> 
> 
>> On Fri, Jul 10, 2015 at 4:01 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> > On 2015-Jul-10, at 15:46, David Majnemer <david.majnemer at gmail.com> wrote:
>> >
>> > Author: majnemer
>> > Date: Fri Jul 10 17:46:02 2015
>> > New Revision: 241958
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=241958&view=rev
>> > Log:
>> > [IR] Switch static const to an enum to silence MSVC linker warnings
>> >
>> > Integral class statics are handled oddly in MSVC, we don't need them
>> > in this case, use an enum instead.
>> 
>> Oddly, how? 
> 
> say we have foo.h with:
> struct S {
>   static const int x = 3;
> };
> 
> and we have foo.cpp with:
> #include "foo.h"
> const int S::x;
> 
> 
> 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.
> 
> 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.
> 
> This is documented, intentional, behavior of MSVC: https://msdn.microsoft.com/en-us/library/34h23df8(v=vs.100).aspx#sectionToggle0
Thanks for the explanation. That is pretty odd behaviour IMO.

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?

Pete
>  
>> What would the fix be if we "needed" them here?
> 
> 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.
>  
>> 
>> >
>> > Modified:
>> >    llvm/trunk/include/llvm/IR/Value.h
>> >    llvm/trunk/lib/IR/Value.cpp
>> >
>> > Modified: llvm/trunk/include/llvm/IR/Value.h
>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Value.h?rev=241958&r1=241957&r2=241958&view=diff
>> > ==============================================================================
>> > --- llvm/trunk/include/llvm/IR/Value.h (original)
>> > +++ llvm/trunk/include/llvm/IR/Value.h Fri Jul 10 17:46:02 2015
>> > @@ -104,8 +104,8 @@ protected:
>> >   ///
>> >   /// Note, this should *NOT* be used directly by any class other than User.
>> >   /// User uses this value to find the Use list.
>> > -  static const unsigned NumUserOperandsBits = 29;
>> > -  unsigned NumUserOperands : 29;
>> > +  enum : unsigned { NumUserOperandsBits = 29 };
>> > +  unsigned NumUserOperands : NumUserOperandsBits;
>> >
>> >   bool IsUsedByMD : 1;
>> >   bool HasName : 1;
>> >
>> > Modified: llvm/trunk/lib/IR/Value.cpp
>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Value.cpp?rev=241958&r1=241957&r2=241958&view=diff
>> > ==============================================================================
>> > --- llvm/trunk/lib/IR/Value.cpp (original)
>> > +++ llvm/trunk/lib/IR/Value.cpp Fri Jul 10 17:46:02 2015
>> > @@ -39,8 +39,6 @@ using namespace llvm;
>> > //===----------------------------------------------------------------------===//
>> > //                                Value Class
>> > //===----------------------------------------------------------------------===//
>> > -const unsigned Value::NumUserOperandsBits;
>> > -
>> > static inline Type *checkType(Type *Ty) {
>> >   assert(Ty && "Value defined with a null type: Error!");
>> >   return Ty;
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150710/d0ac68e7/attachment.html>