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
_
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
@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
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
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
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
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 attac
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 Smit
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 detai
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
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
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/d
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.
>
>
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 librar
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_wc
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 isXX
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.
>
> #in
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
> h
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
___
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 fun
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 p
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 actu
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?
>>>
>
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
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 cf
: 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, s
+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
. 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
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
, 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 pr
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
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, Er
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:
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 s
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 m
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 an
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
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 sur
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 lib
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/D127
40 matches
Mail list logo
