[MSVC] Handle out-of-line definition of static data member correctly (fix for http://llvm.org/PR21164)

[alexfrol]

Summary

There are 3 cases of defining static const member:

1. initialized inside the class, not defined outside the class.
2. initialized inside the class, defined outside the class.
3. not initialized inside the class, defined outside the class.

Revision r213304 was supposed to fix the linkage problem of case (1), but mistakenly it made case (2) behave the same.
As a result, out-of-line definition of static data member is not handled correctly.
Proposed patch distinguishes between cases (1) and (2) and allows to properly emit static const members under –fms-compatibility option.

This fixes http://llvm.org/PR21164.

[rnk]

lib/AST/ASTContext.cpp

4910 ↗

(On Diff #26051)

Can the last two lines be simplified to this?

VD->isFirstDecl() && VD->hasInit()

The first declaration of a static data member cannot be out of line, unless I'm mistaken.

test/CodeGenCXX/dllexport-members.cpp

113–115 ↗

(On Diff #26051)

Can you add at least one test here that exercises the weak_odr linkage case? Do something like this, where the member is referenced but not defined outside the class:

struct A { static const int V = 42; };
int f() { return A::V; }

test/CodeGenCXX/ms-integer-static-data-members.cpp

1–10 ↗

(On Diff #26051)

I think the explosion in RUN lines here is getting out of hand. We can test all the cases in a single TU if we just have multiple static data members in S. Do you mind doing that cleanup?

[alexfrol]

Thank you for the quick review!
I updated the patch according to your notes.

[rnk]

lgtm, thanks for the patch!

cfe/trunk/lib/AST/ASTContext.cpp

CharUnits ASTContext::getObjCEncodingTypeSize(QualType type) const {
   else if (type->isArrayType())
     sz = getTypeSizeInChars(VoidPtrTy);
   return sz;
 }
 
 bool ASTContext::isMSStaticDataMemberInlineDefinition(const VarDecl *VD) const {
   return getLangOpts().MSVCCompat && VD->isStaticDataMember() &&
          VD->getType()->isIntegralOrEnumerationType() &&
-         !VD->getFirstDecl()->isOutOfLine() && VD->getFirstDecl()->hasInit();
+         VD->isFirstDecl() && !VD->isOutOfLine() && VD->hasInit();
 }
 
 static inline 
 std::string charUnitsToString(const CharUnits &CU) {
   return llvm::itostr(CU.getQuantity());
 }
 
 /// getObjCEncodingForBlock - Return the encoded type for this block

cfe/trunk/test/CodeGenCXX/dllexport-members.cpp

private:
   // M64-DAG: define                                   void @"\01?ignored@ExportMembers@@QEAAXXZ"(%struct.ExportMembers* %this)
   // G32-DAG: define                    x86_thiscallcc void @_ZN13ExportMembers7ignoredEv(%struct.ExportMembers* %this)
   // G64-DAG: define                                   void @_ZN13ExportMembers7ignoredEv(%struct.ExportMembers* %this)
 public:
   void ignored();
 
   // MSC-DAG: @"\01?StaticField@ExportMembers@@2HA"               = dllexport global i32 1, align 4
   // MSC-DAG: @"\01?StaticConstField@ExportMembers@@2HB"          = dllexport constant i32 1, align 4
-  // MSC-DAG: @"\01?StaticConstFieldEqualInit@ExportMembers@@2HB" = weak_odr dllexport constant i32 1, comdat, align 4
-  // MSC-DAG: @"\01?StaticConstFieldBraceInit@ExportMembers@@2HB" = weak_odr dllexport constant i32 1, comdat, align 4
-  // MSC-DAG: @"\01?ConstexprField@ExportMembers@@2HB"            = weak_odr dllexport constant i32 1, comdat, align 4
+  // MSC-DAG: @"\01?StaticConstFieldEqualInit@ExportMembers@@2HB" = dllexport constant i32 1, comdat, align 4
+  // MSC-DAG: @"\01?StaticConstFieldBraceInit@ExportMembers@@2HB" = dllexport constant i32 1, comdat, align 4
+  // MSC-DAG: @"\01?StaticConstFieldRefNotDef@ExportMembers@@2HB" = weak_odr dllexport constant i32 1, comdat, align 4
+  // MSC-DAG: @"\01?ConstexprField@ExportMembers@@2HB"            = dllexport constant i32 1, comdat, align 4
   // GNU-DAG: @_ZN13ExportMembers11StaticFieldE                   = dllexport global i32 1, align 4
   // GNU-DAG: @_ZN13ExportMembers16StaticConstFieldE              = dllexport constant i32 1, align 4
   // GNU-DAG: @_ZN13ExportMembers25StaticConstFieldEqualInitE     = dllexport constant i32 1, align 4
   // GNU-DAG: @_ZN13ExportMembers25StaticConstFieldBraceInitE     = dllexport constant i32 1, align 4
   // GNU-DAG: @_ZN13ExportMembers14ConstexprFieldE                = dllexport constant i32 1, align 4
   __declspec(dllexport) static         int  StaticField;
   __declspec(dllexport) static  const  int  StaticConstField;
   __declspec(dllexport) static  const  int  StaticConstFieldEqualInit = 1;
   __declspec(dllexport) static  const  int  StaticConstFieldBraceInit{1};
+  __declspec(dllexport) static  const  int  StaticConstFieldRefNotDef = 1;
   __declspec(dllexport) constexpr static int ConstexprField = 1;
 };
 
        void ExportMembers::normalDef() {}
 inline void ExportMembers::normalInlineDef() {}
        void ExportMembers::normalInlineDecl() {}
        void ExportMembers::virtualDef() {}
 inline void ExportMembers::virtualInlineDef() {}
        void ExportMembers::virtualInlineDecl() {}
        void ExportMembers::staticDef() {}
 inline void ExportMembers::staticInlineDef() {}
        void ExportMembers::staticInlineDecl() {}
        void ExportMembers::ignored() {}
        void ExportMembers::protectedDef() {}
        void ExportMembers::protectedStaticDef() {}
        void ExportMembers::privateDef() {}
        void ExportMembers::privateStaticDef() {}
 
        int  ExportMembers::StaticField = 1;
 const  int  ExportMembers::StaticConstField = 1;
 const  int  ExportMembers::StaticConstFieldEqualInit;
 const  int  ExportMembers::StaticConstFieldBraceInit;
+int foo() { return ExportMembers::StaticConstFieldRefNotDef; }
 constexpr int ExportMembers::ConstexprField;
 
 
 // Export individual members of a nested class.
 struct ExportMembers::Nested {
   // M32-DAG: define          dllexport x86_thiscallcc void @"\01?normalDef@Nested@ExportMembers@@QAEXXZ"(%"struct.ExportMembers::Nested"* %this)
   // M64-DAG: define          dllexport                void @"\01?normalDef@Nested@ExportMembers@@QEAAXXZ"(%"struct.ExportMembers::Nested"* %this)
   // M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?normalInclass@Nested@ExportMembers@@QAEXXZ"(%"struct.ExportMembers::Nested"* %this)
private:
   // M64-DAG: define                                   void @"\01?ignored@Nested@ExportMembers@@QEAAXXZ"(%"struct.ExportMembers::Nested"* %this)
   // G32-DAG: define                    x86_thiscallcc void @_ZN13ExportMembers6Nested7ignoredEv(%"struct.ExportMembers::Nested"* %this)
   // G64-DAG: define                                   void @_ZN13ExportMembers6Nested7ignoredEv(%"struct.ExportMembers::Nested"* %this)
 public:
   void ignored();
 
   // MSC-DAG: @"\01?StaticField@Nested@ExportMembers@@2HA"               = dllexport global i32 1, align 4
   // MSC-DAG: @"\01?StaticConstField@Nested@ExportMembers@@2HB"          = dllexport constant i32 1, align 4
-  // MSC-DAG: @"\01?StaticConstFieldEqualInit@Nested@ExportMembers@@2HB" = weak_odr dllexport constant i32 1, comdat, align 4
-  // MSC-DAG: @"\01?StaticConstFieldBraceInit@Nested@ExportMembers@@2HB" = weak_odr dllexport constant i32 1, comdat, align 4
-  // MSC-DAG: @"\01?ConstexprField@Nested@ExportMembers@@2HB"            = weak_odr dllexport constant i32 1, comdat, align 4
+  // MSC-DAG: @"\01?StaticConstFieldEqualInit@Nested@ExportMembers@@2HB" = dllexport constant i32 1, comdat, align 4
+  // MSC-DAG: @"\01?StaticConstFieldBraceInit@Nested@ExportMembers@@2HB" = dllexport constant i32 1, comdat, align 4
+  // MSC-DAG: @"\01?StaticConstFieldRefNotDef@Nested@ExportMembers@@2HB" = weak_odr dllexport constant i32 1, comdat, align 4
+  // MSC-DAG: @"\01?ConstexprField@Nested@ExportMembers@@2HB"            = dllexport constant i32 1, comdat, align 4
   // GNU-DAG: @_ZN13ExportMembers6Nested11StaticFieldE                   = dllexport global i32 1, align 4
   // GNU-DAG: @_ZN13ExportMembers6Nested16StaticConstFieldE              = dllexport constant i32 1, align 4
   // GNU-DAG: @_ZN13ExportMembers6Nested25StaticConstFieldEqualInitE     = dllexport constant i32 1, align 4
   // GNU-DAG: @_ZN13ExportMembers6Nested25StaticConstFieldBraceInitE     = dllexport constant i32 1, align 4
   // GNU-DAG: @_ZN13ExportMembers6Nested14ConstexprFieldE                = dllexport constant i32 1, align 4
   __declspec(dllexport) static         int  StaticField;
   __declspec(dllexport) static  const  int  StaticConstField;
   __declspec(dllexport) static  const  int  StaticConstFieldEqualInit = 1;
   __declspec(dllexport) static  const  int  StaticConstFieldBraceInit{1};
+  __declspec(dllexport) static  const  int  StaticConstFieldRefNotDef = 1;
   __declspec(dllexport) constexpr static int ConstexprField = 1;
 };
 
        void ExportMembers::Nested::normalDef() {}
 inline void ExportMembers::Nested::normalInlineDef() {}
        void ExportMembers::Nested::normalInlineDecl() {}
        void ExportMembers::Nested::virtualDef() {}
 inline void ExportMembers::Nested::virtualInlineDef() {}
        void ExportMembers::Nested::virtualInlineDecl() {}
        void ExportMembers::Nested::staticDef() {}
 inline void ExportMembers::Nested::staticInlineDef() {}
        void ExportMembers::Nested::staticInlineDecl() {}
        void ExportMembers::Nested::ignored() {}
        void ExportMembers::Nested::protectedDef() {}
        void ExportMembers::Nested::protectedStaticDef() {}
        void ExportMembers::Nested::privateDef() {}
        void ExportMembers::Nested::privateStaticDef() {}
 
        int  ExportMembers::Nested::StaticField = 1;
 const  int  ExportMembers::Nested::StaticConstField = 1;
 const  int  ExportMembers::Nested::StaticConstFieldEqualInit;
 const  int  ExportMembers::Nested::StaticConstFieldBraceInit;
+int fooNested() { return ExportMembers::Nested::StaticConstFieldRefNotDef; }
 constexpr int ExportMembers::Nested::ConstexprField;
 
 
 // Export special member functions.
 struct ExportSpecials {
   // M32-DAG: define dllexport x86_thiscallcc %struct.ExportSpecials* @"\01??0ExportSpecials@@QAE@XZ"(%struct.ExportSpecials* returned %this)
   // M64-DAG: define dllexport                %struct.ExportSpecials* @"\01??0ExportSpecials@@QEAA@XZ"(%struct.ExportSpecials* returned %this)
   // G32-DAG: define dllexport x86_thiscallcc void @_ZN14ExportSpecialsC1Ev(%struct.ExportSpecials* %this)

cfe/trunk/test/CodeGenCXX/ms-integer-static-data-members.cpp

 // RUN: %clang_cc1 -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s
-// RUN: %clang_cc1 -DINLINE_INIT -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s --check-prefix=CHECK-INLINE
-// RUN: %clang_cc1 -DREAL_DEFINITION -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s --check-prefix=CHECK-OUTOFLINE
-// RUN: %clang_cc1 -DINLINE_INIT -DREAL_DEFINITION -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s --check-prefix=CHECK-INLINE
 
 struct S {
-  // For MS ABI, we emit a linkonce_odr definition here, even though it's really just a declaration.
-#ifdef INLINE_INIT
-  static const int x = 5;
-#else
-  static const int x;
-#endif
+  static const int NoInit_Ref;
+  static const int Inline_NotDef_NotRef = 5;
+  static const int Inline_NotDef_Ref = 5;
+  static const int Inline_Def_NotRef = 5;
+  static const int Inline_Def_Ref = 5;
+  static const int OutOfLine_Def_NotRef;
+  static const int OutOfLine_Def_Ref;
 };
 
-const int *f() {
-  return &S::x;
+const int *foo1() {
+  return &S::NoInit_Ref;
 };
 
-#ifdef REAL_DEFINITION
-#ifdef INLINE_INIT
-const int S::x;
-#else
-const int S::x = 5;
-#endif
-#endif
+const int *foo2() {
+  return &S::Inline_NotDef_Ref;
+};
+
+const int *foo3() {
+  return &S::Inline_Def_Ref;
+};
 
+const int *foo4() {
+    return &S::OutOfLine_Def_Ref;
+};
 
-// Inline initialization.
-// CHECK-INLINE: @"\01?x@S@@2HB" = linkonce_odr constant i32 5, comdat, align 4
+const int S::Inline_Def_NotRef;
+const int S::Inline_Def_Ref;
+const int S::OutOfLine_Def_NotRef = 5;
+const int S::OutOfLine_Def_Ref = 5;
 
-// Out-of-line initialization.
-// CHECK-OUTOFLINE: @"\01?x@S@@2HB" = constant i32 5, align 4
 
 // No initialization.
-// CHECK: @"\01?x@S@@2HB" = external constant i32
+// CHECK-DAG: @"\01?NoInit_Ref@S@@2HB" = external constant i32
+
+// Inline initialization, no real definiton, not referenced.
+// CHECK-NOT: @"\01?Inline_NotDef_NotRef@S@@2HB" = {{.*}} constant i32 5
+
+// Inline initialization, no real definiton, referenced.
+// CHECK-DAG: @"\01?Inline_NotDef_Ref@S@@2HB" = linkonce_odr constant i32 5, comdat, align 4
+
+// Inline initialization, real definiton, not referenced.
+// CHECK-DAG: @"\01?Inline_Def_NotRef@S@@2HB" = constant i32 5, align 4
+
+// Inline initialization, real definiton, referenced.
+// CHECK-DAG: @"\01?Inline_Def_Ref@S@@2HB" = constant i32 5, comdat, align 4
+
+// Out-of-line initialization.
+// CHECK-DAG: @"\01?OutOfLine_Def_NotRef@S@@2HB" = constant i32 5, align 4
+// CHECK-DAG: @"\01?OutOfLine_Def_Ref@S@@2HB" = constant i32 5, align 4