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

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+    while (FD && !getDLLAttr(FD) &&
+           !FD->hasAttr<DLLExportStaticLocalAttr>() &&
----------------
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.


================
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:
> > 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?


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+          TSK != TSK_ExplicitInstantiationDeclaration &&
+          TSK != TSK_ExplicitInstantiationDefinition) {
+        if (ClassExported) {
----------------
I still don't understand why we need these checks for template instantiations. Why does it matter whether the functions get inlined or not?


https://reviews.llvm.org/D51340