takuto.ikuta added inline comments.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+ while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr<DLLExportStaticLocalAttr>() &&
----------------
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > hans wrote:
> > > > > Why does this need to be a loop? I don't think FunctionDecl's can be
> > > > > nested?
> > > > This is for static local var in lambda function.
> > > > static_x's ParentFunction does not become f().
> > > > ```
> > > > class __declspec(dllexport) C {
> > > > int f() {
> > > > return ([]() { static int static_x; return ++static_x; })();
> > > > }
> > > > };
> > > > ```
> > > I don't think the lambda (or its static local) should be exported in this
> > > case.
> > If we don't export static_x, dllimport library cannot find static_x when
> > linking?
> >
> >
> I believe even if C is marked dllimport, the lambda will not be dllimport.
> The inline definition will be used.
Do you say importing/implementation library should have different instance of
static_x here?
Current clang does not generate such obj.
I think static_x should be dllimported. But without this loop, static_x cannot
find FD of C::f() having DLLImport/ExportStaticLocalAttr and static_x won't be
treated as imported/exported variable.
And if static_x is not exported, importing library and implementation library
will not have the same instance of static_x when C::f() is inlined.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+ // Function having static local variables should be exported.
+ auto *ExportAttr =
cast<InheritableAttr>(NewAttr->clone(getASTContext()));
----------------
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > Isn't it enough that the static local is exported, does the function
> > > > itself need to be exported too?
> > > Adding dllexport only to variable does not export variable when the
> > > function is not used.
> > > But dllimport is not necessary for function, removed.
> > Hmm, I wonder if we could fix that instead? That is, export the variable in
> > that case even if the function is not used?
> I see. I'll investigate which code path emits static variables
static local variable seems to be exported in
https://github.com/llvm-project/llvm-project-20170507/blob/f54514c50350743d3d1a3c5aa80cbe4bb449eb71/clang/lib/CodeGen/CGDecl.cpp#L378
But emitting static local var only by bypassing function emission seems
difficult.
Let me leave as-is here.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+ if (ClassExported) {
----------------
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.
https://reviews.llvm.org/D51340
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
