<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, Richard,<div class=""><br class=""></div><div class="">Any news on this? The test is still broken, should we revert the commit for now?</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Michael</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Oct 24, 2015, at 1:18 AM, Eric Fiselier <<a href="mailto:eric@efcs.ca" class="">eric@efcs.ca</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><p dir="ltr" class="">Hi Michael,</p><p dir="ltr" class="">Sorry I'm holding this patch up in review. The fix is quite "novel" and I want to make sure we get it right. If we can't land it over the weekend I'll ask Richard to revert while we work on it.</p><p dir="ltr" class="">/Eric</p>
<div class="gmail_quote">On Oct 23, 2015 10:13 PM, "Michael Zolotukhin via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a>> wrote:<br type="attribution" class=""><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="">Hi Richard,<div class=""><br class=""></div><div class="">Is this patch ready for commit, or were you just checking an idea? Our bots are still failing to build povray, so we’re really looking forward for some fix:)</div><div class=""><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Michael</div><div class="elided-text"><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Oct 15, 2015, at 6:21 PM, Manman Ren via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>> wrote:</div><br class=""><div class=""><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><blockquote type="cite" class=""><div class=""><br class="">On Oct 15, 2015, at 1:41 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank" class="">richard@metafoo.co.uk</a>> wrote:</div><br class=""><div class=""><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><div class="gmail_extra"><div class="gmail_quote">On Thu, Oct 15, 2015 at 12:03 PM, Manman Ren via cfe-commits<span class=""> </span><span dir="ltr" class=""><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>></span><span class=""> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><br class=""><div class=""><span class=""><blockquote type="cite" class=""><div class="">On Oct 15, 2015, at 11:25 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank" class="">richard@metafoo.co.uk</a>> wrote:</div><br class=""><div class=""><p dir="ltr" class="">I assume the code in question has a "using namespace std;"?</p><div class=""><br class=""></div></div></blockquote></span>Yes</div><div class=""><span class=""><br class=""><blockquote type="cite" class=""><div class=""><p dir="ltr" class="">I don't see any way around this other than giving up on trying to fix the function signatures here (or maybe adding a Clang feature to let us fix the bad signature).</p><div class=""><br class=""></div></div></blockquote></span>Can you elaborate on how to fix the bad signature by adding a Clang feature? I want to see how hard it is before giving up on trying to fix the signatures.</div></div></blockquote><div class=""><br class=""></div><div class="">I thought about this a bit more, and we already have a feature that can be used for this.</div><div class=""><br class=""></div><div class="">Please let me know if the attached patch resolves the issue for you. This should also fix the wrong overload sets for these functions being provided by <string.h> on Darwin.</div></div></div></div></div></blockquote><div class=""><br class=""></div>This works on my testing case. Thanks!!</div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><br class=""></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="">Manman</div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Eric, Marshall: the attached patch adds a macro _LIBCPP_PREFERRED_OVERLOAD that can be applied to a function to (a) mark it as a separate overload from any other function with the same signature without the overload, and (b) instruct the compiler that it's preferred over another function with the same signature without the attribute. This allows us to replace the libc function</div><div class=""><br class=""></div><div class=""> char *strchr(const char *, int);</div><div class=""><br class=""></div><div class="">with the C++ overload set:</div><div class=""><br class=""></div><div class=""> const char *strchr(const char *, int);</div><div class=""> char *strchr(char *, int);<br class=""></div><div class=""><br class=""></div><div class="">It only works with Clang, though; for other compilers, we leave the C library's signature alone (as we used to before my patches landed).</div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">Thanks,</div><div class="">Manman</div><div class=""><div class=""><div class=""><br class=""></div><div class=""><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote">On Oct 15, 2015 11:07 AM, "Manman Ren via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>> wrote:<br type="attribution" class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Richard,<br class=""><br class="">This is causing a failure when building povray on iOS.<br class=""><br class="">Compilation error:<br class="">/Users/buildslave/tmp/test-suite-externals/speccpu2006/benchspec/CPU2006/453.povray/src/fileinputoutput.cpp:364:20: error: call to 'strrchr' is ambiguous<br class=""> const char *p=strrchr(name, '.’);<br class=""><br class="">iOS.sdk/usr/include/string.h:87:7: note: candidate function<br class="">char *strrchr(const char *, int);<br class=""> ^<br class="">/Users/buildslave/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/cstring:109:46: note: candidate function<br class="">inline _LIBCPP_INLINE_VISIBILITY const char* strrchr(const char* __s, int __c) {return ::strrchr(__s, __c);}<br class=""><br class="">It is a little strange to have "char *strrchr(const char *, int);” in iOS. But it is already in our SDK.<br class=""><br class="">Do you have any suggestion on how to fix this?<br class=""><br class="">Thanks,<br class="">Manman<br class=""><br class="">> On Oct 9, 2015, at 6:25 PM, Richard Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>> wrote:<br class="">><br class="">> Author: rsmith<br class="">> Date: Fri Oct 9 20:25:31 2015<br class="">> New Revision: 249929<br class="">><br class="">> URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project?rev=249929&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=249929&view=rev</a><br class="">> Log:<br class="">> Split <string.h> out of <cstring>.<br class="">><br class="">> Also fix the overload set for the five functions whose signatures change in the<br class="">> case where we can fix it. This is already covered by existing tests for the<br class="">> affected systems.<br class="">><br class="">> Added:<br class="">> libcxx/trunk/include/string.h<br class="">> - copied, changed from r249736, libcxx/trunk/include/cstring<br class="">> Modified:<br class="">> libcxx/trunk/include/cstring<br class="">><br class="">> Modified: libcxx/trunk/include/cstring<br class="">> URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstring?rev=249929&r1=249928&r2=249929&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstring?rev=249929&r1=249928&r2=249929&view=diff</a><br class="">> ==============================================================================<br class="">> --- libcxx/trunk/include/cstring (original)<br class="">> +++ libcxx/trunk/include/cstring Fri Oct 9 20:25:31 2015<br class="">> @@ -78,37 +78,42 @@ using ::strcmp;<br class="">> using ::strncmp;<br class="">> using ::strcoll;<br class="">> using ::strxfrm;<br class="">> +using ::strcspn;<br class="">> +using ::strspn;<br class="">> +#ifndef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS<br class="">> +using ::strtok;<br class="">> +#endif<br class="">> +using ::memset;<br class="">> +using ::strerror;<br class="">> +using ::strlen;<br class="">><br class="">> -using ::memchr;<br class="">> +// MSVCRT, GNU libc and its derivates already have the correct prototype in<br class="">> +// <string.h> if __cplusplus is defined. This macro can be defined by users if<br class="">> +// their C library provides the right signature.<br class="">> +#if defined(__GLIBC__) || defined(_LIBCPP_MSVCRT) || defined(__sun__) || \<br class="">> + defined(_STRING_H_CPLUSPLUS_98_CONFORMANCE_)<br class="">> +#define _LIBCPP_STRING_H_HAS_CONST_OVERLOADS<br class="">> +#endif<br class="">><br class="">> +#ifdef _LIBCPP_STRING_H_HAS_CONST_OVERLOADS<br class="">> using ::strchr;<br class="">> -<br class="">> -using ::strcspn;<br class="">> -<br class="">> using ::strpbrk;<br class="">> -<br class="">> using ::strrchr;<br class="">> -<br class="">> -using ::strspn;<br class="">> -<br class="">> +using ::memchr;<br class="">> using ::strstr;<br class="">> -<br class="">> -// MSVCRT, GNU libc and its derivates already have the correct prototype in <string.h> #ifdef __cplusplus<br class="">> -#if !defined(__GLIBC__) && !defined(_LIBCPP_MSVCRT) && !defined(__sun__) && !defined(_STRING_H_CPLUSPLUS_98_CONFORMANCE_)<br class="">> +#else<br class="">> +inline _LIBCPP_INLINE_VISIBILITY const char* strchr(const char* __s, int __c) {return ::strchr(__s, __c);}<br class="">> inline _LIBCPP_INLINE_VISIBILITY char* strchr( char* __s, int __c) {return ::strchr(__s, __c);}<br class="">> +inline _LIBCPP_INLINE_VISIBILITY const char* strpbrk(const char* __s1, const char* __s2) {return ::strpbrk(__s1, __s2);}<br class="">> inline _LIBCPP_INLINE_VISIBILITY char* strpbrk( char* __s1, const char* __s2) {return ::strpbrk(__s1, __s2);}<br class="">> +inline _LIBCPP_INLINE_VISIBILITY const char* strrchr(const char* __s, int __c) {return ::strrchr(__s, __c);}<br class="">> inline _LIBCPP_INLINE_VISIBILITY char* strrchr( char* __s, int __c) {return ::strrchr(__s, __c);}<br class="">> +inline _LIBCPP_INLINE_VISIBILITY const void* memchr(const void* __s, int __c, size_t __n) {return ::memchr(__s, __c, __n);}<br class="">> inline _LIBCPP_INLINE_VISIBILITY void* memchr( void* __s, int __c, size_t __n) {return ::memchr(__s, __c, __n);}<br class="">> +inline _LIBCPP_INLINE_VISIBILITY const char* strstr(const char* __s1, const char* __s2) {return ::strstr(__s1, __s2);}<br class="">> inline _LIBCPP_INLINE_VISIBILITY char* strstr( char* __s1, const char* __s2) {return ::strstr(__s1, __s2);}<br class="">> #endif<br class="">><br class="">> -#ifndef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS<br class="">> -using ::strtok;<br class="">> -#endif<br class="">> -using ::memset;<br class="">> -using ::strerror;<br class="">> -using ::strlen;<br class="">> -<br class="">> _LIBCPP_END_NAMESPACE_STD<br class="">><br class="">> #endif // _LIBCPP_CSTRING<br class="">><br class="">> Copied: libcxx/trunk/include/string.h (from r249736, libcxx/trunk/include/cstring)<br class="">> URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string.h?p2=libcxx/trunk/include/string.h&p1=libcxx/trunk/include/cstring&r1=249736&r2=249929&rev=249929&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string.h?p2=libcxx/trunk/include/string.h&p1=libcxx/trunk/include/cstring&r1=249736&r2=249929&rev=249929&view=diff</a><br class="">> ==============================================================================<br class="">> --- libcxx/trunk/include/cstring (original)<br class="">> +++ libcxx/trunk/include/string.h Fri Oct 9 20:25:31 2015<br class="">> @@ -1,5 +1,5 @@<br class="">> // -*- C++ -*-<br class="">> -//===--------------------------- cstring ----------------------------------===//<br class="">> +//===--------------------------- string.h ---------------------------------===//<br class="">> //<br class="">> // The LLVM Compiler Infrastructure<br class="">> //<br class="">> @@ -8,19 +8,16 @@<br class="">> //<br class="">> //===----------------------------------------------------------------------===//<br class="">><br class="">> -#ifndef _LIBCPP_CSTRING<br class="">> -#define _LIBCPP_CSTRING<br class="">> +#ifndef _LIBCPP_STRING_H<br class="">> +#define _LIBCPP_STRING_H<br class="">><br class="">> /*<br class="">> - cstring synopsis<br class="">> + string.h synopsis<br class="">><br class="">> Macros:<br class="">><br class="">> NULL<br class="">><br class="">> -namespace std<br class="">> -{<br class="">> -<br class="">> Types:<br class="">><br class="">> size_t<br class="">> @@ -53,62 +50,14 @@ void* memset(void* s, int c, size_t n);<br class="">> char* strerror(int errnum);<br class="">> size_t strlen(const char* s);<br class="">><br class="">> -} // std<br class="">> -<br class="">> */<br class="">><br class="">> #include <__config><br class="">> -#include <string.h><br class="">><br class="">> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)<br class="">> #pragma GCC system_header<br class="">> #endif<br class="">><br class="">> -_LIBCPP_BEGIN_NAMESPACE_STD<br class="">> -<br class="">> -using ::size_t;<br class="">> -using ::memcpy;<br class="">> -using ::memmove;<br class="">> -using ::strcpy;<br class="">> -using ::strncpy;<br class="">> -using ::strcat;<br class="">> -using ::strncat;<br class="">> -using ::memcmp;<br class="">> -using ::strcmp;<br class="">> -using ::strncmp;<br class="">> -using ::strcoll;<br class="">> -using ::strxfrm;<br class="">> -<br class="">> -using ::memchr;<br class="">> -<br class="">> -using ::strchr;<br class="">> -<br class="">> -using ::strcspn;<br class="">> -<br class="">> -using ::strpbrk;<br class="">> -<br class="">> -using ::strrchr;<br class="">> -<br class="">> -using ::strspn;<br class="">> -<br class="">> -using ::strstr;<br class="">> -<br class="">> -// MSVCRT, GNU libc and its derivates already have the correct prototype in <string.h> #ifdef __cplusplus<br class="">> -#if !defined(__GLIBC__) && !defined(_LIBCPP_MSVCRT) && !defined(__sun__) && !defined(_STRING_H_CPLUSPLUS_98_CONFORMANCE_)<br class="">> -inline _LIBCPP_INLINE_VISIBILITY char* strchr( char* __s, int __c) {return ::strchr(__s, __c);}<br class="">> -inline _LIBCPP_INLINE_VISIBILITY char* strpbrk( char* __s1, const char* __s2) {return ::strpbrk(__s1, __s2);}<br class="">> -inline _LIBCPP_INLINE_VISIBILITY char* strrchr( char* __s, int __c) {return ::strrchr(__s, __c);}<br class="">> -inline _LIBCPP_INLINE_VISIBILITY void* memchr( void* __s, int __c, size_t __n) {return ::memchr(__s, __c, __n);}<br class="">> -inline _LIBCPP_INLINE_VISIBILITY char* strstr( char* __s1, const char* __s2) {return ::strstr(__s1, __s2);}<br class="">> -#endif<br class="">> -<br class="">> -#ifndef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS<br class="">> -using ::strtok;<br class="">> -#endif<br class="">> -using ::memset;<br class="">> -using ::strerror;<br class="">> -using ::strlen;<br class="">> -<br class="">> -_LIBCPP_END_NAMESPACE_STD<br class="">> +#include_next <string.h><br class="">><br class="">> -#endif // _LIBCPP_CSTRING<br class="">> +#endif // _LIBCPP_STRING_H<br class="">><br class="">><br class="">> _______________________________________________<br class="">> cfe-commits mailing list<br class="">><span class=""> </span><a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a><br class="">><span class=""> </span><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class=""><br 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" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class=""></blockquote></div></div></blockquote></div><br class=""></div></div></div><br 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" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class=""><br class=""></blockquote></div><br class=""></div></div><span class=""><string-wchar-overload-sets.diff></span></div></blockquote></div><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">_______________________________________________</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">cfe-commits mailing list</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class=""><a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a></span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a></span></div></blockquote></div><br class=""></div></div></div><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="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class="">
<br class=""></blockquote></div>
</div></blockquote></div><br class=""></div></body></html>