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> 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> wrote:
On Oct 15, 2015, at 1:41 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
On Thu, Oct 15, 2015 at 12:03 PM, Manman Ren via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
>
> On Oct 15, 2015, at 11:25 AM, Richard Smith <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> 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> 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
>> > 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
>> >
>> ==============================================================================
>> > --- 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
>> >
>> ==============================================================================
>> > --- 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
>> > 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
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
