Re: [PATCH] D24877: [libc++] Default to DLL semantics on Windows

[Shoaib Meenai via cfe-commits]

smeenai abandoned this revision.
smeenai added a comment.

Your patch looks good to me and is a nicer way of accomplishing this. Thanks 
for taking care of it!


https://reviews.llvm.org/D24877



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

Re: [PATCH] D24877: [libc++] Default to DLL semantics on Windows

[Eric Fiselier via cfe-commits]

EricWF resigned from this revision.
EricWF removed a reviewer: EricWF.
EricWF added a comment.

I fixed this with a slightly different patch in r282449. I hope you don't mind 
that I didn't use this patch, I just wanted to add support for generating 
custom __config headers for static builds on Windows. Let me know if you have 
any issues with my patch.

Resigning as a reviewer since this issue is fixed.


https://reviews.llvm.org/D24877



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

[PATCH] D24877: [libc++] Default to DLL semantics on Windows

[Shoaib Meenai via cfe-commits]

smeenai created this revision.
smeenai added reviewers: compnerd, EricWF, mclow.lists.
smeenai added a subscriber: cfe-commits.

On Windows builds, we currently require `_LIBCPP_DLL` to be specified in
order for dllexport/dllimport annotations to be generated. I believe
it's better to do the opposite, i.e. require a macro to be present in
order to *not* include the annotations, since building libc++ shared is
more common.

The problem with having the DLL annotations be opt-in is that any
project using the libc++ headers and linking against a libc++ DLL must
specify `_LIBCPP_DLL` in its build system, otherwise the libc++ headers
will not be annotated with dllimport. This lack of annotation will then
cause inefficient code generation, since the linker will generate import
thunks. No compile-time errors will be reported (except in the case of
data symbols, for which import thunks are not possible) about this
inefficient code generation.

In contrast, having the DLL annotations be opt-out means that any
project using the libc++ headers and linking against a static libc++
will fail to link with missing import table entries if the project
forgets to specify the macro to suppress DLL annotations. Link errors
seem far preferable to silent inefficient code generation.

https://reviews.llvm.org/D24877

Files:
  include/__config

Index: include/__config
===
--- include/__config
+++ include/__config
@@ -548,15 +548,16 @@
 
 
 #ifdef _WIN32
-// only really useful for a DLL. _LIBCPP_DLL should be a compiler builtin 
define ideally...
-#if defined(_LIBCPP_DLL) && defined(cxx_EXPORTS)
+#if !defined(_LIBCPP_STATIC_BUILD)
+#if defined(cxx_EXPORTS)
 # define _LIBCPP_DLL_VIS __declspec(dllexport)
 # define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
 # define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS _LIBCPP_DLL_VIS
-#elif defined(_LIBCPP_DLL)
+#else
 # define _LIBCPP_DLL_VIS __declspec(dllimport)
 # define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS _LIBCPP_DLL_VIS
 # define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
+#endif
 #else
 # define _LIBCPP_DLL_VIS
 # define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS


Index: include/__config
===
--- include/__config
+++ include/__config
@@ -548,15 +548,16 @@
 
 
 #ifdef _WIN32
-// only really useful for a DLL. _LIBCPP_DLL should be a compiler builtin define ideally...
-#if defined(_LIBCPP_DLL) && defined(cxx_EXPORTS)
+#if !defined(_LIBCPP_STATIC_BUILD)
+#if defined(cxx_EXPORTS)
 # define _LIBCPP_DLL_VIS __declspec(dllexport)
 # define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
 # define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS _LIBCPP_DLL_VIS
-#elif defined(_LIBCPP_DLL)
+#else
 # define _LIBCPP_DLL_VIS __declspec(dllimport)
 # define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS _LIBCPP_DLL_VIS
 # define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
+#endif
 #else
 # define _LIBCPP_DLL_VIS
 # define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits