Keep inherited dllimport/export attrs for explicit specialization of template class member functions

[hans]

Summary

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

[thakis]

lg

clang/lib/Sema/SemaDecl.cpp

7047

Here's hoping this doesn't break any actual code…

[hans]

This did turn up a problem.

With this patch Clang treats the following (based on https://source.chromium.org/chromium/chromium/src/+/main:media/formats/hls/source_string.h;l=125) as invalid, and so does MSVC:

template <typename> struct __declspec(dllexport) Foo {
    static Foo create();
};

// Don't allow creating Foo's of ints.
template <> Foo<int> Foo<int>::create() = delete; // MSVC: attempting to delete a __declspec(dllexport) function

The dllimport case errors similarly.

I'm tempted to make Clang keep allowing this. The intent of the code is perfectly clear -- the user is trying to delete the function, not export/import it -- and I can't think of any good way for the user to work around the error.

(Besides this issue, at least Chromium builds cleanly with this patch.)

[hans]

Looking at the Chromium code some more, the case of trying to delete a specialization of a dllexport class template member seems suspect.

Since the error also matches MSVC I think we should keep that behavior, but I'll add a release note about it.

clang/docs/ReleaseNotes.rst

 - Clang now disallows types whose sizes aren't a multiple of their alignments
   to be used as the element type of arrays.
 
   .. code-block:: c
 
   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|?
 ==============================
 Some of the major new features and improvements to Clang are listed
 here. Generic improvements to Clang as a whole or to its underlying
 infrastructure are described first, followed by language-specific
 sections with improvements to Clang's support for those languages.
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_attribute_invalid_on_definition : Warning<
   InGroup<IgnoredAttributes>;
 def err_attribute_dll_redeclaration : Error<
   "redeclaration of %q0 cannot add %q1 attribute">;
 def warn_attribute_dll_redeclaration : Warning<
   "redeclaration of %q0 should not add %q1 attribute">,
   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<
   "definition of dllimport data">;
 def err_attribute_dllimport_static_field_definition : Error<
   "definition of dllimport static field not allowed">;
 def warn_attribute_dllimport_static_field_definition : Warning<
   "definition of dllimport static field">,

clang/lib/Sema/SemaDecl.cpp

if (const auto *VD = dyn_cast<VarDecl>(NewDecl)) {
     IsQualifiedFriend = FD->getQualifier() &&
                         FD->getFriendObjectKind() == Decl::FOK_Declared;
   }
 
   if (OldImportAttr && !HasNewAttr &&
       (!IsInline || (IsMicrosoftABI && IsTemplate)) && !IsStaticDataMember &&
       !NewDecl->isLocalExternDecl() && !IsQualifiedFriend) {
     if (IsMicrosoftABI && IsDefinition) {
+      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()));
+        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)
           << NewDecl << OldImportAttr;
       S.Diag(OldDecl->getLocation(), diag::note_previous_declaration);
       S.Diag(OldImportAttr->getLocation(), diag::note_previous_attribute);
       OldDecl->dropAttr<DLLImportAttr>();
       NewDecl->dropAttr<DLLImportAttr>();

clang/lib/Sema/SemaTemplate.cpp

if (!Previous.isSingleResult()) {
     return;
   }
   // We unwrap canonical decl late to check for module visibility.
   Context.setPrimaryMergedDecl(NewDecl, OldConcept->getCanonicalDecl());
 }
 
 /// \brief Strips various properties off an implicit instantiation
 /// that has just been explicitly specialized.
-static void StripImplicitInstantiation(NamedDecl *D) {
+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);
 }
 
 /// Compute the diagnostic location for an explicit instantiation
 //  declaration or definition.
 static SourceLocation DiagLocForExplicitInstantiation(
case TSK_ExplicitSpecialization:
       // explicitly specialized or has merely been mentioned without any
       // instantiation.
       return false;
 
     case TSK_ImplicitInstantiation:
       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
       [[fallthrough]];
 
     case TSK_ExplicitInstantiationDeclaration:
     case TSK_ExplicitInstantiationDefinition:
       assert((PrevTSK == TSK_ImplicitInstantiation ||

clang/test/CodeGenCXX/dllexport-members.cpp

 // MSC-DAG: @"??$StaticVar@UExplicitInst_Exported@@@MemVarTmpl@@2HB" = weak_odr dso_local dllexport constant i32 1, comdat, align 4
 // GNU-DAG: @_ZN10MemVarTmpl9StaticVarI21ExplicitInst_ExportedEE        = weak_odr dso_local dllexport constant i32 1, comdat, align 4
 template __declspec(dllexport) const int MemVarTmpl::StaticVar<ExplicitInst_Exported>;
 
 // Export specialization of a non-exported member variable template.
 // 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() {}

clang/test/CodeGenCXX/dllimport-members.cpp

 // cannot be imported because the template must be defined which is illegal. The
 // in-class initializer does not count.
 
 // Import specialization of a non-imported member variable template.
 // MSC-DAG: @"??$StaticVar@UExplicitSpec_Imported@@@MemVarTmpl@@2HB" = external dllimport constant i32
 // 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);

clang/test/SemaCXX/dllexport.cpp

-// 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 {};
 struct ExplicitDecl_Exported {};
 struct ExplicitInst_Exported {};
 struct ExplicitSpec_Exported {};
 struct ExplicitSpec_Def_Exported {};
 struct ExplicitSpec_InlineDef_Exported {};

 template<typename T> __declspec(dllexport) const  int  CTMR<T>::StaticConstField = 1;  // expected-error{{redeclaration of 'CTMR::StaticConstField' cannot add 'dllexport' attribute}}
 #ifdef MS
 // expected-warning@+4{{attribute declaration must precede definition}}
 #else
 // expected-error@+2{{redeclaration of 'CTMR::ConstexprField' cannot add 'dllexport' attribute}}
 #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;
 
 
 //===----------------------------------------------------------------------===//
 // Class template member templates
 //===----------------------------------------------------------------------===//
 
 template<typename T>
 struct ExportClsTmplMemTmpl {

clang/test/SemaCXX/dllimport.cpp

 
 //===----------------------------------------------------------------------===//
 // Class template members
 //===----------------------------------------------------------------------===//
 
 // 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}}
 #endif
   __declspec(dllimport) void normalDef(); // expected-note{{previous declaration is here}}
   __declspec(dllimport)                void normalInlineDef();
   __declspec(dllimport) virtual        void virtualDecl();
 #ifdef GNU

 // expected-warning@+5{{definition of dllimport static field}}
 // expected-note@+4{{attribute is here}}
 #else
 // expected-warning@+2{{attribute declaration must precede definition}}
 #endif
 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
 //===----------------------------------------------------------------------===//
 
 template<typename T>
 struct ImportClsTmplMemTmpl {
   template<typename U> __declspec(dllimport)               void normalDecl();