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

2015-10-14 Thread 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

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

2015-10-09 Thread 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++

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

2015-10-09 Thread 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

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

2015-10-09 Thread 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 >

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

2015-10-09 Thread 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

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

2015-10-08 Thread 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

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

2015-10-08 Thread 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

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

2015-10-08 Thread 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,

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

2015-10-08 Thread 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: >>>

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

2015-10-08 Thread 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

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

2015-10-08 Thread 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

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

2015-10-08 Thread 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: >

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

2015-10-08 Thread 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

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

2015-10-08 Thread 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

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

2015-10-08 Thread 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

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

2015-10-07 Thread 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

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

2015-10-06 Thread 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,

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

2015-10-06 Thread 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

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

2015-10-06 Thread 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

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

2015-10-06 Thread 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

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

2015-10-06 Thread 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

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

2015-10-06 Thread 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

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

2015-10-06 Thread 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

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

2015-10-06 Thread 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

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

2015-10-06 Thread 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

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

2015-10-05 Thread 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: >

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

2015-10-05 Thread 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

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

2015-09-16 Thread 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

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

2015-09-10 Thread 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