r216340 - Don't assert on different DLL attributes in template and explicit instantiation (PR20137)

[Hans Wennborg]

Author: hans
Date: Sat Aug 23 19:12:36 2014
New Revision: 216340

URL: http://llvm.org/viewvc/llvm-project?rev=216340&view=rev
Log:
Don't assert on different DLL attributes in template and explicit instantiation (PR20137)

We would previously assert (a decl cannot have two DLL attributes) on this code:

  template <typename T> struct __declspec(dllimport) S { T f() { return T(); } };
  template struct __declspec(dllexport) S<int>;

The problem was that when instantiating, we would take the attribute from the
template even if the instantiation itself already had an attribute.

Also, don't inherit DLL attributes from the template to its members before
instantiation, as the attribute may change.

I couldn't figure out what MinGW does here, so I'm leaving that open. At least
we're not asserting anymore.

Modified:
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
    cfe/trunk/test/CodeGenCXX/dllexport.cpp
    cfe/trunk/test/CodeGenCXX/dllimport.cpp
    cfe/trunk/test/SemaCXX/dllimport.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=216340&r1=216339&r2=216340&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sat Aug 23 19:12:36 2014
@@ -4437,6 +4437,28 @@ static void checkDLLAttribute(Sema &S, C
   if (!ClassAttr)
     return;
 
+  if (S.Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+      !ClassAttr->isInherited()) {
+    // Diagnose dll attributes on members of class with dll attribute.
+    for (Decl *Member : Class->decls()) {
+      if (!isa<VarDecl>(Member) && !isa<CXXMethodDecl>(Member))
+        continue;
+      InheritableAttr *MemberAttr = getDLLAttr(Member);
+      if (!MemberAttr || MemberAttr->isInherited() || Member->isInvalidDecl())
+        continue;
+
+      S.Diag(MemberAttr->getLocation(),
+             diag::err_attribute_dll_member_of_dll_class)
+          << MemberAttr << ClassAttr;
+      S.Diag(ClassAttr->getLocation(), diag::note_previous_attribute);
+      Member->setInvalidDecl();
+    }
+  }
+
+  if (Class->getDescribedClassTemplate())
+    // Don't inherit dll attribute until the template is instantiated.
+    return;
+
   bool ClassExported = ClassAttr->getKind() == attr::DLLExport;
 
   // Force declaration of implicit members so they can inherit the attribute.
@@ -4467,17 +4489,7 @@ static void checkDLLAttribute(Sema &S, C
       continue;
     }
 
-    if (InheritableAttr *MemberAttr = getDLLAttr(Member)) {
-      if (S.Context.getTargetInfo().getCXXABI().isMicrosoft() &&
-          !MemberAttr->isInherited() && !ClassAttr->isInherited()) {
-        S.Diag(MemberAttr->getLocation(),
-               diag::err_attribute_dll_member_of_dll_class)
-            << MemberAttr << ClassAttr;
-        S.Diag(ClassAttr->getLocation(), diag::note_previous_attribute);
-        Member->setInvalidDecl();
-        continue;
-      }
-    } else {
+    if (!getDLLAttr(Member)) {
       auto *NewAttr =
           cast<InheritableAttr>(ClassAttr->clone(S.getASTContext()));
       NewAttr->setInherited(true);

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=216340&r1=216339&r2=216340&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Sat Aug 23 19:12:36 2014
@@ -183,6 +183,14 @@ void Sema::InstantiateAttrs(const MultiL
       continue;
     }
 
+    // Existing DLL attribute on the instantiation takes precedence.
+    if (TmplAttr->getKind() == attr::DLLExport ||
+        TmplAttr->getKind() == attr::DLLImport) {
+      if (New->hasAttr<DLLExportAttr>() || New->hasAttr<DLLImportAttr>()) {
+        continue;
+      }
+    }
+
     assert(!TmplAttr->isPackExpansion());
     if (TmplAttr->isLateParsed() && LateAttrs) {
       // Late parsed attributes must be instantiated and attached after the

Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=216340&r1=216339&r2=216340&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Sat Aug 23 19:12:36 2014
@@ -600,6 +600,12 @@ USEMEMFUNC(PartiallySpecializedExportedC
 // M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?f@?$PartiallySpecializedExportedClassTemplate2 at PAX@@QAEXXZ"
 // G32-DAG: declare dllimport x86_thiscallcc void @_ZN42PartiallySpecializedExportedClassTemplate2IPvE1fEv
 
+// Attributes on the instantiation take precedence over attributes on the template.
+template <typename T> struct __declspec(dllimport) ExplicitlyInstantiatedWithDifferentAttr { void f() {} };
+template struct __declspec(dllexport) ExplicitlyInstantiatedWithDifferentAttr<int>;
+USEMEMFUNC(ExplicitlyInstantiatedWithDifferentAttr<int>, f);
+// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?f@?$ExplicitlyInstantiatedWithDifferentAttr at H@@QAEXXZ"
+
 
 //===----------------------------------------------------------------------===//
 // Classes with template base classes

Modified: cfe/trunk/test/CodeGenCXX/dllimport.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport.cpp?rev=216340&r1=216339&r2=216340&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/dllimport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllimport.cpp Sat Aug 23 19:12:36 2014
@@ -693,6 +693,12 @@ USEMEMFUNC(PartiallySpecializedImportedC
 // M32-DAG: {{declare|define available_externally}} dllimport x86_thiscallcc void @"\01?f@?$PartiallySpecializedImportedClassTemplate at PAX@@QAEXXZ"
 // G32-DAG: define linkonce_odr x86_thiscallcc void @_ZN41PartiallySpecializedImportedClassTemplateIPvE1fEv
 
+// Attributes on the instantiation take precedence over attributes on the template.
+template <typename T> struct __declspec(dllexport) ExplicitlyInstantiatedWithDifferentAttr { void f() {} };
+template struct __declspec(dllimport) ExplicitlyInstantiatedWithDifferentAttr<int>;
+USEMEMFUNC(ExplicitlyInstantiatedWithDifferentAttr<int>, f);
+// M32-DAG: {{declare|define available_externally}} dllimport x86_thiscallcc void @"\01?f@?$ExplicitlyInstantiatedWithDifferentAttr at H@@QAEXXZ"
+
 
 //===----------------------------------------------------------------------===//
 // Classes with template base classes

Modified: cfe/trunk/test/SemaCXX/dllimport.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/dllimport.cpp?rev=216340&r1=216339&r2=216340&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/dllimport.cpp (original)
+++ cfe/trunk/test/SemaCXX/dllimport.cpp Sat Aug 23 19:12:36 2014
@@ -981,7 +981,7 @@ class __declspec(dllimport) ImportClassW
 // expected-error at +4{{attribute 'dllimport' cannot be applied to member of 'dllexport' class}}
 // expected-error at +4{{attribute 'dllexport' cannot be applied to member of 'dllexport' class}}
 #endif
-class __declspec(dllexport) ExportClassWithDllMember {
+template <typename T> class __declspec(dllexport) ExportClassWithDllMember {
   void __declspec(dllimport) foo();
   void __declspec(dllexport) bar();
 };