[PATCH] D12747: Implement [depr.c.headers]
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 8 21:12:44 PDT 2015
On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier <eric at 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 <eric at 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 <eric at efcs.ca> wrote:
>> >> Patch #10 LGTM.
>> >>
>> >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> >>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow <mclow.lists at gmail.com
>> >
>> >>> wrote:
>> >>>>
>> >>>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith <richard at 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.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151008/7735cf72/attachment-0001.html>
-------------- next part --------------
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
More information about the cfe-commits
mailing list