smeenai added a comment.

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?

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@H@@QAEHXZ`. If I hoist the dllimport explicit instantiation definition 
above `g`, `t.f()` instead assembles to `__imp_?f@?$s@H@@QAEHXZ`. With cl 
19.00.24210, `t.f()` always assembles to `__imp_?f@?$s@H@@QAEHXZ`, regardless 
of the placement of the explicit instantiation definition.

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.

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.


https://reviews.llvm.org/D26657



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to