clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)

[hans]

Summary

This matches what MSVC does, and should make compiles faster by avoiding to unnecessarily emit a lot of code.

[majnemer]

Does this change our behavior for mingw?

[hans]

In D20608#438760, @majnemer wrote:

Does this change our behavior for mingw?

No, this only affects the MS ABI.

MinGW doesn't seem to do this trick. For example, in this code, they will emit a definition for S<int>::Inner::f:

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

[thakis]

Looks great, thanks! A few minor questions below.

I verified that this has the same effect as my brute-force patch I tried locally.

Do we have test coverage for template class __declspec(dllexport) codecvt<char>; somewhere already?

lib/Sema/SemaTemplate.cpp

7382

If there are multiple dllexports and dllimports on an explicit instantiation, cl.exe just silently picks the last one?

7476

HasNoEffect is read two times before it's updated here. Is it intentional that you only change it after it's been read twice? If so, maybe add a comment why, since it looks not intentional else. (And make sure there's test coverage for setting it at the right time)

[hans]

Thanks for the review!

In D20608#439480, @thakis wrote:

Do we have test coverage for template class __declspec(dllexport) codecvt<char>; somewhere already?

Yes, that's covered by tests in CodeGenCXX/dllexport.cpp

lib/Sema/SemaTemplate.cpp

7382

The intention here was really just to reset the DLLImport value if it were set on line 7376, i.e. the attribute on the instantiation should override any attribute on the template.

I didn't even consider putting both dllimport and dllexport on the specialization. What does MSVC do? Well..

template <typename T> struct S { void f() {} };
template struct __declspec(dllimport) __declspec(dllexport) S<int>;
d:\src\tmp\a.cc(2) : warning C4141: 'dllexport' : used more than once

That diagnostic seems a little bit confused, but the effect on codegen is even more so. S<int>::f() does not get defined here, which would suggest the instantiation def is treated as an instantiation decl on account of the dllimport. On the other hand, if I reference S<int>::f, it gets defined and *exported*. We probably don't want to reproduce this.

Clang will generally let dllexport take precedence over dllimport when they're on the same declaration, and has a nice warning for it, so let's do that here too.

7476

I wanted to do this after PrevDecl_TSK has been defined, but we can move this a little earlier. I'll do that.

[hans]

tweaks

[thakis]

lib/Sema/SemaTemplate.cpp

7382

trumps _dllimport_ here

(test?)

7451

does moving this here change behavior? if so, maybe add a test for that too.

[hans]

lib/Sema/SemaTemplate.cpp

7382

Done. Adding a test to dllexport.cpp.

7451

Nope, this doesn't change anything.

[hans]

Fix the "dllexport trumps" comment, and add a test.

lib/Sema/SemaTemplate.cpp

if (TSK == TSK_ExplicitInstantiationDeclaration) {
 
     if (auto *A = ClassTemplate->getTemplatedDecl()->getAttr<DLLExportAttr>()) {
       Diag(ExternLoc,
            diag::warn_attribute_dllexport_explicit_instantiation_decl);
       Diag(A->getLocation(), diag::note_attribute);
     }
   }
 
+  // In MSVC mode, dllimported explicit instantiation definitions are treated as
+  // instantiation declarations for most purposes.
+  bool DLLImportExplicitInstantiationDef = false;
+  if (TSK == TSK_ExplicitInstantiationDefinition &&
+      Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+    // Check for dllimport class template instantiation definitions.
+    bool DLLImport =
+        ClassTemplate->getTemplatedDecl()->getAttr<DLLImportAttr>();
+    for (AttributeList *A = Attr; A; A = A->getNext()) {
+      if (A->getKind() == AttributeList::AT_DLLImport)
+        DLLImport = true;
+      if (A->getKind() == AttributeList::AT_DLLExport) {
+        // dllexport trumps dllimport here.
+        DLLImport = false;
+        break;
+      }
+    }
+    if (DLLImport) {
+      TSK = TSK_ExplicitInstantiationDeclaration;
+      DLLImportExplicitInstantiationDef = true;
+    }
+  }
+
   // Translate the parser's template argument list in our AST format.
   TemplateArgumentListInfo TemplateArgs(LAngleLoc, RAngleLoc);
   translateTemplateArguments(TemplateArgsIn, TemplateArgs);
 
   // Check that the template argument list is well-formed for this
   // template.
   SmallVector<TemplateArgument, 4> Converted;
   if (CheckTemplateArgumentList(ClassTemplate, TemplateNameLoc,
if (PrevDecl_TSK == TSK_ImplicitInstantiation ||
       // arguments was referenced but not declared, reuse that
       // declaration node as our own, updating the source location
       // for the template name to reflect our new declaration.
       // (Other source locations will be updated later.)
       Specialization = PrevDecl;
       Specialization->setLocation(TemplateNameLoc);
       PrevDecl = nullptr;
     }
+
+    if (PrevDecl_TSK == TSK_ExplicitInstantiationDeclaration &&
+        DLLImportExplicitInstantiationDef) {
+      // The new specialization might add a dllimport attribute.
+      HasNoEffect = false;
+    }
   }
 
   if (!Specialization) {
     // Create a new class template specialization declaration node for
     // this explicit specialization.
     Specialization
       = ClassTemplateSpecializationDecl::Create(Context, Kind,
                                              ClassTemplate->getDeclContext(),
                                                 KWLoc, TemplateNameLoc,
                                                 ClassTemplate,
                                                 Converted.data(),
                                                 Converted.size(),
                                                 PrevDecl);
     SetNestedNameSpecifier(Specialization, SS);
 
     if (!HasNoEffect && !PrevDecl) {
       // Insert the new specialization.
       ClassTemplate->AddSpecialization(Specialization, InsertPos);
     }
   }
 
   // Build the fully-sugared type for this explicit instantiation as
   // the user wrote in the explicit instantiation itself. This means
   // that we'll pretty-print the type retrieved from the
   // specialization's declaration the way that the user actually wrote
   // the explicit instantiation, rather than formatting the name based
   // on the "canonical" representation used to store the template
   // arguments in the specialization.
   TypeSourceInfo *WrittenTy
     = Context.getTemplateSpecializationTypeInfo(Name, TemplateNameLoc,
                                                 TemplateArgs,
                                   Context.getTypeDeclType(Specialization));
   Specialization->setTypeAsWritten(WrittenTy);
else if (TSK == TSK_ExplicitInstantiationDefinition) {
     Specialization->setPointOfInstantiation(Def->getPointOfInstantiation());
   }
 
   // Instantiate the members of this class template specialization.
   Def = cast_or_null<ClassTemplateSpecializationDecl>(
                                        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) {
+        (TSK == TSK_ExplicitInstantiationDefinition ||
+         DLLImportExplicitInstantiationDef)) {
       // FIXME: Need to notify the ASTMutationListener that we did this.
       Def->setTemplateSpecializationKind(TSK);
 
       if (!getDLLAttr(Def) && getDLLAttr(Specialization) &&
           Context.getTargetInfo().getCXXABI().isMicrosoft()) {
         // 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.

test/CodeGenCXX/dllexport.cpp

 template <typename T> struct A { };
 struct __declspec(dllexport) B {
   B(int = 0) {}
   A<int> m_fn1() {}
 };
 // M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01??_FB@pr26490@@QAEXXZ"
 }
 
+// dllexport trumps dllexport on an explicit instantiation.
+template <typename T> struct ExplicitInstantiationTwoAttributes { void f() {} };
+template struct __declspec(dllexport) __declspec(dllimport) ExplicitInstantiationTwoAttributes<int>;
+// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?f@?$ExplicitInstantiationTwoAttributes@H@@QAEXXZ"
+
 
 //===----------------------------------------------------------------------===//
 // Classes with template base classes
 //===----------------------------------------------------------------------===//
 
 template <typename T> struct ClassTemplate { void func(); };
 template <typename T> void ClassTemplate<T>::func() {}
 template <typename T> struct __declspec(dllexport) ExportedClassTemplate { void func(); };

 
 template <typename T> struct ExplicitInstantiationDeclTemplateBase { void func() {} };
 extern template struct ExplicitInstantiationDeclTemplateBase<int>;
 struct __declspec(dllexport) DerivedFromExplicitInstantiationDeclTemplateBase : public ExplicitInstantiationDeclTemplateBase<int> {};
 template struct ExplicitInstantiationDeclTemplateBase<int>;
 // M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?func@?$ExplicitInstantiationDeclTemplateBase@H@@QAEXXZ"
 // G32-DAG: define weak_odr x86_thiscallcc void @_ZN37ExplicitInstantiationDeclTemplateBaseIiE4funcEv
 
-template <typename T> struct ExplicitInstantiationDeclTemplateBase2 { void func() {} };
-extern template struct ExplicitInstantiationDeclTemplateBase2<int>;
-struct __declspec(dllexport) DerivedFromExplicitInstantiationDeclTemplateBase2 : public ExplicitInstantiationDeclTemplateBase2<int> {};
-template struct __declspec(dllimport) ExplicitInstantiationDeclTemplateBase2<int>;
-USEMEMFUNC(ExplicitInstantiationDeclTemplateBase2<int>, func)
-// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?func@?$ExplicitInstantiationDeclTemplateBase2@H@@QAEXXZ"
-// G32-DAG: define weak_odr x86_thiscallcc void @_ZN38ExplicitInstantiationDeclTemplateBase2IiE4funcEv
-
 // PR26076
 struct LayerSelectionBound;
 template <typename> struct Selection {};
 typedef Selection<LayerSelectionBound> LayerSelection;
 struct LayerImpl;
 struct __declspec(dllexport) LayerTreeImpl {
   struct __declspec(dllexport) ElementLayers {
     LayerImpl *main = nullptr;

test/CodeGenCXX/dllimport.cpp

namespace DontUseDtorAlias {
   inline B::~B() { }
   // Emit a real definition of B's constructor; don't alias it to A's.
   // MO1-DAG: available_externally dllimport x86_thiscallcc void @"\01??1B@DontUseDtorAlias@@QAE@XZ"
   USECLASS(B)
 }
 
 namespace Vtordisp {
   // Don't dllimport the vtordisp.
-  // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@D@Vtordisp@@$4PPPPPPPM@A@AEXXZ"
+  // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@H@Vtordisp@@$4PPPPPPPM@A@AEXXZ"
 
   class Base {
     virtual void f() {}
   };
   template <typename T>
   class __declspec(dllimport) C : virtual public Base {
   public:
     C() {}
     virtual void f() {}
   };
-  template class C<char>;
+  USECLASS(C<int>);
 }
 
 namespace ClassTemplateStaticDef {
   // Regular template static field:
   template <typename T> struct __declspec(dllimport) S {
     static int x;
   };
   template <typename T> int S<T>::x;

 // Don't dynamically initialize dllimport vars.
 // MSC2-NOT: @llvm.global_ctors
 // GNU2-NOT: @llvm.global_ctors
 
   struct NonPOD { NonPOD(); };
   template <typename T> struct A { static NonPOD x; };
   template <typename T> NonPOD A<T>::x;
   template struct __declspec(dllimport) A<int>;
-  // MSC-DAG: @"\01?x@?$A@H@PR19933@@2UNonPOD@2@A" = available_externally dllimport global %"struct.PR19933::NonPOD" zeroinitializer
+  USEVARTYPE(NonPOD, A<int>::x);
+  // MSC-DAG: @"\01?x@?$A@H@PR19933@@2UNonPOD@2@A" = external dllimport global %"struct.PR19933::NonPOD"
 
   int f();
   template <typename T> struct B { static int x; };
   template <typename T> int B<T>::x = f();
   template struct __declspec(dllimport) B<int>;
-  // MSC-DAG: @"\01?x@?$B@H@PR19933@@2HA" = available_externally dllimport global i32 0
+  USEVAR(B<int>::x);
+  // MSC-DAG: @"\01?x@?$B@H@PR19933@@2HA" = external dllimport global i32
 
   constexpr int g() { return 42; }
   template <typename T> struct C { static int x; };
   template <typename T> int C<T>::x = g();
   template struct __declspec(dllimport) C<int>;
-  // MSC-DAG: @"\01?x@?$C@H@PR19933@@2HA" = available_externally dllimport global i32 42
+  USEVAR(C<int>::x);
+  // MSC-DAG: @"\01?x@?$C@H@PR19933@@2HA" = external dllimport global i32
 
   template <int I> struct D { static int x, y; };
   template <int I> int D<I>::x = I + 1;
   template <int I> int D<I>::y = I + f();
   template struct __declspec(dllimport) D<42>;
-  // MSC-DAG: @"\01?x@?$D@$0CK@@PR19933@@2HA" = available_externally dllimport global i32 43
-  // MSC-DAG: @"\01?y@?$D@$0CK@@PR19933@@2HA" = available_externally dllimport global i32 0
+  USEVAR(D<42>::x);
+  USEVAR(D<42>::y);
+  // MSC-DAG: @"\01?x@?$D@$0CK@@PR19933@@2HA" = external dllimport global i32
+  // MSC-DAG: @"\01?y@?$D@$0CK@@PR19933@@2HA" = external dllimport global i32
 }
 
 namespace PR21355 {
   struct __declspec(dllimport) S {
     virtual ~S();
   };
   S::~S() {}
 

 
 template <typename T> struct PR23770BaseTemplate { void f() {} };
 template <typename T> struct PR23770DerivedTemplate : PR23770BaseTemplate<int> {};
 extern template struct PR23770DerivedTemplate<int>;
 template struct __declspec(dllimport) PR23770DerivedTemplate<int>;
 USEMEMFUNC(PR23770BaseTemplate<int>, f);
 // M32-DAG: declare dllimport x86_thiscallcc void @"\01?f@?$PR23770BaseTemplate@H@@QAEXXZ"
 
+namespace PR27810 {
+  template <class T>
+  struct basic_ostream {
+    struct sentry {
+      sentry() { }
+      void foo() { }
+    };
+  };
+  template class __declspec(dllimport) basic_ostream<char>;
+  // The explicit instantiation definition acts as an explicit instantiation
+  // *declaration*, dllimport is not inherited by the inner class, and no
+  // functions are emitted unless they are used.
+
+  USEMEMFUNC(basic_ostream<char>::sentry, foo);
+  // M32-DAG: {{declare|define available_externally}} x86_thiscallcc void @"\01?foo@sentry@?$basic_ostream@D@PR27810@@QAEXXZ"
+  // M32-NOT: ??0sentry@?$basic_ostream@D@PR27810@@QAE@XZ
+}
+
+namespace PR27811 {
+  template <class T> struct codecvt {
+    virtual ~codecvt() { }
+  };
+  template class __declspec(dllimport) codecvt<char>;
+
+  // dllimport means this explicit instantiation definition gets treated as a
+  // declaration. Thus, the vtable should not be marked used, and in fact
+  // nothing for this class should be emitted at all since it's not used.
+  // M32-NOT: codecvt
+}
+
 //===----------------------------------------------------------------------===//
 // Classes with template base classes
 //===----------------------------------------------------------------------===//
 
 template <typename T> struct ClassTemplate { void func() {} };
 template <typename T> struct __declspec(dllexport) ExportedClassTemplate { void func(); };
 template <typename T> void ExportedClassTemplate<T>::func() {}
 template <typename T> struct __declspec(dllimport) ImportedClassTemplate { void func(); };