<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Eric,<div class=""><br class=""></div><div class="">This is not clear to me that this patch is correct as-is. </div><div class=""><br class=""></div><div class="">See the last message in the thread: <a href="http://lists.llvm.org/pipermail/cfe-dev/2016-July/049985.html" class="">http://lists.llvm.org/pipermail/cfe-dev/2016-July/049985.html</a></div><div class=""><br class=""></div><div class="">I think we should reach a consensus on the right course of actions about this first.</div><div class=""><br class=""></div><div class="">— </div><div class="">Mehdi</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class="">On Sep 24, 2016, at 8:14 PM, Eric Fiselier via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Author: ericwf<br class="">Date: Sat Sep 24 22:14:13 2016<br class="">New Revision: 282345<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=282345&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=282345&view=rev</a><br class="">Log:<br class="">Use __attribute__((internal_linkage)) when available.<br class=""><br class="">Summary:<br class="">This patch has been a long time coming (Thanks @eugenis). It changes `_LIBCPP_INLINE_VISIBILITY` to use `__attribute__((internal_linkage))` instead of `__attribute__((visibility("hidden"), always_inline))`.<br class=""><br class="">The point of `_LIBCPP_INLINE_VISIBILITY` is to prevent inline functions from being exported from both the libc++ library and from user libraries. This helps libc++ better manage it's ABI.<br class="">Previously this was done by forcing inlining and modifying the symbols visibility. However inlining isn't guaranteed and symbol visibility only affects shared libraries making this an imperfect solution. `internal_linkage` improves this situation by making all symbols local to the TU they are emitted in, regardless of inlining or visibility. IIRC the effect of applying `__attribute__((internal_linkage))` to an inline function is the same as applying `static`.<br class=""><br class="">For more information about the attribute see: <a href="http://lists.llvm.org/pipermail/cfe-dev/2015-October/045580.html" class="">http://lists.llvm.org/pipermail/cfe-dev/2015-October/045580.html</a><br class=""><br class="">Most of the work for this patch was done by @eugenis.<br class=""><br class=""><br class="">Reviewers: mclow.lists, eugenis<br class=""><br class="">Subscribers: eugenis, cfe-commits<br class=""><br class="">Differential Revision: <a href="https://reviews.llvm.org/D24642" class="">https://reviews.llvm.org/D24642</a><br class=""><br class="">Modified:<br class=""> libcxx/trunk/include/__config<br class=""> libcxx/trunk/src/string.cpp<br class=""><br class="">Modified: libcxx/trunk/include/__config<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=282345&r1=282344&r2=282345&view=diff" class="">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=282345&r1=282344&r2=282345&view=diff</a><br class="">==============================================================================<br class="">--- libcxx/trunk/include/__config (original)<br class="">+++ libcxx/trunk/include/__config Sat Sep 24 22:14:13 2016<br class="">@@ -34,6 +34,7 @@<br class=""> #endif<br class=""><br class=""> #if defined(_LIBCPP_ABI_UNSTABLE) || _LIBCPP_ABI_VERSION >= 2<br class="">+#define _LIBCPP_ABI_EXTERN_TEMPLATE_SYMBOLS_VERSION 2<br class=""> // Change short string representation so that string data starts at offset 0,<br class=""> // improving its alignment in some cases.<br class=""> #define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT<br class="">@@ -49,6 +50,7 @@<br class=""> #define _LIBCPP_ABI_FIX_UNORDERED_CONTAINER_SIZE_TYPE<br class=""> #define _LIBCPP_ABI_VARIADIC_LOCK_GUARD<br class=""> #elif _LIBCPP_ABI_VERSION == 1<br class="">+#define _LIBCPP_ABI_EXTERN_TEMPLATE_SYMBOLS_VERSION 1<br class=""> // Feature macros for disabling pre ABI v1 features. All of these options<br class=""> // are deprecated.<br class=""> #if defined(__FreeBSD__)<br class="">@@ -629,11 +631,19 @@ namespace std {<br class=""> #endif<br class=""><br class=""> #ifndef _LIBCPP_INLINE_VISIBILITY<br class="">-#define _LIBCPP_INLINE_VISIBILITY __attribute__ ((__visibility__("hidden"), __always_inline__))<br class="">+# if __has_attribute(__internal_linkage__)<br class="">+# define _LIBCPP_INLINE_VISIBILITY __attribute__((__internal_linkage__, __always_inline__))<br class="">+# else<br class="">+# define _LIBCPP_INLINE_VISIBILITY __attribute__ ((__visibility__("hidden"), __always_inline__))<br class="">+# endif<br class=""> #endif<br class=""><br class=""> #ifndef _LIBCPP_ALWAYS_INLINE<br class="">-#define _LIBCPP_ALWAYS_INLINE __attribute__ ((__visibility__("hidden"), __always_inline__))<br class="">+# if __has_attribute(__internal_linkage__)<br class="">+# define _LIBCPP_ALWAYS_INLINE __attribute__((__internal_linkage__, __always_inline__))<br class="">+# else<br class="">+# define _LIBCPP_ALWAYS_INLINE __attribute__ ((__visibility__("hidden"), __always_inline__))<br class="">+# endif<br class=""> #endif<br class=""><br class=""> #ifndef _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY<br class=""><br class="">Modified: libcxx/trunk/src/string.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/string.cpp?rev=282345&r1=282344&r2=282345&view=diff" class="">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/string.cpp?rev=282345&r1=282344&r2=282345&view=diff</a><br class="">==============================================================================<br class="">--- libcxx/trunk/src/string.cpp (original)<br class="">+++ libcxx/trunk/src/string.cpp Sat Sep 24 22:14:13 2016<br class="">@@ -29,6 +29,29 @@ template<br class=""> string<br class=""> operator+<char, char_traits<char>, allocator<char> >(char const*, string const&);<br class=""><br class="">+// These external instantiations are required to maintain dylib compatibility<br class="">+// for ABI v1 when using __attribute__((internal_linkage)) as opposed to<br class="">+// __attribute__((visibility("hidden"), always_inline)).<br class="">+#if _LIBCPP_ABI_EXTERN_TEMPLATE_SYMBOLS_VERSION == 1<br class="">+template basic_string<char>::iterator<br class="">+basic_string<char>::insert(basic_string<char>::const_iterator,<br class="">+ char const *, char const *);<br class="">+<br class="">+template basic_string<wchar_t>::iterator<br class="">+basic_string<wchar_t>::insert(basic_string<wchar_t>::const_iterator,<br class="">+ wchar_t const *, wchar_t const *);<br class="">+<br class="">+template basic_string<char> &<br class="">+basic_string<char>::replace(basic_string<char>::const_iterator,<br class="">+ basic_string<char>::const_iterator,<br class="">+ char const *, char const *);<br class="">+<br class="">+template basic_string<wchar_t> &<br class="">+basic_string<wchar_t>::replace(basic_string<wchar_t>::const_iterator,<br class="">+ basic_string<wchar_t>::const_iterator,<br class="">+ wchar_t const *, wchar_t const *);<br class="">+#endif // _LIBCPP_ABI_EXTERN_TEMPLATE_SYMBOLS_VERSION<br class="">+<br class=""> namespace<br class=""> {<br class=""><br class=""><br class=""><br class="">_______________________________________________<br class="">cfe-commits mailing list<br class=""><a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits<br class=""></div></div></blockquote></div><br class=""></div></body></html>