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

[David Majnemer]

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


> 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/594f74a2/attachment.html>