Fix for PR32992. Static const classes not exported.

[hans]

Interesting!

lib/Sema/SemaDeclCXX.cpp

5479

Maybe rename this to ReferenceDllExportedMembers now that it applies to variables as well.

5504

Let's move the MD part down until where it's used.

5507

The intendation looks off here. Can you run it through clang-format maybe? It would also be good with a comment explaining why the code is here.

5509

Does the 'const' really make a difference for whether the member should be exported?

test/SemaCXX/export_static_const.cpp

1 ↗

(On Diff #132999)

This test doesn't seem to have any "RUN" line, which means nothing actually gets tested.

Can you add a case to test/CodeGenCXX/dllexport.cpp instead?

[zahiraam]

lib/Sema/SemaDeclCXX.cpp

5509

No it doesn't. I removed that condition.

[hans]

Thanks! Just a few more comments.

lib/Sema/SemaDeclCXX.cpp

5500

Since VD is a class member, I think isDefinedOutsideFunctionOrMethod() is always true and not interesting to check.

test/CodeGenCXX/dllexport.cpp

571

Can you move this check down where the class etc. are declared below? Maybe also add a comment explaining what it's testing and reference the bug number.

984

No need for the semicolon in the empty function, here and below. The rest of the file doesn't use this style.

[zahiraam]

[hans]

Looks good to me. Do you have commit access or would you like me to commit it for you?

[zahiraam]

In D42968#1004673, @hans wrote:

Looks good to me. Do you have commit access or would you like me to commit it for you?

Hello.
Please go ahead and to the commit.
Thanks.

[zahiraam]

Fixing the assert in the build.

[zahiraam]

[hans]

In D42968#1013662, @zahiraam wrote:

Fixing the assert in the build.

I don't see any difference compared to the previous version. What is the fix for the assert?

[zahiraam]

Added a test case to check the suppression of the warning.

[hans]

lib/Sema/SemaDeclCXX.cpp

5501

I wonder if this is the right thing to do..

Would MarkVariableReferenced work instead, similarly to how MarkFunctionReferenced is used for the methods below?

[zahiraam]

lib/Sema/SemaDeclCXX.cpp

5501

Yes.

[hans]

I think you only uploaded the diff between this and the previous version. But the S.MarkVariableReferenced version LGTM. Do you want me to commit it for you?

[zahiraam]

In D42968#1019198, @hans wrote:

I think you only uploaded the diff between this and the previous version. But the S.MarkVariableReferenced version LGTM. Do you want me to commit it for you?

Yes only the diff. Do you want the entire diff?
If no please go ahead and commit it.
Thanks.

[hans]

In D42968#1019201, @zahiraam wrote:

In D42968#1019198, @hans wrote:

I think you only uploaded the diff between this and the previous version. But the S.MarkVariableReferenced version LGTM. Do you want me to commit it for you?

Yes only the diff. Do you want the entire diff?
If no please go ahead and commit it.
Thanks.

r326089

lib/Sema/SemaDeclCXX.cpp

// Nested classes and class templates.
       CheckAbstractClassUsage(Info, cast<CXXRecordDecl>(D));
     } else if (isa<ClassTemplateDecl>(D)) {
       CheckAbstractClassUsage(Info,
                              cast<ClassTemplateDecl>(D)->getTemplatedDecl());
     }
   }
 }
 
-static void ReferenceDllExportedMethods(Sema &S, CXXRecordDecl *Class) {
+static void ReferenceDllExportedMembers(Sema &S, CXXRecordDecl *Class) {
   Attr *ClassAttr = getDLLAttr(Class);
   if (!ClassAttr)
     return;
 
   assert(ClassAttr->getKind() == attr::DLLExport);
 
   TemplateSpecializationKind TSK = Class->getTemplateSpecializationKind();
 
   if (TSK == TSK_ExplicitInstantiationDeclaration)
     // Don't go any further if this is just an explicit instantiation
     // declaration.
     return;
 
   for (Decl *Member : Class->decls()) {
+    // Defined static variables that are members of an exported base
+    // class must be marked export too. Push them to implicit instantiation
+    // queue.
+    auto *VD = dyn_cast<VarDecl>(Member);
+    if (VD && Member->getAttr<DLLExportAttr>() &&
+        VD->getStorageClass() == SC_Static &&
+        TSK == TSK_ImplicitInstantiation)
+      S.PendingLocalImplicitInstantiations.push_back(
+          std::make_pair(VD, VD->getLocation()));
+
     auto *MD = dyn_cast<CXXMethodDecl>(Member);
     if (!MD)
       continue;
 
     if (Member->getAttr<DLLExportAttr>()) {
       if (MD->isUserProvided()) {
         // Instantiate non-default class member functions ...
 
         // .. except for certain kinds of template specializations.
         if (TSK == TSK_ImplicitInstantiation && !ClassAttr->isInherited())
           continue;
 
         S.MarkFunctionReferenced(Class->getLocation(), MD);
 

 }
 
 void Sema::ActOnFinishCXXNonNestedClass(Decl *D) {
   referenceDLLExportedClassMethods();
 }
 
 void Sema::referenceDLLExportedClassMethods() {
   if (!DelayedDllExportClasses.empty()) {
-    // Calling ReferenceDllExportedMethods might cause the current function to
+    // Calling ReferenceDllExportedMembers might cause the current function to
     // be called again, so use a local copy of DelayedDllExportClasses.
     SmallVector<CXXRecordDecl *, 4> WorkList;
     std::swap(DelayedDllExportClasses, WorkList);
     for (CXXRecordDecl *Class : WorkList)
-      ReferenceDllExportedMethods(*this, Class);
+      ReferenceDllExportedMembers(*this, Class);
   }
 }
 
 void Sema::AdjustDestructorExceptionSpec(CXXRecordDecl *ClassDecl,
                                          CXXDestructorDecl *Destructor) {
   assert(getLangOpts().CPlusPlus11 &&
          "adjusting dtor exception specs was introduced in c++11");
 

test/CodeGenCXX/dllexport.cpp

 #define USEVAR(var) int UNIQ(use)() { return var; }
 #define USE(func) void UNIQ(use)() { func(); }
 #define USEMEMFUNC(class, func) void (class::*UNIQ(use)())() { return &class::func; }
 #define INSTVAR(var) template int var;
 #define INST(func) template void func();
 
 // The vftable for struct W is comdat largest because we have RTTI.
 // M32-DAG: $"\01??_7W@@6B@" = comdat largest
+// M32-DAG: $"\01?smember@?$Base@H@PR32992@@0HA" = comdat any
 
 
 //===----------------------------------------------------------------------===//
 // Globals
 //===----------------------------------------------------------------------===//
 
 // Declarations are not exported.
 // MSC-NOT: @"\01?ExternGlobalDecl@@3HA"

 
 struct __declspec(dllexport) W { virtual void foo(); };
 void W::foo() {}
 // Default ctor:
 // M32-DAG: define weak_odr dllexport x86_thiscallcc %struct.W* @"\01??0W@@QAE@XZ"
 // Copy ctor:
 // M32-DAG: define weak_odr dllexport x86_thiscallcc %struct.W* @"\01??0W@@QAE@ABU0@@Z"
 // vftable:
 // M32-DAG: [[W_VTABLE:@.*]] = private unnamed_addr constant { [2 x i8*] } { [2 x i8*] [i8* bitcast (%rtti.CompleteObjectLocator* @"\01??_R4W@@6B@" to i8*), i8* bitcast (void (%struct.W*)* @"\01?foo@W@@UAEXXZ" to i8*)] }, comdat($"\01??_7W@@6B@")
 // M32-DAG: @"\01??_7W@@6B@" = dllexport unnamed_addr alias i8*, getelementptr inbounds ({ [2 x i8*] }, { [2 x i8*] }* [[W_VTABLE]], i32 0, i32 0, i32 1)
 // G32-DAG: @_ZTV1W = dllexport unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1W to i8*), i8* bitcast (void (%struct.W*)* @_ZN1W3fooEv to i8*)] }
 
 struct __declspec(dllexport) X : public virtual W {};
 // vbtable:
 // M32-DAG: @"\01??_8X@@7B@" = weak_odr dllexport unnamed_addr constant [2 x i32] [i32 0, i32 4]
 
 struct __declspec(dllexport) Y {

 public:
   virtual ~ACE_Shared_Object();
 };
 class __declspec(dllexport) ACE_Service_Object : public ACE_Shared_Object {};
 // Implicit move constructor declaration.
 // MSVC2015-DAG: define weak_odr dllexport {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
 // The declarations should not be exported.
 // MSVC2013-NOT: define weak_odr dllexport {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
+
+namespace PR32992 {
+// Static data members of a instantiated base class should be exported.
+template <class T>
+class Base {
+  virtual void myfunc() {}
+  static int smember;
+};
+// MS-DAG:  @"\01?smember@?$Base@H@PR32992@@0HA" = weak_odr dllexport global i32 77, comdat, align 4
+template <class T> int Base<T>::smember = 77;
+template <class T>
+class __declspec(dllexport) Derived2 : Base<T> {
+  void myfunc() {}
+};
+class Derived : public Derived2<int> {
+  void myfunc() {}
+};
+}  // namespace PR32992