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

[Richard Smith via cfe-commits]

rsmith abandoned this revision.
rsmith added a comment.

Patch has been split up and the individual parts have all been committed 
(except the module map changes, which are currently problematic due to libc / 
libc++ layering issues).


Repository:
  rL LLVM

http://reviews.llvm.org/D12747



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

On Fri, Oct 9, 2015 at 3:48 PM, Eric Fiselier  wrote:

> Regarding Patch #15.
>
> 1. Tests under 'test/std' shouldn't directly include <__config> or
> depend on any libc++ implementation details. We are trying to make the
> test suite generic so refrain from referencing libc++ symbols.
>

OK, I'll include a different header instead.

2. "static_assert" is C++11 only but this test should work in C++03.
> Can you use "#if TEST_STD_VER >= 11" from "test_macros.h" to use
> static assert in C++11 and just "assert" in C++03 (or something
> similar)?
>

libc++ provides static_assert emulation in the cases where it's not
available, and other tests are using it unconditionally.


> 3. Could you throw the standarese that requires this behavior at the
> top of the test?
>

Done.


> LGTM after you address those points.


Thanks, all done in r249931 (other than the one reverted patch).

/Eric
>
>
> On Fri, Oct 9, 2015 at 4:26 PM, Eric Fiselier  wrote:
> > Patch #12 LGTM. Thanks for doing tho cwchar approach in this patch.
> > One small nit. I would prefer a "negative" feature macro for
> > "_LIBCPP_STRING_H_HAS_CONST_OVERLOADS" because correct defaults
> > shouldn't need a macro definition to be selected. (ie
> > _LIBCPP_STRING_H_HAS_NO_CONST_OVERLOAD.)
> >
> > /Eric
> >
> > On Fri, Oct 9, 2015 at 1:59 PM, Richard Smith 
> wrote:
> >> 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 
> wrote:
> >>>
> >>> On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith 
> >>> wrote:
> 
>  On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier  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
>  , and it makes sense for us to put them in the global
> namespace
>  when including  (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
>  and
>  for , not just for the latter. At the end of all these
> changes,
>  you'll see that all that the  headers do is to include the
> 
>  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  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  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 
> 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
> > >>> 
> > >>> wrote:
> > 
> >  On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith
> >  
> >  wrote:
> > >
> > > . 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
> > > he

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

[Eric Fiselier via cfe-commits]

@Marshall, @Richard Have we fixed the Solaris build yet?

On Fri, Oct 9, 2015 at 4:48 PM, Eric Fiselier  wrote:
> Regarding Patch #15.
>
> 1. Tests under 'test/std' shouldn't directly include <__config> or
> depend on any libc++ implementation details. We are trying to make the
> test suite generic so refrain from referencing libc++ symbols.
> 2. "static_assert" is C++11 only but this test should work in C++03.
> Can you use "#if TEST_STD_VER >= 11" from "test_macros.h" to use
> static assert in C++11 and just "assert" in C++03 (or something
> similar)?
> 3. Could you throw the standarese that requires this behavior at the
> top of the test?
>
> LGTM after you address those points.
>
> /Eric
>
>
> On Fri, Oct 9, 2015 at 4:26 PM, Eric Fiselier  wrote:
>> Patch #12 LGTM. Thanks for doing tho cwchar approach in this patch.
>> One small nit. I would prefer a "negative" feature macro for
>> "_LIBCPP_STRING_H_HAS_CONST_OVERLOADS" because correct defaults
>> shouldn't need a macro definition to be selected. (ie
>> _LIBCPP_STRING_H_HAS_NO_CONST_OVERLOAD.)
>>
>> /Eric
>>
>> On Fri, Oct 9, 2015 at 1:59 PM, Richard Smith  wrote:
>>> 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  wrote:

 On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith 
 wrote:
>
> On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier  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
> , and it makes sense for us to put them in the global namespace
> when including  (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  
> and
> for , not just for the latter. At the end of all these changes,
> you'll see that all that the  headers do is to include the 
> 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  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  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  wrote:
>> >> Patch #10 LGTM.
>> >>
>> >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith 
>> >> wrote:
>> >>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow
>> >>> 
>> >>> wrote:
>> 
>>  On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith
>>  
>>  wrote:
>> >
>> > . 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  to include  (for
>> > consistency with oth

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

[Eric Fiselier via cfe-commits]

Regarding Patch #15.

1. Tests under 'test/std' shouldn't directly include <__config> or
depend on any libc++ implementation details. We are trying to make the
test suite generic so refrain from referencing libc++ symbols.
2. "static_assert" is C++11 only but this test should work in C++03.
Can you use "#if TEST_STD_VER >= 11" from "test_macros.h" to use
static assert in C++11 and just "assert" in C++03 (or something
similar)?
3. Could you throw the standarese that requires this behavior at the
top of the test?

LGTM after you address those points.

/Eric


On Fri, Oct 9, 2015 at 4:26 PM, Eric Fiselier  wrote:
> Patch #12 LGTM. Thanks for doing tho cwchar approach in this patch.
> One small nit. I would prefer a "negative" feature macro for
> "_LIBCPP_STRING_H_HAS_CONST_OVERLOADS" because correct defaults
> shouldn't need a macro definition to be selected. (ie
> _LIBCPP_STRING_H_HAS_NO_CONST_OVERLOAD.)
>
> /Eric
>
> On Fri, Oct 9, 2015 at 1:59 PM, Richard Smith  wrote:
>> 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  wrote:
>>>
>>> On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith 
>>> wrote:

 On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier  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
 , and it makes sense for us to put them in the global namespace
 when including  (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  and
 for , not just for the latter. At the end of all these changes,
 you'll see that all that the  headers do is to include the 
 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  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  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  wrote:
> >> Patch #10 LGTM.
> >>
> >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith 
> >> wrote:
> >>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow
> >>> 
> >>> wrote:
> 
>  On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith
>  
>  wrote:
> >
> > . 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  to include  (for
> > consistency with other headers)
> 
> 
>  No, we do not! :-)
> 
> >
> > , but  must provide a ::nullptr_t (which we don't want
> >  to provide). So neither header includes the other.
> > Instead, both
> > incl

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

[Eric Fiselier via cfe-commits]

Patch #12 LGTM. Thanks for doing tho cwchar approach in this patch.
One small nit. I would prefer a "negative" feature macro for
"_LIBCPP_STRING_H_HAS_CONST_OVERLOADS" because correct defaults
shouldn't need a macro definition to be selected. (ie
_LIBCPP_STRING_H_HAS_NO_CONST_OVERLOAD.)

/Eric

On Fri, Oct 9, 2015 at 1:59 PM, Richard Smith  wrote:
> 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  wrote:
>>
>> On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith 
>> wrote:
>>>
>>> On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier  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
>>> , and it makes sense for us to put them in the global namespace
>>> when including  (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  and
>>> for , not just for the latter. At the end of all these changes,
>>> you'll see that all that the  headers do is to include the 
>>> 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  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  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  wrote:
 >> Patch #10 LGTM.
 >>
 >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith 
 >> wrote:
 >>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow
 >>> 
 >>> wrote:
 
  On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith
  
  wrote:
 >
 > . 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  to include  (for
 > consistency with other headers)
 
 
  No, we do not! :-)
 
 >
 > , but  must provide a ::nullptr_t (which we don't want
 >  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
 > 
 > doesn't provide it.
 >
 > If you prefer, I could make  include  to avoid
 > the
 > duplication of the max_align_t logic.
 
 
  No; this is a minor annoyance, and layer jumping (
  including
  ) 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-comm

Re: [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  wrote:

> On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith 
> wrote:
>
>> On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier  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
>> , and it makes sense for us to put them in the global namespace
>> when including  (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  and
>> for , not just for the latter. At the end of all these changes,
>> you'll see that all that the  headers do is to include the 
>> 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  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  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  wrote:
>>> >> Patch #10 LGTM.
>>> >>
>>> >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith 
>>> 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:
>>> >
>>> > . 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  to include  (for
>>> > consistency with other headers)
>>> 
>>> 
>>>  No, we do not! :-)
>>> 
>>> >
>>> > , but  must provide a ::nullptr_t (which we don't want
>>> >  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
>>> 
>>> > doesn't provide it.
>>> >
>>> > If you prefer, I could make  include  to avoid
>>> the
>>> > duplication of the max_align_t logic.
>>> 
>>> 
>>>  No; this is a minor annoyance, and layer jumping (
>>> including
>>>  ) 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

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

[Richard Smith via cfe-commits]

On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith  wrote:

> On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier  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
> , and it makes sense for us to put them in the global namespace
> when including  (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  and
> for , not just for the latter. At the end of all these changes,
> you'll see that all that the  headers do is to include the 
> 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  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  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  wrote:
>> >> Patch #10 LGTM.
>> >>
>> >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith 
>> wrote:
>> >>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow > >
>> >>> wrote:
>> 
>>  On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith > >
>>  wrote:
>> >
>> > . 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  to include  (for
>> > consistency with other headers)
>> 
>> 
>>  No, we do not! :-)
>> 
>> >
>> > , but  must provide a ::nullptr_t (which we don't want
>> >  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
>> 
>> > doesn't provide it.
>> >
>> > If you prefer, I could make  include  to avoid
>> the
>> > duplication of the max_align_t logic.
>> 
>> 
>>  No; this is a minor annoyance, and layer jumping (
>> including
>>  ) 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 
 #ifdef __cplusplus
-#if !defined(__GLIBC__) && !defined(_LIBCPP_MSVCRT) && !defined(__sun__) && 
!defined(_STRING_H_CPLUSPLUS_98_CONFORMANCE_)
+#else
+inline _LIBCPP_INLINE_VISIBILITY const cha

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

[Richard Smith via cfe-commits]

Attached patch adds a test that all required functions from the C standard
library (and any required overloads) are present with the correct types,
and that the declarations in the  and  headers declare the
same entity as required by [depr.c.headers]p2.

On Thu, Oct 8, 2015 at 6:58 PM, Richard Smith  wrote:

> On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier  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
> , and it makes sense for us to put them in the global namespace
> when including  (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  and
> for , not just for the latter. At the end of all these changes,
> you'll see that all that the  headers do is to include the 
> header and use using-declarations to pull the names into namespace std;
> this is no exception to that pattern.
>
>
>> On Thu, Oct 8, 2015 at 7:09 PM, Eric Fiselier  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  wrote:
>> >> Patch #10 LGTM.
>> >>
>> >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith 
>> wrote:
>> >>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow > >
>> >>> wrote:
>> 
>>  On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith > >
>>  wrote:
>> >
>> > . 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  to include  (for
>> > consistency with other headers)
>> 
>> 
>>  No, we do not! :-)
>> 
>> >
>> > , but  must provide a ::nullptr_t (which we don't want
>> >  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
>> 
>> > doesn't provide it.
>> >
>> > If you prefer, I could make  include  to avoid
>> the
>> > duplication of the max_align_t logic.
>> 
>> 
>>  No; this is a minor annoyance, and layer jumping (
>> including
>>  ) 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.
>>
>
>
diff --git test/std/depr/depr.c.headers/same_decls.pass.cpp 
test/std/depr/depr.c.headers/same_decls.pass.cpp
new file mode 100644
index 000..c5a5cf1
--- /dev/null
+++ test/std/depr/depr.c.headers/same_decls.pass.cpp
@@ -0,0 +1,513 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Every C header behaves as if the names in namespace std are placed into the
+// global namespace. This implies that the addresses of such functions must
+// match between the declaration in the global namespace and the declaration in
+// namespace std.
+
+#include <__

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

[Richard Smith via cfe-commits]

On Thu, Oct 8, 2015 at 6:25 PM, Eric Fiselier  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
, and it makes sense for us to put them in the global namespace
when including  (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  and
for , not just for the latter. At the end of all these changes,
you'll see that all that the  headers do is to include the 
header and use using-declarations to pull the names into namespace std;
this is no exception to that pattern.


> On Thu, Oct 8, 2015 at 7:09 PM, Eric Fiselier  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  wrote:
> >> Patch #10 LGTM.
> >>
> >> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith 
> wrote:
> >>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow 
> >>> wrote:
> 
>  On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith 
>  wrote:
> >
> > . 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  to include  (for
> > consistency with other headers)
> 
> 
>  No, we do not! :-)
> 
> >
> > , but  must provide a ::nullptr_t (which we don't want
> >  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 
> > doesn't provide it.
> >
> > If you prefer, I could make  include  to avoid the
> > duplication of the max_align_t logic.
> 
> 
>  No; this is a minor annoyance, and layer jumping ( including
>  ) 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,30 +78,13 @@
 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 
 #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
Index: include/string.h
=

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

[Eric Fiselier via cfe-commits]

Patch #14 LGTM modulo pragmas.

On Thu, Oct 8, 2015 at 7:39 PM, Eric Fiselier  wrote:
> Patch #13 LGTM after revision.
>
> a system header pragma needs to be added to the __need_wint_t path of wchar.h.
> The existing pragma also needs fixing as previously discussed.
>
> On Thu, Oct 8, 2015 at 7:25 PM, Eric Fiselier  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.
>>
>>
>>
>> On Thu, Oct 8, 2015 at 7:09 PM, Eric Fiselier  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  wrote:
 Patch #10 LGTM.

 On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith  
 wrote:
> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow 
> wrote:
>>
>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith 
>> wrote:
>>>
>>> . 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  to include  (for
>>> consistency with other headers)
>>
>>
>> No, we do not! :-)
>>
>>>
>>> , but  must provide a ::nullptr_t (which we don't want
>>>  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 
>>> doesn't provide it.
>>>
>>> If you prefer, I could make  include  to avoid the
>>> duplication of the max_align_t logic.
>>
>>
>> No; this is a minor annoyance, and layer jumping ( including
>> ) 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

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

[Eric Fiselier via cfe-commits]

Patch #13 LGTM after revision.

a system header pragma needs to be added to the __need_wint_t path of wchar.h.
The existing pragma also needs fixing as previously discussed.

On Thu, Oct 8, 2015 at 7:25 PM, Eric Fiselier  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.
>
>
>
> On Thu, Oct 8, 2015 at 7:09 PM, Eric Fiselier  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  wrote:
>>> Patch #10 LGTM.
>>>
>>> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith  wrote:
 On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow 
 wrote:
>
> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith 
> wrote:
>>
>> . 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  to include  (for
>> consistency with other headers)
>
>
> No, we do not! :-)
>
>>
>> , but  must provide a ::nullptr_t (which we don't want
>>  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 
>> doesn't provide it.
>>
>> If you prefer, I could make  include  to avoid the
>> duplication of the max_align_t logic.
>
>
> No; this is a minor annoyance, and layer jumping ( including
> ) 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

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

[Eric Fiselier via cfe-commits]

Patch #12 needs revision. A bunch of function definitions were moved
out of the std namespace and into the global.
That change is incorrect.



On Thu, Oct 8, 2015 at 7:09 PM, Eric Fiselier  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  wrote:
>> Patch #10 LGTM.
>>
>> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith  wrote:
>>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow 
>>> wrote:

 On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith 
 wrote:
>
> . 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  to include  (for
> consistency with other headers)


 No, we do not! :-)

>
> , but  must provide a ::nullptr_t (which we don't want
>  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 
> doesn't provide it.
>
> If you prefer, I could make  include  to avoid the
> duplication of the max_align_t logic.


 No; this is a minor annoyance, and layer jumping ( including
 ) 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

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

[Eric Fiselier via cfe-commits]

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  wrote:
> Patch #10 LGTM.
>
> On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith  wrote:
>> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow 
>> wrote:
>>>
>>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith 
>>> wrote:

 . 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  to include  (for
 consistency with other headers)
>>>
>>>
>>> No, we do not! :-)
>>>

 , but  must provide a ::nullptr_t (which we don't want
  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 
 doesn't provide it.

 If you prefer, I could make  include  to avoid the
 duplication of the max_align_t logic.
>>>
>>>
>>> No; this is a minor annoyance, and layer jumping ( including
>>> ) 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

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

[Eric Fiselier via cfe-commits]

Patch #10 LGTM.

On Thu, Oct 8, 2015 at 4:28 PM, Richard Smith  wrote:
> On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow 
> wrote:
>>
>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith 
>> wrote:
>>>
>>> . 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  to include  (for
>>> consistency with other headers)
>>
>>
>> No, we do not! :-)
>>
>>>
>>> , but  must provide a ::nullptr_t (which we don't want
>>>  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 
>>> doesn't provide it.
>>>
>>> If you prefer, I could make  include  to avoid the
>>> duplication of the max_align_t logic.
>>
>>
>> No; this is a minor annoyance, and layer jumping ( including
>> ) 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

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

[Richard Smith via cfe-commits]

On Thu, Oct 8, 2015 at 11:50 AM, Marshall Clow 
wrote:

> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith 
> wrote:
>
>> . 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  to include  (for
>> consistency with other headers)
>>
>
> No, we do not! :-)
>
>
>> , but  must provide a ::nullptr_t (which we don't want
>>  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 
>> doesn't provide it.
>>
>> If you prefer, I could make  include  to avoid the
>> duplication of the max_align_t logic.
>>
>
> No; this is a minor annoyance, and layer jumping ( including
> ) 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

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

[Richard Smith via cfe-commits]

On Thu, Oct 8, 2015 at 7:23 AM, Marshall Clow  wrote:

> On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
> wrote:
>
>> Split  out of . This is a big change, but the same pattern
>> as the prior ones.
>>
>> In this patch, you replicate the #ifdef XXX, __libcpp_XXX, #undef XXX
> dance for all the isXXX functions. Is that because they're not required to
> be actual functions in a C library?
>

Yes. Per C11 7.12.3, the following are defined as type-generic macros that
take any real floating-point type:

  fpclassify, isfinite, isinf, isnan, isnormal, signbit

Per C11 7.12.14, the following are defined as type-generic macros that take
any pair of real floating-point types:

  isgreater, isgreaterequal, isless, islessequal, islessgreater, isunordered

Per [c.math]p10, we are required to remove those macros and replace them
with a set of overloaded functions. For those cases (and *only* those
cases), we define a function template to capture the contents of the macro,
then undefine the macro.

Note that libc++'s behavior here is non-conforming (both before and after
this change) because it's required to provide a specific set of overloads,
but instead only provides a template. This patch series neither improves
nor regresses libc++'s conformance in that area.


> Other than that Q, LGTM. Like the extended tests.
>
> -- Marshall
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

On Thu, Oct 8, 2015 at 6:27 AM, Marshall Clow  wrote:

> On Wed, Oct 7, 2015 at 2:38 PM, Richard Smith 
> wrote:
>
>> Marshall: ping, does the below satisfy your concerns about the direction
>> here?
>>
>
> No, not really, because I'm worried about behavior changes with this
> approach.
>
> #include 
>isdigit(c);
>
> will call different code before and after this patch.
> Before the patch, it will use the macro version.
>

That was non-conforming behaviour, per [headers]/6:

  "Names that are defined as functions in C shall be defined as functions
in the C++ standard library. [Footnote: This disallows the practice,
allowed in C, of providing a masking macro in addition to the function
prototype.]"

After, it will use the built-in function.
>
> However, since other standard libraries use this approach, this is
> probably a baseless concern.
>
> Assuming that my concerns are unfounded, the first six patches
> (remove-macros, nullptr, ctype, errno and float) look fine to me.
>
> Working on the rest.
>
> -- Marshall
>
>
>
>> On Wed, Sep 16, 2015 at 2:04 PM, Richard Smith 
>> wrote:
>>
>>> On Mon, Sep 14, 2015 at 7:07 AM, Marshall Clow 
>>> wrote:
>>>
 mclow.lists added a comment.

 I have two concerns about this patch (w/o commenting on the actual
 code).

 1. Until very recently, I was under the impression that C libraries
 _either_ defined a macro, or had a function. I was quite surprised to find
 that glibc did both.
>>>
>>>
>>> Yes, this is required by the C standard. C11 7.1.4/1 says:
>>>
>>> "Any function declared in a header may be additionally implemented as a
>>> function-like macro defined in the header [...]. Any macro definition of a
>>> function can be suppressed locally by enclosing the name of the function in
>>> parentheses, because the name is then not followed by the left parenthesis
>>> that indicates expansion of a macro function name. For the same syntactic
>>> reason, it is permitted to take the address of a library function even if
>>> it is also defined as a macro. [Footnote: This means that an implementation
>>> shall provide an actual function for each library function, even if it also
>>> provides a macro for that function.]"
>>>
>>> Have you checked other C libraries (Apple, FreeBSD, Android, Windows) to
 see if they also define both?
>>>
>>>
>>> No, but libstdc++ does the same #undef thing, so any platform it
>>> supports must have a non-broken C standard library.
>>>
>>>
 2. This adds a lot of header files. Each header file slows down
 compilation, and standard library header files get included *a lot*. We may
 not be able to avoid this, but we should think about the costs here.
>>>
>>>
>>> I created a .cpp file that includes all of the <*.h> headers and does
>>> nothing else (which should maximize the performance difference), and built
>>> it with and without this change. I could not measure any difference (the
>>> average compile time with this change was slightly lower, but that is
>>> almost certainly noise).
>>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Marshall Clow via cfe-commits]

On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith  wrote:

> . 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  to include  (for consistency
> with other headers)
>

No, we do not! :-)


> , but  must provide a ::nullptr_t (which we don't want 
> 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 
> doesn't provide it.
>
> If you prefer, I could make  include  to avoid the
> duplication of the max_align_t logic.
>

No; this is a minor annoyance, and layer jumping ( including
) is a major annoyance - and I'm pretty sure that that would come
back to bite us in the future.

Looks ok to me.

-- Marshall
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Marshall Clow via cfe-commits]

On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith  wrote:

> , an easy one. We guarantee a setjmp macro exists even if this
> header is somehow included from C (the C standard allows that, so it's not
> worth checking for __cplusplus).
>
> This looks fine to me.

-- Marshall
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Marshall Clow via cfe-commits]

On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith  wrote:

> Split  out of . This is a big change, but the same pattern
> as the prior ones.
>
> In this patch, you replicate the #ifdef XXX, __libcpp_XXX, #undef XXX
dance for all the isXXX functions. Is that because they're not required to
be actual functions in a C library?

Other than that Q, LGTM. Like the extended tests.

-- Marshall
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Marshall Clow via cfe-commits]

On Wed, Oct 7, 2015 at 2:38 PM, Richard Smith  wrote:

> Marshall: ping, does the below satisfy your concerns about the direction
> here?
>

No, not really, because I'm worried about behavior changes with this
approach.

#include 
   isdigit(c);

will call different code before and after this patch.
Before the patch, it will use the macro version.
After, it will use the built-in function.

However, since other standard libraries use this approach, this is probably
a baseless concern.

Assuming that my concerns are unfounded, the first six patches
(remove-macros, nullptr, ctype, errno and float) look fine to me.

Working on the rest.

-- Marshall



> On Wed, Sep 16, 2015 at 2:04 PM, Richard Smith 
> wrote:
>
>> On Mon, Sep 14, 2015 at 7:07 AM, Marshall Clow 
>> wrote:
>>
>>> mclow.lists added a comment.
>>>
>>> I have two concerns about this patch (w/o commenting on the actual code).
>>>
>>> 1. Until very recently, I was under the impression that C libraries
>>> _either_ defined a macro, or had a function. I was quite surprised to find
>>> that glibc did both.
>>
>>
>> Yes, this is required by the C standard. C11 7.1.4/1 says:
>>
>> "Any function declared in a header may be additionally implemented as a
>> function-like macro defined in the header [...]. Any macro definition of a
>> function can be suppressed locally by enclosing the name of the function in
>> parentheses, because the name is then not followed by the left parenthesis
>> that indicates expansion of a macro function name. For the same syntactic
>> reason, it is permitted to take the address of a library function even if
>> it is also defined as a macro. [Footnote: This means that an implementation
>> shall provide an actual function for each library function, even if it also
>> provides a macro for that function.]"
>>
>> Have you checked other C libraries (Apple, FreeBSD, Android, Windows) to
>>> see if they also define both?
>>
>>
>> No, but libstdc++ does the same #undef thing, so any platform it supports
>> must have a non-broken C standard library.
>>
>>
>>> 2. This adds a lot of header files. Each header file slows down
>>> compilation, and standard library header files get included *a lot*. We may
>>> not be able to avoid this, but we should think about the costs here.
>>
>>
>> I created a .cpp file that includes all of the <*.h> headers and does
>> nothing else (which should maximize the performance difference), and built
>> it with and without this change. I could not measure any difference (the
>> average compile time with this change was slightly lower, but that is
>> almost certainly noise).
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

Marshall: ping, does the below satisfy your concerns about the direction
here?

On Wed, Sep 16, 2015 at 2:04 PM, Richard Smith 
wrote:

> On Mon, Sep 14, 2015 at 7:07 AM, Marshall Clow 
> wrote:
>
>> mclow.lists added a comment.
>>
>> I have two concerns about this patch (w/o commenting on the actual code).
>>
>> 1. Until very recently, I was under the impression that C libraries
>> _either_ defined a macro, or had a function. I was quite surprised to find
>> that glibc did both.
>
>
> Yes, this is required by the C standard. C11 7.1.4/1 says:
>
> "Any function declared in a header may be additionally implemented as a
> function-like macro defined in the header [...]. Any macro definition of a
> function can be suppressed locally by enclosing the name of the function in
> parentheses, because the name is then not followed by the left parenthesis
> that indicates expansion of a macro function name. For the same syntactic
> reason, it is permitted to take the address of a library function even if
> it is also defined as a macro. [Footnote: This means that an implementation
> shall provide an actual function for each library function, even if it also
> provides a macro for that function.]"
>
> Have you checked other C libraries (Apple, FreeBSD, Android, Windows) to
>> see if they also define both?
>
>
> No, but libstdc++ does the same #undef thing, so any platform it supports
> must have a non-broken C standard library.
>
>
>> 2. This adds a lot of header files. Each header file slows down
>> compilation, and standard library header files get included *a lot*. We may
>> not be able to avoid this, but we should think about the costs here.
>
>
> I created a .cpp file that includes all of the <*.h> headers and does
> nothing else (which should maximize the performance difference), and built
> it with and without this change. I could not measure any difference (the
> average compile time with this change was slightly lower, but that is
> almost certainly noise).
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

On Tue, Oct 6, 2015 at 4:16 PM, Sean Silva  wrote:

> On Tue, Oct 6, 2015 at 4:13 PM, Richard Smith 
> wrote:
>
>> On Tue, Oct 6, 2015 at 4:11 PM, Sean Silva  wrote:
>>
>>> +extern "C++" {
>>> +#include <__nullptr>
>>> +using std::nullptr_t;
>>> +}
>>>
>>> Does this even compile with modules?
>>>
>>
>> Yes. You're allowed to import a C++ module within an 'extern "C++"' block.
>>
>
> Why do we even need `extern "C++"` if we are already under `#ifdef
> __cplusplus`?
>

People like to include C++'s  headers inside 'extern "C"' blocks. We
shouldn't break that if we don't have to.


> -- Sean Silva
>
>
>>
>>
>>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 . 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.

 2) We probably don't want  to include  (for
 consistency with other headers), but  must provide a ::nullptr_t
 (which we don't want  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  doesn't provide it.

 If you prefer, I could make  include  to avoid the
 duplication of the max_align_t logic.

 This patch depends on patch 02.

 On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith 
 wrote:

> , an easy one. We guarantee a setjmp macro exists even if
> this header is somehow included from C (the C standard allows that, so 
> it's
> not worth checking for __cplusplus).
>
> On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
> wrote:
>
>> Split  out of . This is a big change, but the same
>> pattern as the prior ones.
>>
>> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
>> wrote:
>>
>>> Likewise for , , 
>>>
>>> On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith >> > wrote:
>>>
 Split  header out of 

 On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith <
 rich...@metafoo.co.uk> wrote:

> Next: factoring the definition of std::nullptr_t out into a
> separate file, so that  and  can both use it, 
> without
>  including  and without  providing a
> ::nullptr_t like  does.
>
> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier 
> wrote:
>
>> LGTM.
>>
>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith <
>> rich...@metafoo.co.uk> wrote:
>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier 
>> wrote:
>> >>
>> >> EricWF added a comment.
>> >>
>> >> I think thing change will help us close a number out
>> outstanding bugs. I
>> >> don't have any fundamental objections to this approach.
>> However the size of
>> >> this patch scares me.  I understand the changes are mostly
>> mechanical but
>> >> their size can hide things. For example has anybody noticed
>> that your
>> >> internal changes to `` are in this patch? It would be
>> nice to break
>> >> this down into more digestible pieces that could be quickly
>> spot checked.
>> >
>> >
>> > OK. First such patch is attached. It just removes the
>> macro-capturing
>> > wrapper functions.
>>
>
>

>>>
>>
>

 ___
 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

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

[Sean Silva via cfe-commits]

On Tue, Oct 6, 2015 at 4:13 PM, Richard Smith  wrote:

> On Tue, Oct 6, 2015 at 4:11 PM, Sean Silva  wrote:
>
>> +extern "C++" {
>> +#include <__nullptr>
>> +using std::nullptr_t;
>> +}
>>
>> Does this even compile with modules?
>>
>
> Yes. You're allowed to import a C++ module within an 'extern "C++"' block.
>

Why do we even need `extern "C++"` if we are already under `#ifdef
__cplusplus`?

-- Sean Silva


>
>
>> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> . 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.
>>>
>>> 2) We probably don't want  to include  (for
>>> consistency with other headers), but  must provide a ::nullptr_t
>>> (which we don't want  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  doesn't provide it.
>>>
>>> If you prefer, I could make  include  to avoid the
>>> duplication of the max_align_t logic.
>>>
>>> This patch depends on patch 02.
>>>
>>> On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith 
>>> wrote:
>>>
 , an easy one. We guarantee a setjmp macro exists even if
 this header is somehow included from C (the C standard allows that, so it's
 not worth checking for __cplusplus).

 On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
 wrote:

> Split  out of . This is a big change, but the same
> pattern as the prior ones.
>
> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
> wrote:
>
>> Likewise for , , 
>>
>> On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith 
>> wrote:
>>
>>> Split  header out of 
>>>
>>> On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith >> > wrote:
>>>
 Next: factoring the definition of std::nullptr_t out into a
 separate file, so that  and  can both use it, 
 without
  including  and without  providing a
 ::nullptr_t like  does.

 On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:

> LGTM.
>
> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith <
> rich...@metafoo.co.uk> wrote:
> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier 
> wrote:
> >>
> >> EricWF added a comment.
> >>
> >> I think thing change will help us close a number out
> outstanding bugs. I
> >> don't have any fundamental objections to this approach.
> However the size of
> >> this patch scares me.  I understand the changes are mostly
> mechanical but
> >> their size can hide things. For example has anybody noticed
> that your
> >> internal changes to `` are in this patch? It would be
> nice to break
> >> this down into more digestible pieces that could be quickly
> spot checked.
> >
> >
> > OK. First such patch is attached. It just removes the
> macro-capturing
> > wrapper functions.
>


>>>
>>
>

>>>
>>> ___
>>> 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

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

[Richard Smith via cfe-commits]

On Tue, Oct 6, 2015 at 4:11 PM, Sean Silva  wrote:

> +extern "C++" {
> +#include <__nullptr>
> +using std::nullptr_t;
> +}
>
> Does this even compile with modules?
>

Yes. You're allowed to import a C++ module within an 'extern "C++"' block.


> On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> . 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.
>>
>> 2) We probably don't want  to include  (for
>> consistency with other headers), but  must provide a ::nullptr_t
>> (which we don't want  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  doesn't provide it.
>>
>> If you prefer, I could make  include  to avoid the
>> duplication of the max_align_t logic.
>>
>> This patch depends on patch 02.
>>
>> On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith 
>> wrote:
>>
>>> , an easy one. We guarantee a setjmp macro exists even if this
>>> header is somehow included from C (the C standard allows that, so it's not
>>> worth checking for __cplusplus).
>>>
>>> On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
>>> wrote:
>>>
 Split  out of . This is a big change, but the same
 pattern as the prior ones.

 On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
 wrote:

> Likewise for , , 
>
> On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith 
> wrote:
>
>> Split  header out of 
>>
>> On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith 
>> wrote:
>>
>>> Next: factoring the definition of std::nullptr_t out into a separate
>>> file, so that  and  can both use it, without 
>>> 
>>> including  and without  providing a ::nullptr_t like
>>>  does.
>>>
>>> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>>>
 LGTM.

 On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith <
 rich...@metafoo.co.uk> wrote:
 > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier 
 wrote:
 >>
 >> EricWF added a comment.
 >>
 >> I think thing change will help us close a number out outstanding
 bugs. I
 >> don't have any fundamental objections to this approach.  However
 the size of
 >> this patch scares me.  I understand the changes are mostly
 mechanical but
 >> their size can hide things. For example has anybody noticed that
 your
 >> internal changes to `` are in this patch? It would be
 nice to break
 >> this down into more digestible pieces that could be quickly spot
 checked.
 >
 >
 > OK. First such patch is attached. It just removes the
 macro-capturing
 > wrapper functions.

>>>
>>>
>>
>

>>>
>>
>> ___
>> 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

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

[Richard Smith via cfe-commits]

: like , this provides only pieces if __need_FILE or
__need___FILE is defined
: likewise, but for __need_malloc_and_calloc
 and  are straightforward
 is tricky: C libraries that don't implement the right interface
provide non-const-correct functions like "wchar_t *wmemchr(const wchar_t*,
wchar_t, size_t)". We can't fix that, so we don't try.

On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith  wrote:

> . 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.
>
> 2) We probably don't want  to include  (for consistency
> with other headers), but  must provide a ::nullptr_t (which we
> don't want  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
>  doesn't provide it.
>
> If you prefer, I could make  include  to avoid the
> duplication of the max_align_t logic.
>
> This patch depends on patch 02.
>
> On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith 
> wrote:
>
>> , an easy one. We guarantee a setjmp macro exists even if this
>> header is somehow included from C (the C standard allows that, so it's not
>> worth checking for __cplusplus).
>>
>> On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
>> wrote:
>>
>>> Split  out of . This is a big change, but the same
>>> pattern as the prior ones.
>>>
>>> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
>>> wrote:
>>>
 Likewise for , , 

 On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith 
 wrote:

> Split  header out of 
>
> On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith 
> wrote:
>
>> Next: factoring the definition of std::nullptr_t out into a separate
>> file, so that  and  can both use it, without 
>> 
>> including  and without  providing a ::nullptr_t like
>>  does.
>>
>> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>>
>>> LGTM.
>>>
>>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>>> wrote:
>>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier 
>>> wrote:
>>> >>
>>> >> EricWF added a comment.
>>> >>
>>> >> I think thing change will help us close a number out outstanding
>>> bugs. I
>>> >> don't have any fundamental objections to this approach.  However
>>> the size of
>>> >> this patch scares me.  I understand the changes are mostly
>>> mechanical but
>>> >> their size can hide things. For example has anybody noticed that
>>> your
>>> >> internal changes to `` are in this patch? It would be nice
>>> to break
>>> >> this down into more digestible pieces that could be quickly spot
>>> checked.
>>> >
>>> >
>>> > OK. First such patch is attached. It just removes the
>>> macro-capturing
>>> > wrapper functions.
>>>
>>
>>
>

>>>
>>
>
diff --git include/cstdio include/cstdio
index 61985d6..50fdd34 100644
--- include/cstdio
+++ include/cstdio
@@ -103,16 +103,6 @@ void perror(const char* s);
 #pragma GCC system_header
 #endif
 
-// snprintf
-#if defined(_LIBCPP_MSVCRT)
-#include "support/win32/support.h"
-#endif
-
-#undef getc
-#undef putc
-#undef clearerr
-#undef feof
-#undef ferror
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 using ::FILE;
diff --git include/stdio.h include/stdio.h
new file mode 100644
index 000..bebe1fb
--- /dev/null
+++ include/stdio.h
@@ -0,0 +1,121 @@
+// -*- C++ -*-
+//=== stdio.h 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#if defined(__need_FILE) || defined(__need___FILE)
+#include_next 
+
+#elif !defined(_LIBCPP_STDIO_H)
+#define _LIBCPP_STDIO_H
+
+/*
+stdio.h synopsis
+
+Macros:
+
+BUFSIZ
+EOF
+FILENAME_MAX
+FOPEN_MAX
+L_tmpnam
+NULL
+SEEK_CUR
+SEEK_END
+SEEK_SET
+TMP_MAX
+_IOFBF
+_IOLBF
+_IONBF
+stderr
+stdin
+stdout
+
+Types:
+
+FILE
+fpos_t
+size_t
+
+int remove(const char* filename);
+int rename(const char* old, const char* new);
+FILE* tmpfile(void);
+char* tmpnam(char* s);
+int fclose(FILE* stream);
+int fflush(FILE* stream);
+FILE* fopen(const char* restrict filename, const char* restrict mode);
+FILE* freopen(const char* restrict filename, const char * restrict mode,
+  FILE * restrict stream);
+void setbuf(FILE* restrict stream, char* restrict buf);
+int setvbuf(FILE* restrict stream, char* restrict buf, int mode, size_t size);
+int fprintf(FIL

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

[Sean Silva via cfe-commits]

+extern "C++" {
+#include <__nullptr>
+using std::nullptr_t;
+}

Does this even compile with modules?


-- Sean Silva

On Tue, Oct 6, 2015 at 3:57 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> . 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.
>
> 2) We probably don't want  to include  (for consistency
> with other headers), but  must provide a ::nullptr_t (which we
> don't want  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
>  doesn't provide it.
>
> If you prefer, I could make  include  to avoid the
> duplication of the max_align_t logic.
>
> This patch depends on patch 02.
>
> On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith 
> wrote:
>
>> , an easy one. We guarantee a setjmp macro exists even if this
>> header is somehow included from C (the C standard allows that, so it's not
>> worth checking for __cplusplus).
>>
>> On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
>> wrote:
>>
>>> Split  out of . This is a big change, but the same
>>> pattern as the prior ones.
>>>
>>> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
>>> wrote:
>>>
 Likewise for , , 

 On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith 
 wrote:

> Split  header out of 
>
> On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith 
> wrote:
>
>> Next: factoring the definition of std::nullptr_t out into a separate
>> file, so that  and  can both use it, without 
>> 
>> including  and without  providing a ::nullptr_t like
>>  does.
>>
>> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>>
>>> LGTM.
>>>
>>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>>> wrote:
>>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier 
>>> wrote:
>>> >>
>>> >> EricWF added a comment.
>>> >>
>>> >> I think thing change will help us close a number out outstanding
>>> bugs. I
>>> >> don't have any fundamental objections to this approach.  However
>>> the size of
>>> >> this patch scares me.  I understand the changes are mostly
>>> mechanical but
>>> >> their size can hide things. For example has anybody noticed that
>>> your
>>> >> internal changes to `` are in this patch? It would be nice
>>> to break
>>> >> this down into more digestible pieces that could be quickly spot
>>> checked.
>>> >
>>> >
>>> > OK. First such patch is attached. It just removes the
>>> macro-capturing
>>> > wrapper functions.
>>>
>>
>>
>

>>>
>>
>
> ___
> 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

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

[Richard Smith via cfe-commits]

. 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.

2) We probably don't want  to include  (for consistency
with other headers), but  must provide a ::nullptr_t (which we
don't want  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
 doesn't provide it.

If you prefer, I could make  include  to avoid the
duplication of the max_align_t logic.

This patch depends on patch 02.

On Tue, Oct 6, 2015 at 3:42 PM, Richard Smith  wrote:

> , an easy one. We guarantee a setjmp macro exists even if this
> header is somehow included from C (the C standard allows that, so it's not
> worth checking for __cplusplus).
>
> On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith 
> wrote:
>
>> Split  out of . This is a big change, but the same pattern
>> as the prior ones.
>>
>> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
>> wrote:
>>
>>> Likewise for , , 
>>>
>>> On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith 
>>> wrote:
>>>
 Split  header out of 

 On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith 
 wrote:

> Next: factoring the definition of std::nullptr_t out into a separate
> file, so that  and  can both use it, without 
> including  and without  providing a ::nullptr_t like
>  does.
>
> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>
>> LGTM.
>>
>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>> wrote:
>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>> >>
>> >> EricWF added a comment.
>> >>
>> >> I think thing change will help us close a number out outstanding
>> bugs. I
>> >> don't have any fundamental objections to this approach.  However
>> the size of
>> >> this patch scares me.  I understand the changes are mostly
>> mechanical but
>> >> their size can hide things. For example has anybody noticed that
>> your
>> >> internal changes to `` are in this patch? It would be nice
>> to break
>> >> this down into more digestible pieces that could be quickly spot
>> checked.
>> >
>> >
>> > OK. First such patch is attached. It just removes the
>> macro-capturing
>> > wrapper functions.
>>
>
>

>>>
>>
>
diff --git include/cstddef include/cstddef
index 68f52c2..1210b91 100644
--- include/cstddef
+++ include/cstddef
@@ -34,10 +34,10 @@ Types:
 */
 
 #include <__config>
+// Don't include our own ; we don't want to declare ::nullptr_t.
+#include_next 
 #include <__nullptr>
 
-#include 
-
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
diff --git include/stddef.h include/stddef.h
new file mode 100644
index 000..6ffe582
--- /dev/null
+++ include/stddef.h
@@ -0,0 +1,56 @@
+// -*- C++ -*-
+//===--- stddef.h 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#if defined(__need_ptrdiff_t) || defined(__need_size_t) || \
+defined(__need_wchar_t) || defined(__need_NULL) || defined(__need_wint_t)
+#include_next 
+
+#elif !defined(_LIBCPP_STDDEF_H)
+#define _LIBCPP_STDDEF_H
+
+/*
+stddef.h synopsis
+
+Macros:
+
+offsetof(type,member-designator)
+NULL
+
+Types:
+
+ptrdiff_t
+size_t
+max_align_t
+nullptr_t
+
+*/
+
+#include <__config>
+#include_next 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef __cplusplus
+
+extern "C++" {
+#include <__nullptr>
+using std::nullptr_t;
+}
+
+// Re-use the compiler's  max_align_t where possible.
+#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T)
+typedef long double max_align_t;
+#endif
+
+#endif
+
+#endif  // _LIBCPP_STDDEF_H
diff --git test/std/depr/depr.c.headers/stddef_h.pass.cpp 
test/std/depr/depr.c.headers/stddef_h.pass.cpp
index 140c91b..c03c314 100644
--- test/std/depr/depr.c.headers/stddef_h.pass.cpp
+++ test/std/depr/depr.c.headers/stddef_h.pass.cpp
@@ -10,6 +10,7 @@
 // 
 
 #include 
+#include 
 #include 
 
 #ifndef NULL
@@ -22,6 +23,9 @@
 
 int main()
 {
+void *p = NULL;
+assert(!p);
+
 static_assert(sizeof(size_t) == sizeof(void*),
   "sizeof(size_t) == sizeof(void*)");
 static_assert(std::is_unsigned::value,
@@ -34,4 +38,22 @@ int main()
   "std::is_signed::value");
 static_assert(std::is_integral::value,
 

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

[Richard Smith via cfe-commits]

On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith  wrote:

> Next: factoring the definition of std::nullptr_t out into a separate file,
> so that  and  can both use it, without 
> including  and without  providing a ::nullptr_t like
>  does.
>

Sorry, missed a couple of the hunks for this patch.


> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>
>> LGTM.
>>
>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>> wrote:
>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>> >>
>> >> EricWF added a comment.
>> >>
>> >> I think thing change will help us close a number out outstanding bugs.
>> I
>> >> don't have any fundamental objections to this approach.  However the
>> size of
>> >> this patch scares me.  I understand the changes are mostly mechanical
>> but
>> >> their size can hide things. For example has anybody noticed that your
>> >> internal changes to `` are in this patch? It would be nice to
>> break
>> >> this down into more digestible pieces that could be quickly spot
>> checked.
>> >
>> >
>> > OK. First such patch is attached. It just removes the macro-capturing
>> > wrapper functions.
>>
>
>
commit 89b8a2875ac1cf084213ad47623cd92c4a087cc5
Author: Richard Smith 
Date:   Tue Oct 6 15:12:30 2015 -0700

Factor out std::nullptr_t into a separate file.

diff --git include/__nullptr include/__nullptr
new file mode 100644
index 000..95415a6
--- /dev/null
+++ include/__nullptr
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===--- __nullptr 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_NULLPTR
+#define _LIBCPP_NULLPTR
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef _LIBCPP_HAS_NO_NULLPTR
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+struct _LIBCPP_TYPE_VIS_ONLY nullptr_t
+{
+void* __lx;
+
+struct __nat {int __for_bool_;};
+
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t() : __lx(0) {}
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t(int __nat::*) : __lx(0) 
{}
+
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR operator int __nat::*() const 
{return 0;}
+
+template 
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR
+operator _Tp* () const {return 0;}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+operator _Tp _Up::* () const {return 0;}
+
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator==(nullptr_t, 
nullptr_t) {return true;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator!=(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<=(nullptr_t, 
nullptr_t) {return true;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>=(nullptr_t, 
nullptr_t) {return true;}
+};
+
+inline _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t __get_nullptr_t() 
{return nullptr_t(0);}
+
+#define nullptr _VSTD::__get_nullptr_t()
+
+_LIBCPP_END_NAMESPACE_STD
+
+#else  // _LIBCPP_HAS_NO_NULLPTR
+
+namespace std
+{
+typedef decltype(nullptr) nullptr_t;
+}
+
+#endif  // _LIBCPP_HAS_NO_NULLPTR
+
+#endif  // _LIBCPP_NULLPTR
diff --git include/cstddef include/cstddef
index c3ca64a..68f52c2 100644
--- include/cstddef
+++ include/cstddef
@@ -34,6 +34,7 @@ Types:
 */
 
 #include <__config>
+#include <__nullptr>
 
 #include 
 
@@ -53,50 +54,6 @@ using ::max_align_t;
 typedef long double max_align_t;
 #endif
 
-#ifdef _LIBCPP_HAS_NO_NULLPTR
-
-struct _LIBCPP_TYPE_VIS_ONLY nullptr_t
-{
-void* __lx;
-
-struct __nat {int __for_bool_;};
-
-_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t() : __lx(0) {}
-_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t(int __nat::*) : __lx(0) 
{}
-
-_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR operator int __nat::*() const 
{return 0;}
-
-template 
-_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR
-operator _Tp* () const {return 0;}
-
-template 
-_LIBCPP_ALWAYS_INLINE
-operator _Tp _Up::* () const {return 0;}
-
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator==(nullptr_t, 
nullptr_t) {return true;}
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator!=(nullptr_t, 
nullptr_t) {return false;}
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<(nullptr_t, 
nullptr_t) {return false;}
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<=(nullptr_t, 
nullptr_t) {return true;}
-friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>(nullptr_t, 
nullptr_t) {return false;}
-frie

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

[Richard Smith via cfe-commits]

, an easy one. We guarantee a setjmp macro exists even if this
header is somehow included from C (the C standard allows that, so it's not
worth checking for __cplusplus).

On Tue, Oct 6, 2015 at 3:36 PM, Richard Smith  wrote:

> Split  out of . This is a big change, but the same pattern
> as the prior ones.
>
> On Tue, Oct 6, 2015 at 3:28 PM, Richard Smith 
> wrote:
>
>> Likewise for , , 
>>
>> On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith 
>> wrote:
>>
>>> Split  header out of 
>>>
>>> On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith 
>>> wrote:
>>>
 Next: factoring the definition of std::nullptr_t out into a separate
 file, so that  and  can both use it, without 
 including  and without  providing a ::nullptr_t like
  does.

 On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:

> LGTM.
>
> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
> wrote:
> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
> >>
> >> EricWF added a comment.
> >>
> >> I think thing change will help us close a number out outstanding
> bugs. I
> >> don't have any fundamental objections to this approach.  However
> the size of
> >> this patch scares me.  I understand the changes are mostly
> mechanical but
> >> their size can hide things. For example has anybody noticed that
> your
> >> internal changes to `` are in this patch? It would be nice
> to break
> >> this down into more digestible pieces that could be quickly spot
> checked.
> >
> >
> > OK. First such patch is attached. It just removes the macro-capturing
> > wrapper functions.
>


>>>
>>
>
diff --git include/csetjmp include/csetjmp
index d0b2c07..58a9c73 100644
--- include/csetjmp
+++ include/csetjmp
@@ -38,10 +38,6 @@ void longjmp(jmp_buf env, int val);
 #pragma GCC system_header
 #endif
 
-#ifndef setjmp
-#define setjmp(env) setjmp(env)
-#endif
-
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 using ::jmp_buf;
diff --git include/setjmp.h include/setjmp.h
new file mode 100644
index 000..d429c86
--- /dev/null
+++ include/setjmp.h
@@ -0,0 +1,40 @@
+// -*- C++ -*-
+//===--- setjmp.h 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_SETJMP_H
+#define _LIBCPP_SETJMP_H
+
+/*
+setjmp.h synopsis
+
+Macros:
+
+setjmp
+
+Types:
+
+jmp_buf
+
+void longjmp(jmp_buf env, int val);
+
+*/
+
+#include <__config>
+#include_next 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifndef setjmp
+#define setjmp(env) setjmp(env)
+#endif
+
+#endif  // _LIBCPP_SETJMP_H
diff --git test/std/depr/depr.c.headers/setjmp_h.pass.cpp 
test/std/depr/depr.c.headers/setjmp_h.pass.cpp
index 36f4253..9bc35b7 100644
--- test/std/depr/depr.c.headers/setjmp_h.pass.cpp
+++ test/std/depr/depr.c.headers/setjmp_h.pass.cpp
@@ -12,6 +12,10 @@
 #include 
 #include 
 
+#ifndef setjmp
+#error setjmp not defined
+#endif
+
 int main()
 {
 jmp_buf jb;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

Likewise for , , 

On Tue, Oct 6, 2015 at 3:20 PM, Richard Smith  wrote:

> Split  header out of 
>
> On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith 
> wrote:
>
>> Next: factoring the definition of std::nullptr_t out into a separate
>> file, so that  and  can both use it, without 
>> including  and without  providing a ::nullptr_t like
>>  does.
>>
>> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>>
>>> LGTM.
>>>
>>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>>> wrote:
>>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>>> >>
>>> >> EricWF added a comment.
>>> >>
>>> >> I think thing change will help us close a number out outstanding
>>> bugs. I
>>> >> don't have any fundamental objections to this approach.  However the
>>> size of
>>> >> this patch scares me.  I understand the changes are mostly mechanical
>>> but
>>> >> their size can hide things. For example has anybody noticed that your
>>> >> internal changes to `` are in this patch? It would be nice to
>>> break
>>> >> this down into more digestible pieces that could be quickly spot
>>> checked.
>>> >
>>> >
>>> > OK. First such patch is attached. It just removes the macro-capturing
>>> > wrapper functions.
>>>
>>
>>
>
diff --git include/cerrno include/cerrno
index 9804e4e..bab13b8 100644
--- include/cerrno
+++ include/cerrno
@@ -30,364 +30,4 @@ Macros:
 #pragma GCC system_header
 #endif
 
-#if !defined(EOWNERDEAD) || !defined(ENOTRECOVERABLE)
-
-#ifdef ELAST
-
-const int __elast1 = ELAST+1;
-const int __elast2 = ELAST+2;
-
-#else
-
-const int __elast1 = 104;
-const int __elast2 = 105;
-
-#endif
-
-#ifdef ENOTRECOVERABLE
-
-#define EOWNERDEAD __elast1
-
-#ifdef ELAST
-#undef ELAST
-#define ELAST EOWNERDEAD
-#endif
-
-#elif defined(EOWNERDEAD)
-
-#define ENOTRECOVERABLE __elast1
-#ifdef ELAST
-#undef ELAST
-#define ELAST ENOTRECOVERABLE
-#endif
-
-#else  // defined(EOWNERDEAD)
-
-#define EOWNERDEAD __elast1
-#define ENOTRECOVERABLE __elast2
-#ifdef ELAST
-#undef ELAST
-#define ELAST ENOTRECOVERABLE
-#endif
-
-#endif  // defined(EOWNERDEAD)
-
-#endif  // !defined(EOWNERDEAD) || !defined(ENOTRECOVERABLE)
-
-//  supply errno values likely to be missing, particularly on Windows
-
-#ifndef EAFNOSUPPORT
-#define EAFNOSUPPORT 9901
-#endif
-
-#ifndef EADDRINUSE
-#define EADDRINUSE 9902
-#endif
-
-#ifndef EADDRNOTAVAIL
-#define EADDRNOTAVAIL 9903
-#endif
-
-#ifndef EISCONN
-#define EISCONN 9904
-#endif
-
-#ifndef EBADMSG
-#define EBADMSG 9905
-#endif
-
-#ifndef ECONNABORTED
-#define ECONNABORTED 9906
-#endif
-
-#ifndef EALREADY
-#define EALREADY 9907
-#endif
-
-#ifndef ECONNREFUSED
-#define ECONNREFUSED 9908
-#endif
-
-#ifndef ECONNRESET
-#define ECONNRESET 9909
-#endif
-
-#ifndef EDESTADDRREQ
-#define EDESTADDRREQ 9910
-#endif
-
-#ifndef EHOSTUNREACH
-#define EHOSTUNREACH 9911
-#endif
-
-#ifndef EIDRM
-#define EIDRM 9912
-#endif
-
-#ifndef EMSGSIZE
-#define EMSGSIZE 9913
-#endif
-
-#ifndef ENETDOWN
-#define ENETDOWN 9914
-#endif
-
-#ifndef ENETRESET
-#define ENETRESET 9915
-#endif
-
-#ifndef ENETUNREACH
-#define ENETUNREACH 9916
-#endif
-
-#ifndef ENOBUFS
-#define ENOBUFS 9917
-#endif
-
-#ifndef ENOLINK
-#define ENOLINK 9918
-#endif
-
-#ifndef ENODATA
-#define ENODATA 9919
-#endif
-
-#ifndef ENOMSG
-#define ENOMSG 9920
-#endif
-
-#ifndef ENOPROTOOPT
-#define ENOPROTOOPT 9921
-#endif
-
-#ifndef ENOSR
-#define ENOSR 9922
-#endif
-
-#ifndef ENOTSOCK
-#define ENOTSOCK 9923
-#endif
-
-#ifndef ENOSTR
-#define ENOSTR 9924
-#endif
-
-#ifndef ENOTCONN
-#define ENOTCONN 9925
-#endif
-
-#ifndef ENOTSUP
-#define ENOTSUP 9926
-#endif
-
-#ifndef ECANCELED
-#define ECANCELED 9927
-#endif
-
-#ifndef EINPROGRESS
-#define EINPROGRESS 9928
-#endif
-
-#ifndef EOPNOTSUPP
-#define EOPNOTSUPP 9929
-#endif
-
-#ifndef EWOULDBLOCK
-#define EWOULDBLOCK 9930
-#endif
-
-#ifndef EOWNERDEAD
-#define EOWNERDEAD  9931
-#endif
-
-#ifndef EPROTO
-#define EPROTO 9932
-#endif
-
-#ifndef EPROTONOSUPPORT
-#define EPROTONOSUPPORT 9933
-#endif
-
-#ifndef ENOTRECOVERABLE
-#define ENOTRECOVERABLE 9934
-#endif
-
-#ifndef ETIME
-#define ETIME 9935
-#endif
-
-#ifndef ETXTBSY
-#define ETXTBSY 9936
-#endif
-
-#ifndef ETIMEDOUT
-#define ETIMEDOUT 9938
-#endif
-
-#ifndef ELOOP
-#define ELOOP 9939
-#endif
-
-#ifndef EOVERFLOW
-#define EOVERFLOW 9940
-#endif
-
-#ifndef EPROTOTYPE
-#define EPROTOTYPE 9941
-#endif
-
-#ifndef ENOSYS
-#define ENOSYS 9942
-#endif
-
-#ifndef EINVAL
-#define EINVAL 9943
-#endif
-
-#ifndef ERANGE
-#define ERANGE 9944
-#endif
-
-#ifndef EILSEQ
-#define EILSEQ 9945
-#endif
-
-//  Windows Mobile doesn't appear to define these:
-
-#ifndef E2BIG
-#define E2BIG 9946
-#endif
-
-#ifndef EDOM
-#define EDOM 9947
-#endif
-
-#ifndef EFAULT
-#define EFAULT 9948
-#endif
-
-#ifndef EBADF
-#define EBADF 9949
-#endif
-
-#ifndef EPIPE
-#define EPIPE 9950
-#endif
-
-#ifndef EXDEV
-#define EXDEV 9951
-#endif
-
-#ifndef EBUSY
-#define EBUSY 9952
-#endif
-
-#ifndef ENOTEMPTY
-#define ENOTEMPTY 9953
-#endif
-
-#ifndef ENOEXEC
-#define ENOEXEC 9954
-#endif
-
-#ifndef EEXIST
-#define EEXIS

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

[Richard Smith via cfe-commits]

Split  header out of 

On Tue, Oct 6, 2015 at 3:07 PM, Richard Smith  wrote:

> Next: factoring the definition of std::nullptr_t out into a separate file,
> so that  and  can both use it, without 
> including  and without  providing a ::nullptr_t like
>  does.
>
> On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:
>
>> LGTM.
>>
>> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
>> wrote:
>> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>> >>
>> >> EricWF added a comment.
>> >>
>> >> I think thing change will help us close a number out outstanding bugs.
>> I
>> >> don't have any fundamental objections to this approach.  However the
>> size of
>> >> this patch scares me.  I understand the changes are mostly mechanical
>> but
>> >> their size can hide things. For example has anybody noticed that your
>> >> internal changes to `` are in this patch? It would be nice to
>> break
>> >> this down into more digestible pieces that could be quickly spot
>> checked.
>> >
>> >
>> > OK. First such patch is attached. It just removes the macro-capturing
>> > wrapper functions.
>>
>
>
diff --git include/cctype include/cctype
index db16343..a68c2a0 100644
--- include/cctype
+++ include/cctype
@@ -37,10 +37,6 @@ int toupper(int c);
 
 #include <__config>
 #include 
-#if defined(_LIBCPP_MSVCRT)
-#include "support/win32/support.h"
-#include "support/win32/locale_win32.h"
-#endif // _LIBCPP_MSVCRT
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
@@ -48,33 +44,19 @@ int toupper(int c);
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#undef isalnum
 using ::isalnum;
-#undef isalpha
 using ::isalpha;
-#undef isblank
 using ::isblank;
-#undef iscntrl
 using ::iscntrl;
-#undef isdigit
 using ::isdigit;
-#undef isgraph
 using ::isgraph;
-#undef islower
 using ::islower;
-#undef isprint
 using ::isprint;
-#undef ispunct
 using ::ispunct;
-#undef isspace
 using ::isspace;
-#undef isupper
 using ::isupper;
-#undef isxdigit
 using ::isxdigit;
-#undef tolower
 using ::tolower;
-#undef toupper
 using ::toupper;
 
 _LIBCPP_END_NAMESPACE_STD
diff --git include/ctype.h include/ctype.h
new file mode 100644
index 000..63f0b29
--- /dev/null
+++ include/ctype.h
@@ -0,0 +1,68 @@
+// -*- C++ -*-
+//=== ctype.h 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CTYPE_H
+#define _LIBCPP_CTYPE_H
+
+/*
+ctype.h synopsis
+
+int isalnum(int c);
+int isalpha(int c);
+int isblank(int c);  // C99
+int iscntrl(int c);
+int isdigit(int c);
+int isgraph(int c);
+int islower(int c);
+int isprint(int c);
+int ispunct(int c);
+int isspace(int c);
+int isupper(int c);
+int isxdigit(int c);
+int tolower(int c);
+int toupper(int c);
+*/
+
+#include <__config>
+#include_next 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef __cplusplus
+
+#if defined(_LIBCPP_MSVCRT)
+// We support including .h headers inside 'extern "C"' contexts, so switch
+// back to C++ linkage before including these C++ headers.
+extern "C++" {
+  #include "support/win32/support.h"
+  #include "support/win32/locale_win32.h"
+}
+#endif // _LIBCPP_MSVCRT
+
+#undef isalnum
+#undef isalpha
+#undef isblank
+#undef iscntrl
+#undef isdigit
+#undef isgraph
+#undef islower
+#undef isprint
+#undef ispunct
+#undef isspace
+#undef isupper
+#undef isxdigit
+#undef tolower
+#undef toupper
+
+#endif
+
+#endif  // _LIBCPP_CTYPE_H
diff --git test/std/strings/c.strings/cctype.pass.cpp 
test/std/strings/c.strings/cctype.pass.cpp
index 867338f..027fbcd 100644
--- test/std/strings/c.strings/cctype.pass.cpp
+++ test/std/strings/c.strings/cctype.pass.cpp
@@ -86,18 +86,18 @@ int main()
 static_assert((std::is_same::value), "");
 static_assert((std::is_same::value), "");
 
-assert(isalnum('a'));
-assert(isalpha('a'));
-assert(isblank(' '));
-assert(!iscntrl(' '));
-assert(!isdigit('a'));
-assert(isgraph('a'));
-assert(islower('a'));
-assert(isprint('a'));
-assert(!ispunct('a'));
-assert(!isspace('a'));
-assert(!isupper('a'));
-assert(isxdigit('a'));
-assert(tolower('A') == 'a');
-assert(toupper('a') == 'A');
+assert(std::isalnum('a'));
+assert(std::isalpha('a'));
+assert(std::isblank(' '));
+assert(!std::iscntrl(' '));
+assert(!std::isdigit('a'));
+assert(std::isgraph('a'));
+assert(std::islower('a'));
+assert(std::isprint('a'));
+assert(!std::ispunct('a'));
+assert(!std::isspace('a'));
+assert(!std::isupper('a'));
+assert(std::isxdigit('a'));
+assert(std::tolower('A') == 'a');
+assert(std::toupper('a') == 'A');
 }
___
cfe-commits mailing list
cfe-commits@lists.ll

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

[Richard Smith via cfe-commits]

Next: factoring the definition of std::nullptr_t out into a separate file,
so that  and  can both use it, without 
including  and without  providing a ::nullptr_t like
 does.

On Tue, Oct 6, 2015 at 3:02 PM, Eric Fiselier  wrote:

> LGTM.
>
> On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith 
> wrote:
> > On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
> >>
> >> EricWF added a comment.
> >>
> >> I think thing change will help us close a number out outstanding bugs. I
> >> don't have any fundamental objections to this approach.  However the
> size of
> >> this patch scares me.  I understand the changes are mostly mechanical
> but
> >> their size can hide things. For example has anybody noticed that your
> >> internal changes to `` are in this patch? It would be nice to
> break
> >> this down into more digestible pieces that could be quickly spot
> checked.
> >
> >
> > OK. First such patch is attached. It just removes the macro-capturing
> > wrapper functions.
>
Index: __nullptr
===
--- __nullptr   (revision 0)
+++ __nullptr   (working copy)
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===--- __nullptr 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_NULLPTR
+#define _LIBCPP_NULLPTR
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#ifdef _LIBCPP_HAS_NO_NULLPTR
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+struct _LIBCPP_TYPE_VIS_ONLY nullptr_t
+{
+void* __lx;
+
+struct __nat {int __for_bool_;};
+
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t() : __lx(0) {}
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t(int __nat::*) : __lx(0) 
{}
+
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR operator int __nat::*() const 
{return 0;}
+
+template 
+_LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR
+operator _Tp* () const {return 0;}
+
+template 
+_LIBCPP_ALWAYS_INLINE
+operator _Tp _Up::* () const {return 0;}
+
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator==(nullptr_t, 
nullptr_t) {return true;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator!=(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator<=(nullptr_t, 
nullptr_t) {return true;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>(nullptr_t, 
nullptr_t) {return false;}
+friend _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR bool operator>=(nullptr_t, 
nullptr_t) {return true;}
+};
+
+inline _LIBCPP_ALWAYS_INLINE _LIBCPP_CONSTEXPR nullptr_t __get_nullptr_t() 
{return nullptr_t(0);}
+
+#define nullptr _VSTD::__get_nullptr_t()
+
+_LIBCPP_END_NAMESPACE_STD
+
+#else  // _LIBCPP_HAS_NO_NULLPTR
+
+namespace std
+{
+typedef decltype(nullptr) nullptr_t;
+}
+
+#endif  // _LIBCPP_HAS_NO_NULLPTR
+
+#endif  // _LIBCPP_NULLPTR
Index: cstddef
===
--- cstddef (revision 249368)
+++ cstddef (working copy)
@@ -34,6 +34,7 @@
 */
 
 #include <__config>
+#include <__nullptr>
 
 #include 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eric Fiselier via cfe-commits]

LGTM.

On Tue, Oct 6, 2015 at 3:58 PM, Richard Smith  wrote:
> On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:
>>
>> EricWF added a comment.
>>
>> I think thing change will help us close a number out outstanding bugs. I
>> don't have any fundamental objections to this approach.  However the size of
>> this patch scares me.  I understand the changes are mostly mechanical but
>> their size can hide things. For example has anybody noticed that your
>> internal changes to `` are in this patch? It would be nice to break
>> this down into more digestible pieces that could be quickly spot checked.
>
>
> OK. First such patch is attached. It just removes the macro-capturing
> wrapper functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

On Mon, Oct 5, 2015 at 7:10 PM, Eric Fiselier  wrote:

> EricWF added a comment.
>
> I think thing change will help us close a number out outstanding bugs. I
> don't have any fundamental objections to this approach.  However the size
> of this patch scares me.  I understand the changes are mostly mechanical
> but their size can hide things. For example has anybody noticed that your
> internal changes to `` are in this patch? It would be nice to break
> this down into more digestible pieces that could be quickly spot checked.


OK. First such patch is attached. It just removes the macro-capturing
wrapper functions.
Index: cctype
===
--- cctype  (revision 249368)
+++ cctype  (working copy)
@@ -48,117 +48,34 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#ifdef isalnum
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isalnum(int __c) {return 
isalnum(__c);}
 #undef isalnum
-inline _LIBCPP_INLINE_VISIBILITY int isalnum(int __c) {return 
__libcpp_isalnum(__c);}
-#else  // isalnum
 using ::isalnum;
-#endif  // isalnum
-
-#ifdef isalpha
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isalpha(int __c) {return 
isalpha(__c);}
 #undef isalpha
-inline _LIBCPP_INLINE_VISIBILITY int isalpha(int __c) {return 
__libcpp_isalpha(__c);}
-#else  // isalpha
 using ::isalpha;
-#endif  // isalpha
-
-#ifdef isblank
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isblank(int __c) {return 
isblank(__c);}
 #undef isblank
-inline _LIBCPP_INLINE_VISIBILITY int isblank(int __c) {return 
__libcpp_isblank(__c);}
-#else  // isblank
 using ::isblank;
-#endif  // isblank
-
-#ifdef iscntrl
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_iscntrl(int __c) {return 
iscntrl(__c);}
 #undef iscntrl
-inline _LIBCPP_INLINE_VISIBILITY int iscntrl(int __c) {return 
__libcpp_iscntrl(__c);}
-#else  // iscntrl
 using ::iscntrl;
-#endif  // iscntrl
-
-#ifdef isdigit
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isdigit(int __c) {return 
isdigit(__c);}
 #undef isdigit
-inline _LIBCPP_INLINE_VISIBILITY int isdigit(int __c) {return 
__libcpp_isdigit(__c);}
-#else  // isdigit
 using ::isdigit;
-#endif  // isdigit
-
-#ifdef isgraph
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isgraph(int __c) {return 
isgraph(__c);}
 #undef isgraph
-inline _LIBCPP_INLINE_VISIBILITY int isgraph(int __c) {return 
__libcpp_isgraph(__c);}
-#else  // isgraph
 using ::isgraph;
-#endif  // isgraph
-
-#ifdef islower
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_islower(int __c) {return 
islower(__c);}
 #undef islower
-inline _LIBCPP_INLINE_VISIBILITY int islower(int __c) {return 
__libcpp_islower(__c);}
-#else  // islower
 using ::islower;
-#endif  // islower
-
-#ifdef isprint
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isprint(int __c) {return 
isprint(__c);}
 #undef isprint
-inline _LIBCPP_INLINE_VISIBILITY int isprint(int __c) {return 
__libcpp_isprint(__c);}
-#else  // isprint
 using ::isprint;
-#endif  // isprint
-
-#ifdef ispunct
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_ispunct(int __c) {return 
ispunct(__c);}
 #undef ispunct
-inline _LIBCPP_INLINE_VISIBILITY int ispunct(int __c) {return 
__libcpp_ispunct(__c);}
-#else  // ispunct
 using ::ispunct;
-#endif  // ispunct
-
-#ifdef isspace
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isspace(int __c) {return 
isspace(__c);}
 #undef isspace
-inline _LIBCPP_INLINE_VISIBILITY int isspace(int __c) {return 
__libcpp_isspace(__c);}
-#else  // isspace
 using ::isspace;
-#endif  // isspace
-
-#ifdef isupper
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isupper(int __c) {return 
isupper(__c);}
 #undef isupper
-inline _LIBCPP_INLINE_VISIBILITY int isupper(int __c) {return 
__libcpp_isupper(__c);}
-#else  // isupper
 using ::isupper;
-#endif  // isupper
-
-#ifdef isxdigit
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_isxdigit(int __c) {return 
isxdigit(__c);}
 #undef isxdigit
-inline _LIBCPP_INLINE_VISIBILITY int isxdigit(int __c) {return 
__libcpp_isxdigit(__c);}
-#else  // isxdigit
 using ::isxdigit;
-#endif  // isxdigit
-
-#ifdef tolower
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_tolower(int __c) {return 
tolower(__c);}
 #undef tolower
-inline _LIBCPP_INLINE_VISIBILITY int tolower(int __c) {return 
__libcpp_tolower(__c);}
-#else  // tolower
 using ::tolower;
-#endif  // tolower
-
-#ifdef toupper
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_toupper(int __c) {return 
toupper(__c);}
 #undef toupper
-inline _LIBCPP_INLINE_VISIBILITY int toupper(int __c) {return 
__libcpp_toupper(__c);}
-#else  // toupper
 using ::toupper;
-#endif  // toupper
 
 _LIBCPP_END_NAMESPACE_STD
 
Index: cstdio
===
--- cstdio  (revision 249368)
+++ cstdio  (working copy)
@@ -108,36 +108,11 @@
 #include "support/win32/support.h"
 #endif
 
-#ifdef getc
-inline _LIBCPP_INLINE_VISIBILITY int __libcpp_getc(FILE* __stream) {return 
getc(__stream);}
 #undef getc
-inline _LIBCPP_INLINE_VISIBILITY int getc(FILE* __stream) {retur

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

[Eric Fiselier via cfe-commits]

EricWF added a comment.

I think thing change will help us close a number out outstanding bugs. I  don't 
have any fundamental objections to this approach.  However the size of this 
patch scares me.  I understand the changes are mostly mechanical but their size 
can hide things. For example has anybody noticed that your internal changes to 
`` are in this patch? It would be nice to break this down into more 
digestible pieces that could be quickly spot checked.


Repository:
  rL LLVM

http://reviews.llvm.org/D12747



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Chandler Carruth via cfe-commits]

Marshall, I think Richard has responded to your concerns. Anything left?
This is blocking some things on our end.

On Wed, Sep 16, 2015 at 2:04 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Mon, Sep 14, 2015 at 7:07 AM, Marshall Clow 
> wrote:
>
>> mclow.lists added a comment.
>>
>> I have two concerns about this patch (w/o commenting on the actual code).
>>
>> 1. Until very recently, I was under the impression that C libraries
>> _either_ defined a macro, or had a function. I was quite surprised to find
>> that glibc did both.
>
>
> Yes, this is required by the C standard. C11 7.1.4/1 says:
>
> "Any function declared in a header may be additionally implemented as a
> function-like macro defined in the header [...]. Any macro definition of a
> function can be suppressed locally by enclosing the name of the function in
> parentheses, because the name is then not followed by the left parenthesis
> that indicates expansion of a macro function name. For the same syntactic
> reason, it is permitted to take the address of a library function even if
> it is also defined as a macro. [Footnote: This means that an implementation
> shall provide an actual function for each library function, even if it also
> provides a macro for that function.]"
>
> Have you checked other C libraries (Apple, FreeBSD, Android, Windows) to
>> see if they also define both?
>
>
> No, but libstdc++ does the same #undef thing, so any platform it supports
> must have a non-broken C standard library.
>
>
>> 2. This adds a lot of header files. Each header file slows down
>> compilation, and standard library header files get included *a lot*. We may
>> not be able to avoid this, but we should think about the costs here.
>
>
> I created a .cpp file that includes all of the <*.h> headers and does
> nothing else (which should maximize the performance difference), and built
> it with and without this change. I could not measure any difference (the
> average compile time with this change was slightly lower, but that is
> almost certainly noise).
> ___
> 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

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

[Richard Smith via cfe-commits]

On Mon, Sep 14, 2015 at 7:07 AM, Marshall Clow 
wrote:

> mclow.lists added a comment.
>
> I have two concerns about this patch (w/o commenting on the actual code).
>
> 1. Until very recently, I was under the impression that C libraries
> _either_ defined a macro, or had a function. I was quite surprised to find
> that glibc did both.


Yes, this is required by the C standard. C11 7.1.4/1 says:

"Any function declared in a header may be additionally implemented as a
function-like macro defined in the header [...]. Any macro definition of a
function can be suppressed locally by enclosing the name of the function in
parentheses, because the name is then not followed by the left parenthesis
that indicates expansion of a macro function name. For the same syntactic
reason, it is permitted to take the address of a library function even if
it is also defined as a macro. [Footnote: This means that an implementation
shall provide an actual function for each library function, even if it also
provides a macro for that function.]"

Have you checked other C libraries (Apple, FreeBSD, Android, Windows) to
> see if they also define both?


No, but libstdc++ does the same #undef thing, so any platform it supports
must have a non-broken C standard library.


> 2. This adds a lot of header files. Each header file slows down
> compilation, and standard library header files get included *a lot*. We may
> not be able to avoid this, but we should think about the costs here.


I created a .cpp file that includes all of the <*.h> headers and does
nothing else (which should maximize the performance difference), and built
it with and without this change. I could not measure any difference (the
average compile time with this change was slightly lower, but that is
almost certainly noise).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Marshall Clow via cfe-commits]

mclow.lists added a comment.

I have two concerns about this patch (w/o commenting on the actual code).

1. Until very recently, I was under the impression that C libraries _either_ 
defined a macro, or had a function. I was quite surprised to find that glibc 
did both. Have you checked other C libraries (Apple, FreeBSD, Android, Windows) 
to see if they also define both?  The purpose of this code:



  #ifdef getc
  inline _LIBCPP_INLINE_VISIBILITY int __libcpp_getc(FILE* __stream) {return 
getc(__stream);}
  #undef getc
  inline _LIBCPP_INLINE_VISIBILITY int getc(FILE* __stream) {return 
__libcpp_getc(__stream);}
  #endif  // getc

is to (a) remove the macro `getc`, since we are required to define `std::getc` 
and (b) to provide a global function with the same functionality that we can 
hoist into namespace `std`.

2. This adds a lot of header files. Each header file slows down compilation, 
and standard library header files get included *a lot*. We may not be able to 
avoid this, but we should think about the costs here.


Repository:
  rL LLVM

http://reviews.llvm.org/D12747



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

rsmith added a comment.

Each of the foo.h files added here was svn cp'd from the corresponding cfoo 
file. The listed diffs are against the base file. Likewise, __nullptr was 
copied from cstddef.

(Please disregard the changes to lib/buildit.)


Repository:
  rL LLVM

http://reviews.llvm.org/D12747



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits