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> 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 > 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
