Hi Eric, Richard, Any news on this? The test is still broken, should we revert the commit for now?
Thanks, Michael > On Oct 24, 2015, at 1:18 AM, Eric Fiselier <e...@efcs.ca> wrote: > > Hi Michael, > > 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. > > /Eric > > On Oct 23, 2015 10:13 PM, "Michael Zolotukhin via cfe-commits" > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: > Hi Richard, > > 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:) > > Thanks, > Michael > >> On Oct 15, 2015, at 6:21 PM, Manman Ren via cfe-commits >> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> >>> >>> On Oct 15, 2015, at 1:41 PM, Richard Smith <rich...@metafoo.co.uk >>> <mailto:rich...@metafoo.co.uk>> wrote: >>> >>> On Thu, Oct 15, 2015 at 12:03 PM, Manman Ren via cfe-commits >>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>> >>>> On Oct 15, 2015, at 11:25 AM, Richard Smith <rich...@metafoo.co.uk >>>> <mailto:rich...@metafoo.co.uk>> wrote: >>>> >>>> I assume the code in question has a "using namespace std;"? >>>> >>>> >>> Yes >>> >>>> 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). >>>> >>>> >>> 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. >>> >>> I thought about this a bit more, and we already have a feature that can be >>> used for this. >>> >>> 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. >> >> This works on my testing case. Thanks!! >> >> Manman >> >>> >>> >>> 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 >>> >>> char *strchr(const char *, int); >>> >>> with the C++ overload set: >>> >>> const char *strchr(const char *, int); >>> char *strchr(char *, int); >>> >>> 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). >>> >>> Thanks, >>> Manman >>> >>> >>>> On Oct 15, 2015 11:07 AM, "Manman Ren via cfe-commits" >>>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>>> Hi Richard, >>>> >>>> This is causing a failure when building povray on iOS. >>>> >>>> Compilation error: >>>> /Users/buildslave/tmp/test-suite-externals/speccpu2006/benchspec/CPU2006/453.povray/src/fileinputoutput.cpp:364:20: >>>> error: call to 'strrchr' is ambiguous >>>> const char *p=strrchr(name, '.’); >>>> >>>> iOS.sdk/usr/include/string.h:87:7: note: candidate function >>>> char *strrchr(const char *, int); >>>> ^ >>>> /Users/buildslave/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/cstring:109:46: >>>> note: candidate function >>>> inline _LIBCPP_INLINE_VISIBILITY const char* strrchr(const char* __s, int >>>> __c) {return ::strrchr(__s, __c);} >>>> >>>> It is a little strange to have "char *strrchr(const char *, int);” in >>>> iOS. But it is already in our SDK. >>>> >>>> Do you have any suggestion on how to fix this? >>>> >>>> Thanks, >>>> Manman >>>> >>>> > On Oct 9, 2015, at 6:25 PM, Richard Smith via cfe-commits >>>> > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>>> > >>>> > Author: rsmith >>>> > Date: Fri Oct 9 20:25:31 2015 >>>> > New Revision: 249929 >>>> > >>>> > URL: http://llvm.org/viewvc/llvm-project?rev=249929&view=rev >>>> > <http://llvm.org/viewvc/llvm-project?rev=249929&view=rev> >>>> > Log: >>>> > Split <string.h> out of <cstring>. >>>> > >>>> > Also fix the overload set for the five functions whose signatures change >>>> > in the >>>> > case where we can fix it. This is already covered by existing tests for >>>> > the >>>> > affected systems. >>>> > >>>> > Added: >>>> > libcxx/trunk/include/string.h >>>> > - copied, changed from r249736, libcxx/trunk/include/cstring >>>> > Modified: >>>> > libcxx/trunk/include/cstring >>>> > >>>> > Modified: libcxx/trunk/include/cstring >>>> > URL: >>>> > http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstring?rev=249929&r1=249928&r2=249929&view=diff >>>> > >>>> > <http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstring?rev=249929&r1=249928&r2=249929&view=diff> >>>> > ============================================================================== >>>> > --- libcxx/trunk/include/cstring (original) >>>> > +++ libcxx/trunk/include/cstring Fri Oct 9 20:25:31 2015 >>>> > @@ -78,37 +78,42 @@ using ::strcmp; >>>> > using ::strncmp; >>>> > using ::strcoll; >>>> > using ::strxfrm; >>>> > +using ::strcspn; >>>> > +using ::strspn; >>>> > +#ifndef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS >>>> > +using ::strtok; >>>> > +#endif >>>> > +using ::memset; >>>> > +using ::strerror; >>>> > +using ::strlen; >>>> > >>>> > -using ::memchr; >>>> > +// MSVCRT, GNU libc and its derivates already have the correct >>>> > prototype in >>>> > +// <string.h> if __cplusplus is defined. This macro can be defined by >>>> > users if >>>> > +// their C library provides the right signature. >>>> > +#if defined(__GLIBC__) || defined(_LIBCPP_MSVCRT) || defined(__sun__) >>>> > || \ >>>> > + defined(_STRING_H_CPLUSPLUS_98_CONFORMANCE_) >>>> > +#define _LIBCPP_STRING_H_HAS_CONST_OVERLOADS >>>> > +#endif >>>> > >>>> > +#ifdef _LIBCPP_STRING_H_HAS_CONST_OVERLOADS >>>> > using ::strchr; >>>> > - >>>> > -using ::strcspn; >>>> > - >>>> > using ::strpbrk; >>>> > - >>>> > using ::strrchr; >>>> > - >>>> > -using ::strspn; >>>> > - >>>> > +using ::memchr; >>>> > using ::strstr; >>>> > - >>>> > -// MSVCRT, GNU libc and its derivates already have the correct >>>> > prototype in <string.h> #ifdef __cplusplus >>>> > -#if !defined(__GLIBC__) && !defined(_LIBCPP_MSVCRT) && >>>> > !defined(__sun__) && !defined(_STRING_H_CPLUSPLUS_98_CONFORMANCE_) >>>> > +#else >>>> > +inline _LIBCPP_INLINE_VISIBILITY const char* strchr(const char* __s, >>>> > int __c) {return ::strchr(__s, __c);} >>>> > inline _LIBCPP_INLINE_VISIBILITY char* strchr( char* __s, int >>>> > __c) {return ::strchr(__s, __c);} >>>> > +inline _LIBCPP_INLINE_VISIBILITY const char* strpbrk(const char* __s1, >>>> > const char* __s2) {return ::strpbrk(__s1, __s2);} >>>> > inline _LIBCPP_INLINE_VISIBILITY char* strpbrk( char* __s1, >>>> > const char* __s2) {return ::strpbrk(__s1, __s2);} >>>> > +inline _LIBCPP_INLINE_VISIBILITY const char* strrchr(const char* __s, >>>> > int __c) {return ::strrchr(__s, __c);} >>>> > inline _LIBCPP_INLINE_VISIBILITY char* strrchr( char* __s, >>>> > int __c) {return ::strrchr(__s, __c);} >>>> > +inline _LIBCPP_INLINE_VISIBILITY const void* memchr(const void* __s, >>>> > int __c, size_t __n) {return ::memchr(__s, __c, __n);} >>>> > inline _LIBCPP_INLINE_VISIBILITY void* memchr( void* __s, int >>>> > __c, size_t __n) {return ::memchr(__s, __c, __n);} >>>> > +inline _LIBCPP_INLINE_VISIBILITY const char* strstr(const char* __s1, >>>> > const char* __s2) {return ::strstr(__s1, __s2);} >>>> > inline _LIBCPP_INLINE_VISIBILITY char* strstr( char* __s1, >>>> > const char* __s2) {return ::strstr(__s1, __s2);} >>>> > #endif >>>> > >>>> > -#ifndef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS >>>> > -using ::strtok; >>>> > -#endif >>>> > -using ::memset; >>>> > -using ::strerror; >>>> > -using ::strlen; >>>> > - >>>> > _LIBCPP_END_NAMESPACE_STD >>>> > >>>> > #endif // _LIBCPP_CSTRING >>>> > >>>> > Copied: libcxx/trunk/include/string.h (from r249736, >>>> > libcxx/trunk/include/cstring) >>>> > URL: >>>> > 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 >>>> > >>>> > <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> >>>> > ============================================================================== >>>> > --- libcxx/trunk/include/cstring (original) >>>> > +++ libcxx/trunk/include/string.h Fri Oct 9 20:25:31 2015 >>>> > @@ -1,5 +1,5 @@ >>>> > // -*- C++ -*- >>>> > -//===--------------------------- cstring >>>> > ----------------------------------===// >>>> > +//===--------------------------- string.h >>>> > ---------------------------------===// >>>> > // >>>> > // The LLVM Compiler Infrastructure >>>> > // >>>> > @@ -8,19 +8,16 @@ >>>> > // >>>> > //===----------------------------------------------------------------------===// >>>> > >>>> > -#ifndef _LIBCPP_CSTRING >>>> > -#define _LIBCPP_CSTRING >>>> > +#ifndef _LIBCPP_STRING_H >>>> > +#define _LIBCPP_STRING_H >>>> > >>>> > /* >>>> > - cstring synopsis >>>> > + string.h synopsis >>>> > >>>> > Macros: >>>> > >>>> > NULL >>>> > >>>> > -namespace std >>>> > -{ >>>> > - >>>> > Types: >>>> > >>>> > size_t >>>> > @@ -53,62 +50,14 @@ void* memset(void* s, int c, size_t n); >>>> > char* strerror(int errnum); >>>> > size_t strlen(const char* s); >>>> > >>>> > -} // std >>>> > - >>>> > */ >>>> > >>>> > #include <__config> >>>> > -#include <string.h> >>>> > >>>> > #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) >>>> > #pragma GCC system_header >>>> > #endif >>>> > >>>> > -_LIBCPP_BEGIN_NAMESPACE_STD >>>> > - >>>> > -using ::size_t; >>>> > -using ::memcpy; >>>> > -using ::memmove; >>>> > -using ::strcpy; >>>> > -using ::strncpy; >>>> > -using ::strcat; >>>> > -using ::strncat; >>>> > -using ::memcmp; >>>> > -using ::strcmp; >>>> > -using ::strncmp; >>>> > -using ::strcoll; >>>> > -using ::strxfrm; >>>> > - >>>> > -using ::memchr; >>>> > - >>>> > -using ::strchr; >>>> > - >>>> > -using ::strcspn; >>>> > - >>>> > -using ::strpbrk; >>>> > - >>>> > -using ::strrchr; >>>> > - >>>> > -using ::strspn; >>>> > - >>>> > -using ::strstr; >>>> > - >>>> > -// MSVCRT, GNU libc and its derivates already have the correct >>>> > prototype in <string.h> #ifdef __cplusplus >>>> > -#if !defined(__GLIBC__) && !defined(_LIBCPP_MSVCRT) && >>>> > !defined(__sun__) && !defined(_STRING_H_CPLUSPLUS_98_CONFORMANCE_) >>>> > -inline _LIBCPP_INLINE_VISIBILITY char* strchr( char* __s, >>>> > int __c) {return ::strchr(__s, __c);} >>>> > -inline _LIBCPP_INLINE_VISIBILITY char* strpbrk( char* __s1, >>>> > const char* __s2) {return ::strpbrk(__s1, __s2);} >>>> > -inline _LIBCPP_INLINE_VISIBILITY char* strrchr( char* __s, >>>> > int __c) {return ::strrchr(__s, __c);} >>>> > -inline _LIBCPP_INLINE_VISIBILITY void* memchr( void* __s, >>>> > int __c, size_t __n) {return ::memchr(__s, __c, __n);} >>>> > -inline _LIBCPP_INLINE_VISIBILITY char* strstr( char* __s1, >>>> > const char* __s2) {return ::strstr(__s1, __s2);} >>>> > -#endif >>>> > - >>>> > -#ifndef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS >>>> > -using ::strtok; >>>> > -#endif >>>> > -using ::memset; >>>> > -using ::strerror; >>>> > -using ::strlen; >>>> > - >>>> > -_LIBCPP_END_NAMESPACE_STD >>>> > +#include_next <string.h> >>>> > >>>> > -#endif // _LIBCPP_CSTRING >>>> > +#endif // _LIBCPP_STRING_H >>>> > >>>> > >>>> > _______________________________________________ >>>> > cfe-commits mailing list >>>> > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>> > <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >>> >>> >>> <string-wchar-overload-sets.diff> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
