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 <rich...@metafoo.co.uk> wrote: > 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. >>> >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits