rnk added inline comments.
================
Comment at: clang/lib/AST/ASTContext.cpp:9552
+ // overwrite linkage of explicit template instantiation
+ // definition/declaration.
+ return GVA_DiscardableODR;
----------------
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > takuto.ikuta wrote:
> > > > > > hans wrote:
> > > > > > > Can you give an example for why this is needed?
> > > > > > Sorry, this change does not need. Removed.
> > > > > Sorry, this change is necessary still.
> > > > >
> > > > > Without this, definition of inline function in explicit template
> > > > > instantiation declaration is not be emitted, due to
> > > > > GVA_AvailableExternally linkage.
> > > > > But we stop exporting definition of inline function in explicit
> > > > > template instantiation definition too.
> > > > >
> > > > > So without this, definition of dllimported inline function of
> > > > > explicit template instantiation declaration won't be available.
> > > > >
> > > > Can you provide a code example of why this is needed?
> > > If we don't change function linkage, following code will be compiled like
> > > below with -fno-dllexport-inlines.
> > >
> > > ```
> > > template<typename>
> > > class M{
> > > public:
> > > void foo() {}
> > > };
> > >
> > > template class __declspec(dllexport) M<int>;
> > >
> > > extern template class __declspec(dllimport) M<short>;
> > >
> > > void f (){
> > > M<int> mi;
> > > mi.foo();
> > >
> > > M<short> ms;
> > > ms.foo();
> > > }
> > > ```
> > >
> > > ```
> > > $"?foo@?$M@H@@QEAAXXZ" = comdat any
> > >
> > > ; Function Attrs: noinline nounwind optnone
> > > define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %this) #0
> > > comdat align 2 {
> > > entry:
> > > %this.addr = alloca %class.M*, align 8
> > > store %class.M* %this, %class.M** %this.addr, align 8
> > > %this1 = load %class.M*, %class.M** %this.addr, align 8
> > > ret void
> > > }
> > >
> > > ; Function Attrs: noinline nounwind optnone
> > > define dso_local void @"?f@@YAXXZ"() #0 {
> > > entry:
> > > %mi = alloca %class.M, align 1
> > > %ms = alloca %class.M.0, align 1
> > > call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
> > > call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
> > > ret void
> > > }
> > >
> > > declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
> > > ```
> > >
> > > M<short>::foo() is declared, but inline function is not dllexported (See
> > > M<int>::foo() is not dllexported). So this code cannot be linked because
> > > definition of M<short>::foo does not exist. If the function is properly
> > > inlined, we don't need to have this code. But I'm not sure why the
> > > function is not inlined.
> > Interesting. I wonder how -fvisibility-inlines-hidden handles this...
> >
> >
> > ```
> > template <typename> struct S {
> > void foo() {}
> > };
> >
> > template struct S<int>;
> >
> > void use() {
> > S<int> s;
> > s.foo();
> > }
> >
> > $ g++ -fvisibility-inlines-hidden -c a.cc && objdump -t a.o | grep
> > _ZN1SIiE3fooEv
> > 0000000000000000 l d .text._ZN1SIiE3fooEv 0000000000000000
> > .text._ZN1SIiE3fooEv
> > 0000000000000000 w F .text._ZN1SIiE3fooEv 000000000000000b
> > _ZN1SIiE3fooEv <--- Not hidden.
> > ```
> >
> > (If I comment out the explicit instantiation definition above, foo() is
> > hidden as expected.)
> >
> > Okay, it seems that for explicit instantiation definitions,
> > -fvisibility-inlines-hidden does not apply.
> >
> > And thinking more about it, that makes sense.
> >
> > I don't think we should change the linkage like this though, I think we
> > should just not apply the /Zc:dllexportInlines- for explicit instantiation
> > decls and definitions in checkClassLevelDLLAttribute(). I realize you had
> > code to check for this before, but now we have a good and well understood
> > reason.
> Thank you for confirmation. I revived the check.
I agree, this option should not affect explicit instantiations. Inline
functions from explicit instantiations should still be exported+weak_odr, and
imported+available_externally for extern declarations.
https://reviews.llvm.org/D51340
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
