Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL270748: clang-cl: Treat dllimport explicit template
instantiation definitions as… (authored by hans).
Changed prior to commit:
http://reviews.llvm.org/D20608?vs=58484=58488#toc
Repository:
rL LLVM
http://reviews.llvm.org/D20608
Files:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/CodeGenCXX/dllexport.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp
Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -7367,6 +7367,29 @@
}
}
+ // 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();
+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);
@@ -7420,6 +7443,12 @@
Specialization->setLocation(TemplateNameLoc);
PrevDecl = nullptr;
}
+
+if (PrevDecl_TSK == TSK_ExplicitInstantiationDeclaration &&
+DLLImportExplicitInstantiationDef) {
+ // The new specialization might add a dllimport attribute.
+ HasNoEffect = false;
+}
}
if (!Specialization) {
@@ -7497,11 +7526,11 @@
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);
Index: cfe/trunk/test/CodeGenCXX/dllimport.cpp
===
--- cfe/trunk/test/CodeGenCXX/dllimport.cpp
+++ cfe/trunk/test/CodeGenCXX/dllimport.cpp
@@ -656,7 +656,7 @@
namespace Vtordisp {
// Don't dllimport the vtordisp.
- // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@D@Vtordisp@@$4PPPM@A@AEXXZ"
+ // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@H@Vtordisp@@$4PPPM@A@AEXXZ"
class Base {
virtual void f() {}
@@ -667,7 +667,7 @@
C() {}
virtual void f() {}
};
- template class C;
+ USECLASS(C);
}
namespace ClassTemplateStaticDef {
@@ -698,26 +698,31 @@
template struct A { static NonPOD x; };
template NonPOD A::x;
template struct __declspec(dllimport) A;
- // MSC-DAG: @"\01?x@?$A@H@PR19933@@2UNonPOD@2@A" = available_externally dllimport global %"struct.PR19933::NonPOD" zeroinitializer
+ USEVARTYPE(NonPOD, A::x);
+ // MSC-DAG: @"\01?x@?$A@H@PR19933@@2UNonPOD@2@A" = external dllimport global %"struct.PR19933::NonPOD"
int f();
template struct B { static int x; };
template int B::x = f();
template struct __declspec(dllimport) B;
- // MSC-DAG: @"\01?x@?$B@H@PR19933@@2HA" = available_externally dllimport global i32 0
+ USEVAR(B::x);
+ // MSC-DAG: @"\01?x@?$B@H@PR19933@@2HA" = external dllimport global i32
constexpr int g() { return 42; }
template struct C { static int x; };
template int C::x = g();
template struct __declspec(dllimport) C;
- // MSC-DAG: @"\01?x@?$C@H@PR19933@@2HA" = available_externally dllimport global i32 42
+ USEVAR(C::x);
+ // MSC-DAG: @"\01?x@?$C@H@PR19933@@2HA" = external dllimport global i32
template struct D { static int x, y; };
template int D::x = I + 1;
template int D::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"
Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
hans updated this revision to Diff 58484.
hans added a comment.
Fix the "dllexport trumps" comment, and add a test.
http://reviews.llvm.org/D20608
Files:
lib/Sema/SemaTemplate.cpp
test/CodeGenCXX/dllexport.cpp
test/CodeGenCXX/dllimport.cpp
Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -656,7 +656,7 @@
namespace Vtordisp {
// Don't dllimport the vtordisp.
- // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@D@Vtordisp@@$4PPPM@A@AEXXZ"
+ // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@H@Vtordisp@@$4PPPM@A@AEXXZ"
class Base {
virtual void f() {}
@@ -667,7 +667,7 @@
C() {}
virtual void f() {}
};
- template class C;
+ USECLASS(C);
}
namespace ClassTemplateStaticDef {
@@ -698,26 +698,31 @@
template struct A { static NonPOD x; };
template NonPOD A::x;
template struct __declspec(dllimport) A;
- // MSC-DAG: @"\01?x@?$A@H@PR19933@@2UNonPOD@2@A" = available_externally dllimport global %"struct.PR19933::NonPOD" zeroinitializer
+ USEVARTYPE(NonPOD, A::x);
+ // MSC-DAG: @"\01?x@?$A@H@PR19933@@2UNonPOD@2@A" = external dllimport global %"struct.PR19933::NonPOD"
int f();
template struct B { static int x; };
template int B::x = f();
template struct __declspec(dllimport) B;
- // MSC-DAG: @"\01?x@?$B@H@PR19933@@2HA" = available_externally dllimport global i32 0
+ USEVAR(B::x);
+ // MSC-DAG: @"\01?x@?$B@H@PR19933@@2HA" = external dllimport global i32
constexpr int g() { return 42; }
template struct C { static int x; };
template int C::x = g();
template struct __declspec(dllimport) C;
- // MSC-DAG: @"\01?x@?$C@H@PR19933@@2HA" = available_externally dllimport global i32 42
+ USEVAR(C::x);
+ // MSC-DAG: @"\01?x@?$C@H@PR19933@@2HA" = external dllimport global i32
template struct D { static int x, y; };
template int D::x = I + 1;
template int D::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 {
@@ -805,6 +810,36 @@
USEMEMFUNC(PR23770BaseTemplate, f);
// M32-DAG: declare dllimport x86_thiscallcc void @"\01?f@?$PR23770BaseTemplate@H@@QAEXXZ"
+namespace PR27810 {
+ template
+ struct basic_ostream {
+struct sentry {
+ sentry() { }
+ void foo() { }
+};
+ };
+ template class __declspec(dllimport) basic_ostream;
+ // 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::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 struct codecvt {
+virtual ~codecvt() { }
+ };
+ template class __declspec(dllimport) codecvt;
+
+ // 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
//===--===//
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -844,6 +844,11 @@
// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01??_FB@pr26490@@QAEXXZ"
}
+// dllexport trumps dllexport on an explicit instantiation.
+template struct ExplicitInstantiationTwoAttributes { void f() {} };
+template struct __declspec(dllexport) __declspec(dllimport) ExplicitInstantiationTwoAttributes;
+// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?f@?$ExplicitInstantiationTwoAttributes@H@@QAEXXZ"
+
//===--===//
// Classes with template base classes
@@ -958,14 +963,6 @@
// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?func@?$ExplicitInstantiationDeclTemplateBase@H@@QAEXXZ"
// G32-DAG: define weak_odr x86_thiscallcc void @_ZN37ExplicitInstantiationDeclTemplateBaseIiE4funcEv
-template struct ExplicitInstantiationDeclTemplateBase2 { void func() {} };
-extern template struct
Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
hans added inline comments.
Comment at: lib/Sema/SemaTemplate.cpp:7382
@@ +7381,3 @@
+ if (A->getKind() == AttributeList::AT_DLLExport) {
+// dllexport trumps dllexport here.
+DLLImport = false;
thakis wrote:
> trumps _dllimport_ here
>
> (test?)
Done. Adding a test to dllexport.cpp.
Comment at: lib/Sema/SemaTemplate.cpp:7451
@@ -7423,1 +7450,3 @@
+ HasNoEffect = false;
+}
}
thakis wrote:
> does moving this here change behavior? if so, maybe add a test for that too.
Nope, this doesn't change anything.
http://reviews.llvm.org/D20608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
thakis accepted this revision.
This revision is now accepted and ready to land.
Comment at: lib/Sema/SemaTemplate.cpp:7382
@@ +7381,3 @@
+ if (A->getKind() == AttributeList::AT_DLLExport) {
+// dllexport trumps dllexport here.
+DLLImport = false;
trumps _dllimport_ here
(test?)
Comment at: lib/Sema/SemaTemplate.cpp:7451
@@ -7423,1 +7450,3 @@
+ HasNoEffect = false;
+}
}
does moving this here change behavior? if so, maybe add a test for that too.
http://reviews.llvm.org/D20608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
hans added a comment.
Thanks for the review!
In http://reviews.llvm.org/D20608#439480, @thakis wrote:
> Do we have test coverage for `template class __declspec(dllexport)
> codecvt;` somewhere already?
Yes, that's covered by tests in CodeGenCXX/dllexport.cpp
Comment at: lib/Sema/SemaTemplate.cpp:7382
@@ +7381,3 @@
+ if (A->getKind() == AttributeList::AT_DLLExport)
+DLLImport = false;
+}
thakis wrote:
> If there are multiple dllexports and dllimports on an explicit instantiation,
> cl.exe just silently picks the last one?
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 struct S { void f() {} };
template struct __declspec(dllimport) __declspec(dllexport) S;
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::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::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.
Comment at: lib/Sema/SemaTemplate.cpp:7467
@@ +7466,3 @@
+// The new specialization might add a dllimport attribute.
+HasNoEffect = false;
+ }
thakis wrote:
> 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)
I wanted to do this after PrevDecl_TSK has been defined, but we can move this a
little earlier. I'll do that.
http://reviews.llvm.org/D20608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
hans updated this revision to Diff 58461.
hans added a comment.
tweaks
http://reviews.llvm.org/D20608
Files:
lib/Sema/SemaTemplate.cpp
test/CodeGenCXX/dllexport.cpp
test/CodeGenCXX/dllimport.cpp
Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -656,7 +656,7 @@
namespace Vtordisp {
// Don't dllimport the vtordisp.
- // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@D@Vtordisp@@$4PPPM@A@AEXXZ"
+ // MO1-DAG: define linkonce_odr x86_thiscallcc void @"\01?f@?$C@H@Vtordisp@@$4PPPM@A@AEXXZ"
class Base {
virtual void f() {}
@@ -667,7 +667,7 @@
C() {}
virtual void f() {}
};
- template class C;
+ USECLASS(C);
}
namespace ClassTemplateStaticDef {
@@ -698,26 +698,31 @@
template struct A { static NonPOD x; };
template NonPOD A::x;
template struct __declspec(dllimport) A;
- // MSC-DAG: @"\01?x@?$A@H@PR19933@@2UNonPOD@2@A" = available_externally dllimport global %"struct.PR19933::NonPOD" zeroinitializer
+ USEVARTYPE(NonPOD, A::x);
+ // MSC-DAG: @"\01?x@?$A@H@PR19933@@2UNonPOD@2@A" = external dllimport global %"struct.PR19933::NonPOD"
int f();
template struct B { static int x; };
template int B::x = f();
template struct __declspec(dllimport) B;
- // MSC-DAG: @"\01?x@?$B@H@PR19933@@2HA" = available_externally dllimport global i32 0
+ USEVAR(B::x);
+ // MSC-DAG: @"\01?x@?$B@H@PR19933@@2HA" = external dllimport global i32
constexpr int g() { return 42; }
template struct C { static int x; };
template int C::x = g();
template struct __declspec(dllimport) C;
- // MSC-DAG: @"\01?x@?$C@H@PR19933@@2HA" = available_externally dllimport global i32 42
+ USEVAR(C::x);
+ // MSC-DAG: @"\01?x@?$C@H@PR19933@@2HA" = external dllimport global i32
template struct D { static int x, y; };
template int D::x = I + 1;
template int D::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 {
@@ -805,6 +810,36 @@
USEMEMFUNC(PR23770BaseTemplate, f);
// M32-DAG: declare dllimport x86_thiscallcc void @"\01?f@?$PR23770BaseTemplate@H@@QAEXXZ"
+namespace PR27810 {
+ template
+ struct basic_ostream {
+struct sentry {
+ sentry() { }
+ void foo() { }
+};
+ };
+ template class __declspec(dllimport) basic_ostream;
+ // 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::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 struct codecvt {
+virtual ~codecvt() { }
+ };
+ template class __declspec(dllimport) codecvt;
+
+ // 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
//===--===//
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -958,14 +958,6 @@
// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?func@?$ExplicitInstantiationDeclTemplateBase@H@@QAEXXZ"
// G32-DAG: define weak_odr x86_thiscallcc void @_ZN37ExplicitInstantiationDeclTemplateBaseIiE4funcEv
-template struct ExplicitInstantiationDeclTemplateBase2 { void func() {} };
-extern template struct ExplicitInstantiationDeclTemplateBase2;
-struct __declspec(dllexport) DerivedFromExplicitInstantiationDeclTemplateBase2 : public ExplicitInstantiationDeclTemplateBase2 {};
-template struct __declspec(dllimport) ExplicitInstantiationDeclTemplateBase2;
-USEMEMFUNC(ExplicitInstantiationDeclTemplateBase2, 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 struct Selection {};
Index:
Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
thakis added a comment. 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;` somewhere already? Comment at: lib/Sema/SemaTemplate.cpp:7382 @@ +7381,3 @@ + if (A->getKind() == AttributeList::AT_DLLExport) +DLLImport = false; +} If there are multiple dllexports and dllimports on an explicit instantiation, cl.exe just silently picks the last one? Comment at: lib/Sema/SemaTemplate.cpp:7467 @@ +7466,3 @@ +// The new specialization might add a dllimport attribute. +HasNoEffect = false; + } 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) http://reviews.llvm.org/D20608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
hans added a comment.
In http://reviews.llvm.org/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::Inner::f:
template struct S {
struct Inner {
void f() {}
};
};
template struct __declspec(dllimport) S;
http://reviews.llvm.org/D20608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20608: clang-cl: Treat dllimport explicit template instantiation definitions as declarations (PR27810, PR27811)
majnemer added a subscriber: majnemer. majnemer added a comment. Does this change our behavior for mingw? http://reviews.llvm.org/D20608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
