hans added a comment.
In https://reviews.llvm.org/D51340#1266312, @takuto.ikuta wrote:
> Hans, I addressed all your comments.
> How do you think about current implementation?
Just a few questions, but I think it's pretty good.
================
Comment at: clang/include/clang/Basic/Attr.td:2688
+ // This attribute is used internally only when -fno-dllexport-inlines is
+ // passed.
+ let Spellings = [];
----------------
The comment should explain what the attribute does, also the dllimport one
below.
================
Comment at: clang/lib/AST/ASTContext.cpp:9552
+ // overwrite linkage of explicit template instantiation
+ // definition/declaration.
+ return GVA_DiscardableODR;
----------------
Can you give an example for why this is needed?
================
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+ while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr<DLLExportStaticLocalAttr>() &&
----------------
takuto.ikuta wrote:
> 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.
Look at what MSVC does:
```
>type a.cc
struct __declspec(dllexport) S {
int f() {
static int y;
return ([]() { static int static_x; return ++static_x; })() + y;
}
};
void use(S *s) { s->f(); }
>cl -c a.cc && dumpbin /directives a.obj
Microsoft (R) C/C++ Optimizing Compiler Version 19.12.25834 for x86
Copyright (C) Microsoft Corporation. All rights reserved.
a.cc
Microsoft (R) COFF/PE Dumper Version 14.12.25834.0
Copyright (C) Microsoft Corporation. All rights reserved.
Dump of file a.obj
File Type: COFF OBJECT
Linker Directives
-----------------
/DEFAULTLIB:LIBCMT
/DEFAULTLIB:OLDNAMES
/EXPORT:?f@S@@QAEHXZ
/EXPORT:??4S@@QAEAAU0@ABU0@@Z
/EXPORT:??4S@@QAEAAU0@$$QAU0@@Z
/EXPORT:?y@?1??f@S@@QAEHXZ@4HA,DATA
Summary
8 .bss
50 .chks64
64 .debug$S
A6 .drectve
6A .text$mn
```
As you can see S::f and S::f::y is exported, but the lambda and its static
local are not.
Oh, but I guess you're asking what if we don't dllexport S::f anymore because
we're not dllexporting inline methods anymore... hmm, yes, then I do suppose it
needs to be exported so maybe this is right after all.
https://reviews.llvm.org/D51340
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
