Keep inherited dllimport/export attrs for explicit specialization of …

…class template member functions Previously we were stripping these normally inherited attributes during explicit specialization. However for class template member functions (but not function templates), MSVC keeps the attribute. This makes Clang match that behavior, and fixes GitHub issue #54717 Differential revision: https://reviews.llvm.org/D135154
llvm · Oct 7, 2022 · c9b771b · c9b771b
1 parent e09aa0d
commit c9b771b
Show file tree

Hide file tree

Showing 8 changed files with 143 additions and 19 deletions.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -86,6 +86,36 @@ code bases.
   typedef char int8_a16 __attribute__((aligned(16)));
   int8_a16 array[4]; // Now diagnosed as the element size not being a multiple of the array alignment.
 
+- When compiling for Windows in MSVC compatibility mode (for example by using
+  clang-cl), the compiler will now propagate dllimport/export declspecs in
+  explicit specializations of class template member functions (`Issue 54717
+  <https://github.com/llvm/llvm-project/issues/54717>`_):
+
+  .. code-block:: c++
+
+    template <typename> struct __declspec(dllexport) S {
+      void f();
+    };
+    template<> void S<int>::f() {}  // clang-cl will now dllexport this.
+
+  This matches what MSVC does, so it improves compatibility, but it can also
+  cause errors for code which clang-cl would previously accept, for example:
+
+  .. code-block:: c++
+
+    template <typename> struct __declspec(dllexport) S {
+      void f();
+    };
+    template<> void S<int>::f() = delete;  // Error: cannot delete dllexport function.
+
+  .. code-block:: c++
+
+    template <typename> struct __declspec(dllimport) S {
+      void f();
+    };
+    template<> void S<int>::f() {};  // Error: cannot define dllimport function.
+
+  These errors also match MSVC's behavior.
 
 What's New in Clang |release|?
 ==============================

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3469,6 +3469,8 @@ def warn_attribute_dll_redeclaration : Warning<
   InGroup<DiagGroup<"dll-attribute-on-redeclaration">>;
 def err_attribute_dllimport_function_definition : Error<
   "dllimport cannot be applied to non-inline function definition">;
+def err_attribute_dllimport_function_specialization_definition : Error<
+  "cannot define non-inline dllimport template specialization">;
 def err_attribute_dll_deleted : Error<
   "attribute %q0 cannot be applied to a deleted function">;
 def err_attribute_dllimport_data_definition : Error<

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
@@ -7041,13 +7041,24 @@ static void checkDLLAttributeRedeclaration(Sema &S, NamedDecl *OldDecl,
       (!IsInline || (IsMicrosoftABI && IsTemplate)) && !IsStaticDataMember &&
       !NewDecl->isLocalExternDecl() && !IsQualifiedFriend) {
     if (IsMicrosoftABI && IsDefinition) {
-      S.Diag(NewDecl->getLocation(),
-             diag::warn_redeclaration_without_import_attribute)
-          << NewDecl;
-      S.Diag(OldDecl->getLocation(), diag::note_previous_declaration);
-      NewDecl->dropAttr<DLLImportAttr>();
-      NewDecl->addAttr(
-          DLLExportAttr::CreateImplicit(S.Context, NewImportAttr->getRange()));
+      if (IsSpecialization) {
+        S.Diag(
+            NewDecl->getLocation(),
+            diag::err_attribute_dllimport_function_specialization_definition);
+        S.Diag(OldImportAttr->getLocation(), diag::note_attribute);
+        NewDecl->dropAttr<DLLImportAttr>();
+      } else {
+        S.Diag(NewDecl->getLocation(),
+               diag::warn_redeclaration_without_import_attribute)
+            << NewDecl;
+        S.Diag(OldDecl->getLocation(), diag::note_previous_declaration);
+        NewDecl->dropAttr<DLLImportAttr>();
+        NewDecl->addAttr(DLLExportAttr::CreateImplicit(
+            S.Context, NewImportAttr->getRange()));
+      }
+    } else if (IsMicrosoftABI && IsSpecialization) {
+      assert(!IsDefinition);
+      // MSVC allows this. Keep the inherited attribute.
     } else {
       S.Diag(NewDecl->getLocation(),
              diag::warn_redeclaration_without_attribute_prev_attribute_ignored)

diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
@@ -8919,9 +8919,12 @@ void Sema::CheckConceptRedefinition(ConceptDecl *NewDecl,
 
 /// \brief Strips various properties off an implicit instantiation
 /// that has just been explicitly specialized.
-static void StripImplicitInstantiation(NamedDecl *D) {
-  D->dropAttr<DLLImportAttr>();
-  D->dropAttr<DLLExportAttr>();
+static void StripImplicitInstantiation(NamedDecl *D, bool MinGW) {
+  if (MinGW || (isa<FunctionDecl>(D) &&
+                cast<FunctionDecl>(D)->isFunctionTemplateSpecialization())) {
+    D->dropAttr<DLLImportAttr>();
+    D->dropAttr<DLLExportAttr>();
+  }
 
   if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
     FD->setInlineSpecified(false);
@@ -8996,7 +8999,9 @@ Sema::CheckSpecializationInstantiationRedecl(SourceLocation NewLoc,
       if (PrevPointOfInstantiation.isInvalid()) {
         // The declaration itself has not actually been instantiated, so it is
         // still okay to specialize it.
-        StripImplicitInstantiation(PrevDecl);
+        StripImplicitInstantiation(
+            PrevDecl,
+            Context.getTargetInfo().getTriple().isWindowsGNUEnvironment());
         return false;
       }
       // Fall through

diff --git a/clang/test/CodeGenCXX/dllexport-members.cpp b/clang/test/CodeGenCXX/dllexport-members.cpp
@@ -679,3 +679,21 @@ template __declspec(dllexport) const int MemVarTmpl::StaticVar<ExplicitInst_Expo
 // MSC-DAG: @"??$StaticVar@UExplicitSpec_Def_Exported@@@MemVarTmpl@@2HB" = weak_odr dso_local dllexport constant i32 1, comdat, align 4
 // GNU-DAG: @_ZN10MemVarTmpl9StaticVarI25ExplicitSpec_Def_ExportedEE        = dso_local dllexport constant i32 1, align 4
 template<> __declspec(dllexport) const int MemVarTmpl::StaticVar<ExplicitSpec_Def_Exported> = 1;
+
+
+//===----------------------------------------------------------------------===//
+// Class template members
+//===----------------------------------------------------------------------===//
+
+template <typename> struct ClassTmplMem {
+  void __declspec(dllexport) exportedNormal();
+  static void __declspec(dllexport) exportedStatic();
+};
+// MSVC exports explicit specialization of exported class template member function; MinGW does not.
+// M32-DAG: define dso_local dllexport x86_thiscallcc void @"?exportedNormal@?$ClassTmplMem@H@@QAEXXZ"
+// G32-DAG: define dso_local           x86_thiscallcc void @_ZN12ClassTmplMemIiE14exportedNormalEv
+template<> void ClassTmplMem<int>::exportedNormal() {}
+
+// M32-DAG: define dso_local dllexport void @"?exportedStatic@?$ClassTmplMem@H@@SAXXZ"
+// G32-DAG: define dso_local           void @_ZN12ClassTmplMemIiE14exportedStaticEv
+template<> void ClassTmplMem<int>::exportedStatic() {}
diff --git a/clang/test/CodeGenCXX/dllimport-members.cpp b/clang/test/CodeGenCXX/dllimport-members.cpp
@@ -875,3 +875,23 @@ USEMV(MemVarTmpl, StaticVar<ExplicitDecl_Imported>)
 // GNU-DAG: @_ZN10MemVarTmpl9StaticVarI21ExplicitSpec_ImportedEE        = external dllimport constant i32
 template<> __declspec(dllimport) const int MemVarTmpl::StaticVar<ExplicitSpec_Imported>;
 USEMV(MemVarTmpl, StaticVar<ExplicitSpec_Imported>)
+
+
+//===----------------------------------------------------------------------===//
+// Class template members
+//===----------------------------------------------------------------------===//
+
+template <typename> struct ClassTmplMem {
+  void __declspec(dllimport) importedNormal();
+  static void __declspec(dllimport) importedStatic();
+};
+// MSVC imports explicit specialization of imported class template member function; MinGW does not.
+// M32-DAG: declare dllimport x86_thiscallcc void @"?importedNormal@?$ClassTmplMem@H@@QAEXXZ"
+// G32-DAG: declare dso_local x86_thiscallcc void @_ZN12ClassTmplMemIiE14importedNormalEv
+template<> void ClassTmplMem<int>::importedNormal();
+USEMF(ClassTmplMem<int>, importedNormal);
+
+// M32-DAG: declare dllimport void @"?importedStatic@?$ClassTmplMem@H@@SAXXZ"
+// G32-DAG: declare dso_local void @_ZN12ClassTmplMemIiE14importedStaticEv
+template<> void ClassTmplMem<int>::importedStatic();
+USEMF(ClassTmplMem<int>, importedStatic);
diff --git a/clang/test/SemaCXX/dllexport.cpp b/clang/test/SemaCXX/dllexport.cpp
@@ -1,11 +1,11 @@
-// RUN: %clang_cc1 -triple i686-win32             -fsyntax-only -fms-extensions -verify -std=c++11 -Wunsupported-dll-base-class-template -DMS %s
-// RUN: %clang_cc1 -triple x86_64-win32           -fsyntax-only -fms-extensions -verify -std=c++1y -Wunsupported-dll-base-class-template -DMS %s
-// RUN: %clang_cc1 -triple i686-mingw32           -fsyntax-only -fms-extensions -verify -std=c++1y -Wunsupported-dll-base-class-template %s
-// RUN: %clang_cc1 -triple x86_64-mingw32         -fsyntax-only -fms-extensions -verify -std=c++11 -Wunsupported-dll-base-class-template %s
-// RUN: %clang_cc1 -triple i686-windows-itanium   -fsyntax-only -fms-extensions -verify -std=c++11 -Wunsupported-dll-base-class-template -DWI %s
-// RUN: %clang_cc1 -triple x86_64-windows-itanium -fsyntax-only -fms-extensions -verify -std=c++1y -Wunsupported-dll-base-class-template -DWI %s
-// RUN: %clang_cc1 -triple x86_64-scei-ps4        -fsyntax-only -fdeclspec      -verify -std=c++11 -Wunsupported-dll-base-class-template -DWI %s
-// RUN: %clang_cc1 -triple x86_64-sie-ps5         -fsyntax-only -fdeclspec      -verify -std=c++1y -Wunsupported-dll-base-class-template -DWI %s
+// RUN: %clang_cc1 -triple i686-win32             -fsyntax-only -fms-extensions -verify -std=c++11 -Wunsupported-dll-base-class-template -DMS  %s
+// RUN: %clang_cc1 -triple x86_64-win32           -fsyntax-only -fms-extensions -verify -std=c++1y -Wunsupported-dll-base-class-template -DMS  %s
+// RUN: %clang_cc1 -triple i686-mingw32           -fsyntax-only -fms-extensions -verify -std=c++1y -Wunsupported-dll-base-class-template -DGNU %s
+// RUN: %clang_cc1 -triple x86_64-mingw32         -fsyntax-only -fms-extensions -verify -std=c++11 -Wunsupported-dll-base-class-template -DGNU %s
+// RUN: %clang_cc1 -triple i686-windows-itanium   -fsyntax-only -fms-extensions -verify -std=c++11 -Wunsupported-dll-base-class-template -DWI  %s
+// RUN: %clang_cc1 -triple x86_64-windows-itanium -fsyntax-only -fms-extensions -verify -std=c++1y -Wunsupported-dll-base-class-template -DWI  %s
+// RUN: %clang_cc1 -triple x86_64-scei-ps4        -fsyntax-only -fdeclspec      -verify -std=c++11 -Wunsupported-dll-base-class-template -DWI  %s
+// RUN: %clang_cc1 -triple x86_64-sie-ps5         -fsyntax-only -fdeclspec      -verify -std=c++1y -Wunsupported-dll-base-class-template -DWI  %s
 
 // Helper structs to make templates more expressive.
 struct ImplicitInst_Exported {};
@@ -1087,6 +1087,13 @@ template<typename T> __declspec(dllexport) const  int  CTMR<T>::StaticConstField
 #endif
 template<typename T> __declspec(dllexport) constexpr int CTMR<T>::ConstexprField;
 
+// MSVC exports explicit specialization of exported class template member
+// function, and errors on such definitions. MinGW does not treat them as
+// dllexport.
+#if !defined(GNU)
+// expected-error@+2{{attribute 'dllexport' cannot be applied to a deleted function}}
+#endif
+template <> void ExportClassTmplMembers<int>::normalDecl() = delete;
 
 
 //===----------------------------------------------------------------------===//

diff --git a/clang/test/SemaCXX/dllimport.cpp b/clang/test/SemaCXX/dllimport.cpp
@@ -1138,6 +1138,9 @@ template<> __declspec(dllimport) const int MemVarTmpl::StaticVar<ExplicitSpec_De
 // Import individual members of a class template.
 template<typename T>
 struct ImportClassTmplMembers {
+#ifndef GNU
+// expected-note@+2{{attribute is here}}
+#endif
   __declspec(dllimport)                void normalDecl();
 #ifdef GNU
 // expected-note@+2{{previous attribute is here}}
@@ -1300,6 +1303,34 @@ template<typename T> __declspec(dllimport) const  int  CTMR<T>::StaticConstField
 template<typename T> __declspec(dllimport) constexpr int CTMR<T>::ConstexprField;
 
 
+// MSVC imports explicit specialization of imported class template member
+// function, and errors on such definitions. MinGW does not treat them as
+// dllimport.
+template <typename> struct ClassTmpl {
+#if !defined(GNU)
+// expected-note@+2{{attribute is here}}
+#endif
+  void __declspec(dllimport) importedNormal();
+#if !defined(GNU)
+// expected-note@+2{{attribute is here}}
+#endif
+  static void __declspec(dllimport) importedStatic();
+};
+#if !defined(GNU)
+// expected-error@+2{{cannot define non-inline dllimport template specialization}}
+#endif
+template<> void ClassTmpl<int>::importedNormal() {}
+#if !defined(GNU)
+// expected-error@+2{{cannot define non-inline dllimport template specialization}}
+#endif
+template<> void ClassTmpl<int>::importedStatic() {}
+
+#if !defined(GNU)
+// expected-error@+3{{cannot define non-inline dllimport template specialization}}
+// expected-error@+2{{attribute 'dllimport' cannot be applied to a deleted function}}
+#endif
+template <> void ImportClassTmplMembers<int>::normalDecl() = delete;
+
 
 //===----------------------------------------------------------------------===//
 // Class template member templates