[PATCH] D26657: [Sema] Respect DLL attributes more faithfully

[Hans Wennborg via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D26657#596897, @smeenai wrote:

> In https://reviews.llvm.org/D26657#596759, @hans wrote:
>
> > > On MSVC, if an implicit instantiation already exists and an explicit
> > >  instantiation definition with a DLL attribute is created, the DLL
> > >  attribute still takes effect. Make clang match this behavior.
> >
> > This is scary territory, and behaviour I think might be hard for us to match.
> >
> > What if we already codegenned the implicit instantiation?
>
>
> Hmm. Under what specific cases would that happen?


Actually maybe I was being overly cautius. I think https://reviews.llvm.org/rL225570 might just make this work.

> I can see odd behavior differences occurring for `dllimport`. For example, for the C++ source in https://reviews.llvm.org/P7936, if I compile with clang (even with this patch applied), the `t.f()` call in `g` gets assembled to a call to `?f@?$s at H@@QAEHXZ`. If I hoist the dllimport explicit instantiation definition above `g`, `t.f()` instead assembles to `__imp_?f@?$s at H@@QAEHXZ`.

Yeah, dllimport is interesting here. If you turn on `-O1`, it will call the `__imp` one. Fun times :-)

> With cl 19.00.24210, `t.f()` always assembles to `__imp_?f@?$s at H@@QAEHXZ`, regardless of the placement of the explicit instantiation definition.

Presumably they parse and analyze the whole translation unit before emitting any code, which makes changing things around later easier for them than for us.

> I'm having a harder time imagining things going awry for `dllexport`. I can limit this patch to only `dllexport`, if that's going to be safer.

Yeah, I think https://reviews.llvm.org/rL225570 makes this work actually.

> To give some context, libc++ uses extern templates extensively, and I ran into an issue with dllexport explicit locale template instantiations <https://reviews.llvm.org/diffusion/L/browse/libcxx/trunk/src/locale.cpp;287072$6034-6094?color=1>. Some of those instantiations are also implicitly instantiated via sizeof calls <https://reviews.llvm.org/diffusion/L/browse/libcxx/trunk/src/locale.cpp;287072$67,77,87?color=1>. Therefore, all instances which make is called on <https://reviews.llvm.org/diffusion/L/browse/libcxx/trunk/src/locale.cpp;287072$177-204?color=1> end up having their `dllexport` attribute ignored.
> 
> We could work around this in libc++ by hoisting the affected instantiations to near the top of the file, but it seemed preferable to make clang match MSVC's behavior instead, at least for `dllexport`. I don't have any particular interest in making `dllimport` semantics match MSVC for this specific case.

Dont' we need `dllimport` to work analogously though? When using libc++, don't these template members need to be dllimported? If not, why are they being exported in the first place?


https://reviews.llvm.org/D26657