[PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

As of r249890, all committed other than patches 12 (string.h) and 15 (more
tests).

On Thu, Oct 8, 2015 at 9:12 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> 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/20151009/deffc0ef/attachment-0001.html>