hans 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: > > > 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. https://reviews.llvm.org/D51340 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits