<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="">Thanks Eric!<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Oct 12, 2016, at 9:18 PM, Eric Fiselier <<a href="mailto:eric@efcs.ca" class="">eric@efcs.ca</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Sure, I've reverted the change in r284101.<div class=""><br class=""></div><div class="">I'll try and respond in detail tomorrow.</div><div class=""><br class=""></div><div class="">/Eric</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Oct 12, 2016 at 9:57 PM, Mehdi Amini <span dir="ltr" class=""><<a href="mailto:mehdi.amini@apple.com" target="_blank" class="">mehdi.amini@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" 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" target="_blank" class="">http://lists.llvm.org/<wbr class="">pipermail/cfe-dev/2016-July/<wbr class="">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><span class="HOEnZb"><font color="#888888" class=""><div class="">Mehdi</div></font></span><div class=""><div class="h5"><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><div class=""><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" target="_blank" class="">cfe-commits@lists.llvm.org</a>> wrote:</div><br class="m_-959051960974549967Apple-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" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project?rev=282345&view=rev</a><br class="">Log:<br class="">Use __attribute__((internal_<wbr class="">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_<wbr class="">linkage))` instead of `__attribute__((visibility("<wbr class="">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_<wbr class="">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" target="_blank" class="">http://lists.llvm.org/<wbr class="">pipermail/cfe-dev/2015-<wbr class="">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" target="_blank" class="">https://reviews.llvm.org/<wbr class="">D24642</a><br class=""><br class="">Modified:<br class=""> libcxx/trunk/include/__<wbr class="">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" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/libcxx/trunk/include/_<wbr class="">_config?rev=282345&r1=282344&<wbr class="">r2=282345&view=diff</a><br class="">==============================<wbr class="">==============================<wbr 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_<wbr class="">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_<wbr class="">LAYOUT<br class="">@@ -49,6 +50,7 @@<br class=""> #define _LIBCPP_ABI_FIX_UNORDERED_<wbr class="">CONTAINER_SIZE_TYPE<br class=""> #define _LIBCPP_ABI_VARIADIC_LOCK_<wbr class="">GUARD<br class=""> #elif _LIBCPP_ABI_VERSION == 1<br class="">+#define _LIBCPP_ABI_EXTERN_TEMPLATE_<wbr class="">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_<wbr class="">linkage__)<br class="">+# define _LIBCPP_INLINE_VISIBILITY __attribute__((__internal_<wbr class="">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_<wbr class="">linkage__)<br class="">+# define _LIBCPP_ALWAYS_INLINE __attribute__((__internal_<wbr class="">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_<wbr class="">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" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/libcxx/trunk/src/<wbr class="">string.cpp?rev=282345&r1=<wbr class="">282344&r2=282345&view=diff</a><br class="">==============================<wbr class="">==============================<wbr 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_<wbr class="">linkage)) as opposed to<br class="">+// __attribute__((visibility("<wbr class="">hidden"), always_inline)).<br class="">+#if _LIBCPP_ABI_EXTERN_TEMPLATE_<wbr class="">SYMBOLS_VERSION == 1<br class="">+template basic_string<char>::iterator<br class="">+basic_string<char>::insert(<wbr class="">basic_string<char>::const_<wbr class="">iterator,<br class="">+ char const *, char const *);<br class="">+<br class="">+template basic_string<wchar_t>::<wbr class="">iterator<br class="">+basic_string<wchar_t>::<wbr class="">insert(basic_string<wchar_t>::<wbr class="">const_iterator,<br class="">+ <wbr class="">wchar_t const *, wchar_t const *);<br class="">+<br class="">+template basic_string<char> &<br class="">+basic_string<char>::replace(<wbr class="">basic_string<char>::const_<wbr class="">iterator,<br class="">+ <wbr class="">basic_string<char>::const_<wbr class="">iterator,<br class="">+ <wbr class="">char const *, char const *);<br class="">+<br class="">+template basic_string<wchar_t> &<br class="">+basic_string<wchar_t>::<wbr class="">replace(basic_string<wchar_t>:<wbr class="">:const_iterator,<br class="">+ <wbr class="">basic_string<wchar_t>::const_<wbr class="">iterator,<br class="">+ <wbr class="">wchar_t const *, wchar_t const *);<br class="">+#endif // _LIBCPP_ABI_EXTERN_TEMPLATE_<wbr class="">SYMBOLS_VERSION<br class="">+<br class=""> namespace<br class=""> {<br class=""><br class=""><br class=""><br class="">______________________________<wbr class="">_________________<br class="">cfe-commits mailing list<br class=""><a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank" class="">http://lists.llvm.org/cgi-bin/<wbr class="">mailman/listinfo/cfe-commits</a><br class=""></div></div></blockquote></div><br class=""></div></div></div></div></blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></body></html>