On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier <e...@efcs.ca> wrote: > >> Patch #12 needs revision. A bunch of function definitions were moved >> out of the std namespace and into the global. >> That change is incorrect. > > > Slightly updated version attached. I should probably explain what's going > on here in more detail: > > Per [c.strings]p4-p13, we're supposed to replace C functions of the form > > char *foo(const char*); > > with a pair of const-correct overloads of the form > > char *foo(char *); > const char *foo(const char*); > > Now, most C standard libraries will do this for us when included in C++ > mode (because it's not possible for the C++ library to fix this up after > the fact). For the cases where we *don't* believe we have such a > considerate C library, we add one declaration to C's overload, to get: > > char *foo(char*); > char *foo(const char*) > > ... which doesn't really help much, but is the closest we can get to the > right set of declarations. The declarations we add just dispatch to the C > declarations. > > These new declarations *should* be in the global namespace when including > <string.h>, and it makes sense for us to put them in the global namespace > when including <cstring> (otherwise, that header leaves us with a broken > overload set in the global namespace, containing just one of the two > expected functions). > > Anyway, most of the above is a description of what we did before. What's > new here is that we attempt to fix the overload set for both <string.h> and > for <cstring>, not just for the latter. At the end of all these changes, > you'll see that all that the <cfoo> headers do is to include the <foo.h> > header and use using-declarations to pull the names into namespace std; > this is no exception to that pattern. > Per Eric and my discussion on IRC, the pattern used by <cwchar> seems better here: If libc has left us with a bad overload set, don't try to fix the names in ::, just provide a complete set of overloads in namespace std. A patch for that approach is attached. On Thu, Oct 8, 2015 at 7:09 PM, Eric Fiselier <e...@efcs.ca> wrote: >> > Patch #11 LGTM. Any reason you removed the "#pragma diagnostic ignored >> > "-Wnonnull"" in test/std/depr/depr.c.headers/stdlib_h.pass.cpp? >> > I would like to leave it in so this test doesn't fail with older clang >> > versions. >> > >> > /Eric >> > >> > On Thu, Oct 8, 2015 at 6:47 PM, Eric Fiselier <e...@efcs.ca> wrote: >> >> Patch #10 LGTM. >> >> >> >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith <rich...@metafoo.co.uk> >> wrote: >> >>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow <mclow.li...@gmail.com >> > >> >>> wrote: >> >>>> >> >>>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith <rich...@metafoo.co.uk >> > >> >>>> wrote: >> >>>>> >> >>>>> <stddef.h>. This one is tricky: >> >>>>> >> >>>>> 1) There's an (undocumented) interface between the C standard >> library and >> >>>>> this header, where the macros __need_ptrdiff_t, __need_size_t, >> >>>>> __need_wchar_t, __need_NULL, __need_wint_t request just a piece of >> this >> >>>>> header rather than the whole thing. If we see any of those, just go >> straight >> >>>>> to the underlying header. >> >>>> >> >>>> >> >>>> Ok, but in that case we don't get nullptr. I suspect that's OK. >> >>>> >> >>>>> >> >>>>> 2) We probably don't want <stddef.h> to include <cstddef> (for >> >>>>> consistency with other headers) >> >>>> >> >>>> >> >>>> No, we do not! :-) >> >>>> >> >>>>> >> >>>>> , but <stddef.h> must provide a ::nullptr_t (which we don't want >> >>>>> <cstddef> to provide). So neither header includes the other. >> Instead, both >> >>>>> include <__nullptr> for std::nullptr_t, and we duplicate the >> definition of >> >>>>> max_align_t between them, in the case where the compiler's >> <stddef.h> >> >>>>> doesn't provide it. >> >>>>> >> >>>>> If you prefer, I could make <stddef.h> include <cstddef> to avoid >> the >> >>>>> duplication of the max_align_t logic. >> >>>> >> >>>> >> >>>> No; this is a minor annoyance, and layer jumping (<stdXXX.h> >> including >> >>>> <cstdXXX>) is a major annoyance - and I'm pretty sure that that >> would come >> >>>> back to bite us in the future. >> >>>> >> >>>> Looks ok to me. >> >>> >> >>> >> >>> Thanks, everything up to and including patch 09 is now committed. >> > >
Index: include/cstring =================================================================== --- include/cstring (revision 249736) +++ include/cstring (working copy) @@ -78,37 +78,34 @@ 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; - +#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 Index: include/string.h =================================================================== --- include/string.h (revision 0) +++ include/string.h (working copy) @@ -0,0 +1,69 @@ +// -*- C++ -*- +//===--------------------------- string.h ---------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef _LIBCPP_STRING_H +#define _LIBCPP_STRING_H + +/* + string.h synopsis + +Macros: + + NULL + +Types: + + size_t + +void* memcpy(void* restrict s1, const void* restrict s2, size_t n); +void* memmove(void* s1, const void* s2, size_t n); +char* strcpy (char* restrict s1, const char* restrict s2); +char* strncpy(char* restrict s1, const char* restrict s2, size_t n); +char* strcat (char* restrict s1, const char* restrict s2); +char* strncat(char* restrict s1, const char* restrict s2, size_t n); +int memcmp(const void* s1, const void* s2, size_t n); +int strcmp (const char* s1, const char* s2); +int strncmp(const char* s1, const char* s2, size_t n); +int strcoll(const char* s1, const char* s2); +size_t strxfrm(char* restrict s1, const char* restrict s2, size_t n); +const void* memchr(const void* s, int c, size_t n); + void* memchr( void* s, int c, size_t n); +const char* strchr(const char* s, int c); + char* strchr( char* s, int c); +size_t strcspn(const char* s1, const char* s2); +const char* strpbrk(const char* s1, const char* s2); + char* strpbrk( char* s1, const char* s2); +const char* strrchr(const char* s, int c); + char* strrchr( char* s, int c); +size_t strspn(const char* s1, const char* s2); +const char* strstr(const char* s1, const char* s2); + char* strstr( char* s1, const char* s2); +char* strtok(char* restrict s1, const char* restrict s2); +void* memset(void* s, int c, size_t n); +char* strerror(int errnum); +size_t strlen(const char* s); + +*/ + +#include <__config> + +#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) +#pragma GCC system_header +#endif + +#include_next <string.h> + +// MSVCRT, GNU libc and its derivates already have the correct prototype in <string.h> if __cplusplus is defined +#if defined(__GLIBC__) || defined(_LIBCPP_MSVCRT) || defined(__sun__) || \ + defined(_STRING_H_CPLUSPLUS_98_CONFORMANCE_) +#define _LIBCPP_STRING_H_HAS_CONST_OVERLOADS +#endif + +#endif // _LIBCPP_STRING_H
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits