[PATCH] D26657: [Sema] Respect DLL attributes more faithfully

[Shoaib Meenai via cfe-commits]

smeenai updated this revision to Diff 78324.
smeenai added a comment.

Moving explanation to comment instead of referencing Phabricator


https://reviews.llvm.org/D26657

Files:
  lib/Sema/SemaTemplate.cpp
  test/CodeGenCXX/dllexport.cpp
  test/CodeGenCXX/windows-itanium-dllexport.cpp


Index: test/CodeGenCXX/windows-itanium-dllexport.cpp
===================================================================
--- test/CodeGenCXX/windows-itanium-dllexport.cpp
+++ test/CodeGenCXX/windows-itanium-dllexport.cpp
@@ -23,3 +23,8 @@
 // CHECK: define {{.*}} dllexport {{.*}} @_ZN1cIcEaSERKS0_
 // CHECK: define {{.*}} dllexport {{.*}} @_ZN1cIcE1fEv
 
+c<double> g;
+template class __declspec(dllexport) c<double>;
+
+// CHECK: define {{.*}} dllexport {{.*}} @_ZN1cIdEaSERKS0_
+// CHECK: define {{.*}} dllexport {{.*}} @_ZN1cIdE1fEv
Index: test/CodeGenCXX/dllexport.cpp
===================================================================
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -771,6 +771,13 @@
 // M32-DAG: define weak_odr dllexport x86_thiscallcc %struct.ExplicitInstantiationDeclExportedDefTemplate* @"\01??0?$ExplicitInstantiationDeclExportedDefTemplate at H@@QAE at XZ"
 // G32-DAG: define weak_odr x86_thiscallcc void @_ZN44ExplicitInstantiationDeclExportedDefTemplateIiE1fEv
 
+template <typename T> struct ImplicitInstantiationExplicitInstantiationDefExportedTemplate { void f() {} };
+ImplicitInstantiationExplicitInstantiationDefExportedTemplate<int> ImplicitInstantiationExplicitInstantiationDefExportedTemplateInstance;
+template class __declspec(dllexport) ImplicitInstantiationExplicitInstantiationDefExportedTemplate<int>;
+USEMEMFUNC(ImplicitInstantiationExplicitInstantiationDefExportedTemplate<int>, f);
+// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?f@?$ImplicitInstantiationExplicitInstantiationDefExportedTemplate at H@@QAEXXZ"
+// G32-DAG: define weak_odr x86_thiscallcc void @_ZN61ImplicitInstantiationExplicitInstantiationDefExportedTemplateIiE1fEv
+
 namespace { struct InternalLinkageType {}; }
 struct __declspec(dllexport) PR23308 {
   void f(InternalLinkageType*);
Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -7663,20 +7663,31 @@
                                        Specialization->getDefinition());
   if (Def) {
     TemplateSpecializationKind Old_TSK = Def->getTemplateSpecializationKind();
-    // Fix a TSK_ExplicitInstantiationDeclaration followed by a
-    // TSK_ExplicitInstantiationDefinition
-    if (Old_TSK == TSK_ExplicitInstantiationDeclaration &&
-        (TSK == TSK_ExplicitInstantiationDefinition ||
-         DLLImportExplicitInstantiationDef)) {
+    // Fix a TSK_ExplicitInstantiationDeclaration or a TSK_ImplicitInstantiation
+    // followed by a TSK_ExplicitInstantiationDefinition
+    if ((Old_TSK == TSK_ExplicitInstantiationDeclaration &&
+         (TSK == TSK_ExplicitInstantiationDefinition ||
+          DLLImportExplicitInstantiationDef)) ||
+        (Old_TSK == TSK_ImplicitInstantiation &&
+         TSK == TSK_ExplicitInstantiationDefinition)) {
       // FIXME: Need to notify the ASTMutationListener that we did this.
       Def->setTemplateSpecializationKind(TSK);
 
-      if (!getDLLAttr(Def) && getDLLAttr(Specialization) &&
+      if (((!getDLLAttr(Def) && getDLLAttr(Specialization)) ||
+           (Old_TSK == TSK_ImplicitInstantiation &&
+            Specialization->hasAttr<DLLExportAttr>())) &&
           (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
            Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment())) {
         // In the MS ABI, an explicit instantiation definition can add a dll
-        // attribute to a template with a previous instantiation declaration.
-        // MinGW doesn't allow this.
+        // attribute to a template with a previous instantiation declaration
+        // or implicit instantiation. MinGW doesn't allow this. In the implicit
+        // instantiation case, we limit clang to only adding dllexport, to avoid
+        // potentially strange codegen behavior. For example, if we extend this
+        // conditional to dllimport, and we have a source file calling a method
+        // on an implicitly instantiated template class instance and then
+        // declaring a dllimport explicit instantiation definition for the same
+        // template class, the codegen for the method call will not respect the
+        // dllimport, while it will with cl.
         auto *A = cast<InheritableAttr>(
             getDLLAttr(Specialization)->clone(getASTContext()));
         A->setInherited(true);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26657.78324.patch
Type: text/x-patch
Size: 4410 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161117/181046c7/attachment.bin>