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

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added a comment.

Thank you for review!



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5599
 
+bool Sema::isInlineFunctionDLLExportable(const CXXMethodDecl *MD) {
+  assert(MD->isInlined());
----------------
hans wrote:
> Okay, breaking out this logic is a little better, but I still don't like that we now have split the "inherit dllexport attribute" in two places: one for non-inline functions and one for inline (even if they both call this function). It feels like it will be hard to maintain.
> 
> Here is another idea:
> 
> When we inherit the dllexport attribute to class members, if getLangOpts().DllExportInlines is false, we don't put dllexport on inline functions, but instead we put a new attribute "dllexportstaticlocals".
> 
> That attribute only has the effect that it makes static locals exported. We would check for it when computing the linkage of the static local, similar to how it works in hidden functions.
> 
> This has two benefits: it doesn't complicate the "inherit dllexport" code very much, and it avoids the need for a separate AST walk of the function.
> 
> What do you think?
I implemented your idea the way I understood.
Use new attribute to check static local var later.

Due to difference of treating between linkage and dll attribute, I inherit dll attribute in Sema/SemaDecl.cpp instead of  AST/Decl.cpp


https://reviews.llvm.org/D51340