[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added a comment.
Herald added a subscriber: nhaehnle.

Hans, I addressed all your comments.
How do you think about current implementation?



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+          TSK != TSK_ExplicitInstantiationDeclaration &&
+          TSK != TSK_ExplicitInstantiationDefinition) {
+        if (ClassExported) {
----------------
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > I still don't understand why we need these checks for template instantiations. Why does it matter whether the functions get inlined or not?
> > > When function of dllimported class is not inlined, such funtion needs to be dllexported from implementation library.
> > > 
> > > c.h
> > > ```
> > > template<typename T>
> > > class EXPORT C {
> > >  public:
> > >   void f() {}
> > > };
> > > ```
> > > 
> > > cuser.cc
> > > ```
> > > #include "c.h"
> > > 
> > > void cuser() {
> > >   C<int> c;
> > >   c.f();  // This function may not be inlined when EXPORT is __declspec(dllimport), so link may fail.
> > > }
> > > ```
> > > 
> > > When cuser.cc and c.h are built to different library, cuser.cc needs to be able to see dllexported C::f() when C::f() is not inlined.
> > > This is my understanding.
> > Your example doesn't use explicit instantiation definition or declaration, so doesn't apply here I think.
> > 
> > As long as the dllexport and dllimport attributes matches it's fine. It doesn't matter whether the function gets inlined or not, the only thing that matters is that if it's marked dllimport on the "consumer side", it must be dllexport when building the dll.
> Hmm, I changed the linkage for functions having DLLExport/ImportStaticLocal Attr.
> 
I changed linkage in ASTContext so that inline function is emitted when it is necessary when we use fno-dllexport-inlines.


https://reviews.llvm.org/D51340