subject:"\[libcxx\] r249798 \- Split out of ."

Re: [libcxx] r249798 - Split out of .

2016-01-26 Thread Hans Wennborg via cfe-commits

Eric, Marshall: has there been any progress here?

On Wed, Jan 20, 2016 at 10:29 AM, Hans Wennborg  wrote:
> /sub
>
> On Wed, Jan 20, 2016 at 4:45 AM, Nico Weber via cfe-commits
>  wrote:
>> Eric, Marshall: another ping. This should be fixed on the 3.8 branch, so it
>> needs to be resolved soon.
>>
>> On Jan 5, 2016 5:25 PM, "Nico Weber"  wrote:
>>>
>>> On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith 
>>> wrote:

 On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber  wrote:
>
> One problem with this patch: stdio.h can be used in .c files, and when
> building .c files with -gnu99 -pedantic,


 Do you mean -std=gnu89?

>
> clang will complain about // comments. Not only does this stdio.h have
> // comments, it also pulls in some libc++ headers (__config) that have //
> comments as well. I suppose all the comments in header files pulled in by 
> C
> headers need to become /* */ comments?


 I suppose so too. Your configuration is probably somewhat broken if
 libc++'s headers are in your include path while building C code, but it
 doesn't seem unreasonable to properly support that mode, and my changes 
 were
 already trying to do so.

 Eric, Marshall, what do you think about using only /*...*/-style comments
 in these headers, to handle the case where libc++ is somehow in the include
 path for a C89 compilation?
>>>
>>>
>>> Eric, Marshall: Ping ^
>>>


>
> On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits
>  wrote:
>>
>> On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier  wrote:
>>>
>>> This change LGTM. Let's hold off on the using "_Static_assert" until
>>> we understand how that would work with "-pedantic" when the macro is
>>> expanded in user code.
>>
>>
>> Committed as r250247, thanks.
>>
>>>
>>> /Eric
>>>
>>> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith 
>>> wrote:

 On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits
  wrote:
>
> I would rather not do this if possible but I understand why we need
> to do it.
>
> Richard is there a cost associated with the 'extern "C++"'
> construct? or by forcing the compiler to switch modes in general?


 Not a significant one compared to the cost of the code wrapped in the
 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only 
 applies
 in C++98; we could further reduce the set of cases when this happens by
 using _Static_assert when available instead of this static_assert
 emulation.)

>
> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith
>  wrote:
>>
>> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits
>>  wrote:
>>>
>>> Hi Richard
>>>
>>> Your splitting seems causing problem when using extern "C". Here
>>> is a test case:
>>>
>>> $ cat test.cpp
>>> #ifdef __cplusplus
>>> extern "C" {
>>> #endif
>>> #include 
>>> #ifdef __cplusplus
>>> }
>>> #endif
>>>
>>> Error:
>>> clang -fsyntax-only test.cpp
>>> In file included from test.cpp:4:
>>> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
>>> /usr/bin/../include/c++/v1/__config:593:1: error:
>>>   templates must have C++ linkage
>>> template  struct __static_assert_test;
>>> ^~~
>>> /usr/bin/../include/c++/v1/__config:594:20: error:
>>>   explicit specialization of non-template struct
>>> '__static_assert_test'
>>> template <> struct __static_assert_test {};
>>>^   ~~
>>> /usr/bin/../include/c++/v1/__config:595:1: error:
>>>   templates must have C++ linkage
>>> template  struct __static_assert_check {};
>>> ^~~
>>> 3 errors generated.
>>>
>>> Because the code is actually compiled in C++, the guard in the
>>> header failed to exclude the templates. In the meantime, I don't 
>>> know if
>>> there are ways to detect the header is in extern "C".
>>
>>
>> This was supposed to work, but apparently I only tested it when
>> compiling as C++11; the static_assert emulation in C++98 mode needs 
>> some
>> massaging to cope with this.
>>
>> Eric, Marshall: Are you OK with the attached patch? The idea is to
>> make

Re: [libcxx] r249798 - Split out of .

2016-01-26 Thread Eric Fiselier via cfe-commits

>  Eric, Marshall, what do you think about using only /*...*/-style
comments in these headers, to handle the case where libc++ is somehow in
the include path for a C89 compilation?

I'm NOT OK with this in general. These are C++ standard library headers
that are written in C++. These headers should *tolerate* being compiled as
C *system-headers*, but thats it.

If your passing "-std=c89 -pedantic" then your asking the compiler to
diagnose incompatibilities in *non-system-header* code. The assumption is
that code that compiles cleanly should  be portable to systems/compilers
that only provide C89 compilation. However libc++'s C headers can never
compile as pedantic C89 because they use "include_next". The reason you
haven't seen a -Wgnu-include-next diagnostic is because libc++ headers
manually turn on #pragma system_header near the top of the
file. Unfortunately since the license header comments appear before the
pragma they emit the "-Wcomment" diagnostics.

Installed libc++ headers are only included during C++ compilation so C
conformance is only an issue for projects that manually include a custom
libc++.These projects can include these headers as either "user-headers" or
"system-headers" by using "-I " or "-isystem " respectively
(ignoring -cxx-isystem for now). "user-headers" should compile with the
warning flags provided by the project, "system-headers" should not.
Including libc++ as "user-code" while compiling pedantic C89 that's a bug
because your project cannot actually compile with a C89 compiler.  Emitting
warnings/errors in this case seems correct and desired behavior. If the
choice to treat lib++ as "user-code" was not a conscious one then it's even
more important to diagnose it.

If I had my way we would turn off "#pragma system_header" all together
during C compilation and force the user to use -isystem.

/Eric

On Tue, Jan 26, 2016 at 12:14 PM, Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Eric, Marshall: has there been any progress here?
>
> On Wed, Jan 20, 2016 at 10:29 AM, Hans Wennborg  wrote:
> > /sub
> >
> > On Wed, Jan 20, 2016 at 4:45 AM, Nico Weber via cfe-commits
> >  wrote:
> >> Eric, Marshall: another ping. This should be fixed on the 3.8 branch,
> so it
> >> needs to be resolved soon.
> >>
> >> On Jan 5, 2016 5:25 PM, "Nico Weber"  wrote:
> >>>
> >>> On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith 
> >>> wrote:
> 
>  On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber 
> wrote:
> >
> > One problem with this patch: stdio.h can be used in .c files, and
> when
> > building .c files with -gnu99 -pedantic,
> 
> 
>  Do you mean -std=gnu89?
> 
> >
> > clang will complain about // comments. Not only does this stdio.h
> have
> > // comments, it also pulls in some libc++ headers (__config) that
> have //
> > comments as well. I suppose all the comments in header files pulled
> in by C
> > headers need to become /* */ comments?
> 
> 
>  I suppose so too. Your configuration is probably somewhat broken if
>  libc++'s headers are in your include path while building C code, but
> it
>  doesn't seem unreasonable to properly support that mode, and my
> changes were
>  already trying to do so.
> 
>  Eric, Marshall, what do you think about using only /*...*/-style
> comments
>  in these headers, to handle the case where libc++ is somehow in the
> include
>  path for a C89 compilation?
> >>>
> >>>
> >>> Eric, Marshall: Ping ^
> >>>
> 
> 
> >
> > On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits
> >  wrote:
> >>
> >> On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier 
> wrote:
> >>>
> >>> This change LGTM. Let's hold off on the using "_Static_assert"
> until
> >>> we understand how that would work with "-pedantic" when the macro
> is
> >>> expanded in user code.
> >>
> >>
> >> Committed as r250247, thanks.
> >>
> >>>
> >>> /Eric
> >>>
> >>> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith <
> rich...@metafoo.co.uk>
> >>> wrote:
> 
>  On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits
>   wrote:
> >
> > I would rather not do this if possible but I understand why we
> need
> > to do it.
> >
> > Richard is there a cost associated with the 'extern "C++"'
> > construct? or by forcing the compiler to switch modes in general?
> 
> 
>  Not a significant one compared to the cost of the code wrapped in
> the
>  'extern "C++"' here. (Also, this is wrapped in an #ifdef that
> only applies
>  in C++98; we could further reduce the set of cases when this
> happens by
>  using _Static_assert when available

Re: [libcxx] r249798 - Split out of .

2016-01-20 Thread Hans Wennborg via cfe-commits

/sub

On Wed, Jan 20, 2016 at 4:45 AM, Nico Weber via cfe-commits
 wrote:
> Eric, Marshall: another ping. This should be fixed on the 3.8 branch, so it
> needs to be resolved soon.
>
> On Jan 5, 2016 5:25 PM, "Nico Weber"  wrote:
>>
>> On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith 
>> wrote:
>>>
>>> On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber  wrote:

 One problem with this patch: stdio.h can be used in .c files, and when
 building .c files with -gnu99 -pedantic,
>>>
>>>
>>> Do you mean -std=gnu89?
>>>

 clang will complain about // comments. Not only does this stdio.h have
 // comments, it also pulls in some libc++ headers (__config) that have //
 comments as well. I suppose all the comments in header files pulled in by C
 headers need to become /* */ comments?
>>>
>>>
>>> I suppose so too. Your configuration is probably somewhat broken if
>>> libc++'s headers are in your include path while building C code, but it
>>> doesn't seem unreasonable to properly support that mode, and my changes were
>>> already trying to do so.
>>>
>>> Eric, Marshall, what do you think about using only /*...*/-style comments
>>> in these headers, to handle the case where libc++ is somehow in the include
>>> path for a C89 compilation?
>>
>>
>> Eric, Marshall: Ping ^
>>
>>>
>>>

 On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits
  wrote:
>
> On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier  wrote:
>>
>> This change LGTM. Let's hold off on the using "_Static_assert" until
>> we understand how that would work with "-pedantic" when the macro is
>> expanded in user code.
>
>
> Committed as r250247, thanks.
>
>>
>> /Eric
>>
>> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith 
>> wrote:
>>>
>>> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits
>>>  wrote:

 I would rather not do this if possible but I understand why we need
 to do it.

 Richard is there a cost associated with the 'extern "C++"'
 construct? or by forcing the compiler to switch modes in general?
>>>
>>>
>>> Not a significant one compared to the cost of the code wrapped in the
>>> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only 
>>> applies
>>> in C++98; we could further reduce the set of cases when this happens by
>>> using _Static_assert when available instead of this static_assert
>>> emulation.)
>>>

 On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith
  wrote:
>
> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits
>  wrote:
>>
>> Hi Richard
>>
>> Your splitting seems causing problem when using extern "C". Here
>> is a test case:
>>
>> $ cat test.cpp
>> #ifdef __cplusplus
>> extern "C" {
>> #endif
>> #include 
>> #ifdef __cplusplus
>> }
>> #endif
>>
>> Error:
>> clang -fsyntax-only test.cpp
>> In file included from test.cpp:4:
>> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
>> /usr/bin/../include/c++/v1/__config:593:1: error:
>>   templates must have C++ linkage
>> template  struct __static_assert_test;
>> ^~~
>> /usr/bin/../include/c++/v1/__config:594:20: error:
>>   explicit specialization of non-template struct
>> '__static_assert_test'
>> template <> struct __static_assert_test {};
>>^   ~~
>> /usr/bin/../include/c++/v1/__config:595:1: error:
>>   templates must have C++ linkage
>> template  struct __static_assert_check {};
>> ^~~
>> 3 errors generated.
>>
>> Because the code is actually compiled in C++, the guard in the
>> header failed to exclude the templates. In the meantime, I don't 
>> know if
>> there are ways to detect the header is in extern "C".
>
>
> This was supposed to work, but apparently I only tested it when
> compiling as C++11; the static_assert emulation in C++98 mode needs 
> some
> massaging to cope with this.
>
> Eric, Marshall: Are you OK with the attached patch? The idea is to
> make <__config> be fine to include in extern "C" or extern "C++" 
> modes (and
> likewise for the  headers). This is something that comes up 
> pretty
> often in practice (people wrap an include of a C header in 'extern 
> "C"', and

Re: [libcxx] r249798 - Split out of .

2016-01-20 Thread Nico Weber via cfe-commits

Eric, Marshall: another ping. This should be fixed on the 3.8 branch, so it
needs to be resolved soon.
On Jan 5, 2016 5:25 PM, "Nico Weber"  wrote:

> On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith 
> wrote:
>
>> On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber  wrote:
>>
>>> One problem with this patch: stdio.h can be used in .c files, and when
>>> building .c files with -gnu99 -pedantic,
>>>
>>
>> Do you mean -std=gnu89?
>>
>>
>>> clang will complain about // comments. Not only does this stdio.h have
>>> // comments, it also pulls in some libc++ headers (__config) that have //
>>> comments as well. I suppose all the comments in header files pulled in by C
>>> headers need to become /* */ comments?
>>>
>>
>> I suppose so too. Your configuration is probably somewhat broken if
>> libc++'s headers are in your include path while building C code, but it
>> doesn't seem unreasonable to properly support that mode, and my changes
>> were already trying to do so.
>>
>> Eric, Marshall, what do you think about using only /*...*/-style comments
>> in these headers, to handle the case where libc++ is somehow in the include
>> path for a C89 compilation?
>>
>
> Eric, Marshall: Ping ^
>
>
>>
>>
>>> On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier  wrote:

> This change LGTM. Let's hold off on the using "_Static_assert" until
> we understand how that would work with "-pedantic" when the macro is
> expanded in user code.
>

 Committed as r250247, thanks.


> /Eric
>
> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith 
> wrote:
>
>> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> I would rather not do this if possible but I understand why we need
>>> to do it.
>>>
>>> Richard is there a cost associated with the 'extern "C++"'
>>> construct? or by forcing the compiler to switch modes in general?
>>>
>>
>> Not a significant one compared to the cost of the code wrapped in the
>> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only 
>> applies
>> in C++98; we could further reduce the set of cases when this happens by
>> using _Static_assert when available instead of this static_assert
>> emulation.)
>>
>>
>>> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith <
>>> rich...@metafoo.co.uk> wrote:
>>>
 On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Hi Richard
>
> Your splitting seems causing problem when using extern "C". Here
> is a test case:
>
> $ cat test.cpp
> #ifdef __cplusplus
> extern "C" {
> #endif
> #include 
> #ifdef __cplusplus
> }
> #endif
>
> Error:
> clang -fsyntax-only test.cpp
> In file included from test.cpp:4:
> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
> /usr/bin/../include/c++/v1/__config:593:1: error:
>   templates must have C++ linkage
> template  struct __static_assert_test;
> ^~~
> /usr/bin/../include/c++/v1/__config:594:20: error:
>   explicit specialization of non-template struct
> '__static_assert_test'
> template <> struct __static_assert_test {};
>^   ~~
> /usr/bin/../include/c++/v1/__config:595:1: error:
>   templates must have C++ linkage
> template  struct __static_assert_check {};
> ^~~
> 3 errors generated.
>
> Because the code is actually compiled in C++, the guard in the
> header failed to exclude the templates. In the meantime, I don't know 
> if
> there are ways to detect the header is in extern "C".
>

 This was supposed to work, but apparently I only tested it when
 compiling as C++11; the static_assert emulation in C++98 mode needs 
 some
 massaging to cope with this.

 Eric, Marshall: Are you OK with the attached patch? The idea is to
 make <__config> be fine to include in extern "C" or extern "C++" modes 
 (and
 likewise for the  headers). This is something that comes up 
 pretty
 often in practice (people wrap an include of a C header in 'extern 
 "C"',
 and that C header includes a  file that libc++ provides).


> Steven
>
>
> > On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >

Re: [libcxx] r249798 - Split out of .

2015-12-31 Thread Nico Weber via cfe-commits

On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith 
wrote:

> On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber  wrote:
>
>> One problem with this patch: stdio.h can be used in .c files, and when
>> building .c files with -gnu99 -pedantic,
>>
>
> Do you mean -std=gnu89?
>

Sorry, I meant -std=gnu99 -pedantic -ansi:

$ clang -c test.c -std=gnu99 -pedantic -ansi
test.c:1:1: warning: // comments are not allowed in this language
[-Wcomment]
// hi
^

Might be a clang bug?


>
>> clang will complain about // comments. Not only does this stdio.h have //
>> comments, it also pulls in some libc++ headers (__config) that have //
>> comments as well. I suppose all the comments in header files pulled in by C
>> headers need to become /* */ comments?
>>
>
> I suppose so too. Your configuration is probably somewhat broken if
> libc++'s headers are in your include path while building C code, but it
> doesn't seem unreasonable to properly support that mode, and my changes
> were already trying to do so.
>

Thanks!


>
> Eric, Marshall, what do you think about using only /*...*/-style comments
> in these headers, to handle the case where libc++ is somehow in the include
> path for a C89 compilation?
>
>
>> On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier  wrote:
>>>
 This change LGTM. Let's hold off on the using "_Static_assert" until we
 understand how that would work with "-pedantic" when the macro is expanded
 in user code.

>>>
>>> Committed as r250247, thanks.
>>>
>>>
 /Eric

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

> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I would rather not do this if possible but I understand why we need
>> to do it.
>>
>> Richard is there a cost associated with the 'extern "C++"' construct?
>> or by forcing the compiler to switch modes in general?
>>
>
> Not a significant one compared to the cost of the code wrapped in the
> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only applies
> in C++98; we could further reduce the set of cases when this happens by
> using _Static_assert when available instead of this static_assert
> emulation.)
>
>
>> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith <
>> rich...@metafoo.co.uk> wrote:
>>
>>> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Hi Richard

 Your splitting seems causing problem when using extern "C". Here is
 a test case:

 $ cat test.cpp
 #ifdef __cplusplus
 extern "C" {
 #endif
 #include 
 #ifdef __cplusplus
 }
 #endif

 Error:
 clang -fsyntax-only test.cpp
 In file included from test.cpp:4:
 In file included from /usr/bin/../include/c++/v1/stdio.h:102:
 /usr/bin/../include/c++/v1/__config:593:1: error:
   templates must have C++ linkage
 template  struct __static_assert_test;
 ^~~
 /usr/bin/../include/c++/v1/__config:594:20: error:
   explicit specialization of non-template struct
 '__static_assert_test'
 template <> struct __static_assert_test {};
^   ~~
 /usr/bin/../include/c++/v1/__config:595:1: error:
   templates must have C++ linkage
 template  struct __static_assert_check {};
 ^~~
 3 errors generated.

 Because the code is actually compiled in C++, the guard in the
 header failed to exclude the templates. In the meantime, I don't know 
 if
 there are ways to detect the header is in extern "C".

>>>
>>> This was supposed to work, but apparently I only tested it when
>>> compiling as C++11; the static_assert emulation in C++98 mode needs some
>>> massaging to cope with this.
>>>
>>> Eric, Marshall: Are you OK with the attached patch? The idea is to
>>> make <__config> be fine to include in extern "C" or extern "C++" modes 
>>> (and
>>> likewise for the  headers). This is something that comes up 
>>> pretty
>>> often in practice (people wrap an include of a C header in 'extern "C"',
>>> and that C header includes a  file that libc++ provides).
>>>
>>>
 Steven


 > On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:
 >
 > Author: rsmith
 > Date: Thu Oct  8 20:29:09 2015
 > New Revision: 249798
 >
 > URL:

Re: [libcxx] r249798 - Split out of .

2015-12-31 Thread Nico Weber via cfe-commits

On Thu, Dec 31, 2015 at 10:52 AM, Nico Weber  wrote:

> On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith 
> wrote:
>
>> On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber  wrote:
>>
>>> One problem with this patch: stdio.h can be used in .c files, and when
>>> building .c files with -gnu99 -pedantic,
>>>
>>
>> Do you mean -std=gnu89?
>>
>
> Sorry, I meant -std=gnu99 -pedantic -ansi:
>
> $ clang -c test.c -std=gnu99 -pedantic -ansi
> test.c:1:1: warning: // comments are not allowed in this language
> [-Wcomment]
> // hi
> ^
>
> Might be a clang bug?
>

Ah, -ansi just means -std=c89, so passing both -std=gnu99 and -ansi doesn't
make a lot of sense. So not a clang bug :-)


> clang will complain about // comments. Not only does this stdio.h have //
>>> comments, it also pulls in some libc++ headers (__config) that have //
>>> comments as well. I suppose all the comments in header files pulled in by C
>>> headers need to become /* */ comments?
>>>
>>
>> I suppose so too. Your configuration is probably somewhat broken if
>> libc++'s headers are in your include path while building C code, but it
>> doesn't seem unreasonable to properly support that mode, and my changes
>> were already trying to do so.
>>
>
> Thanks!
>
>
>>
>> Eric, Marshall, what do you think about using only /*...*/-style comments
>> in these headers, to handle the case where libc++ is somehow in the include
>> path for a C89 compilation?
>>
>>
>>> On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier  wrote:

> This change LGTM. Let's hold off on the using "_Static_assert" until
> we understand how that would work with "-pedantic" when the macro is
> expanded in user code.
>

 Committed as r250247, thanks.


> /Eric
>
> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith 
> wrote:
>
>> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> I would rather not do this if possible but I understand why we need
>>> to do it.
>>>
>>> Richard is there a cost associated with the 'extern "C++"'
>>> construct? or by forcing the compiler to switch modes in general?
>>>
>>
>> Not a significant one compared to the cost of the code wrapped in the
>> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only 
>> applies
>> in C++98; we could further reduce the set of cases when this happens by
>> using _Static_assert when available instead of this static_assert
>> emulation.)
>>
>>
>>> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith <
>>> rich...@metafoo.co.uk> wrote:
>>>
 On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Hi Richard
>
> Your splitting seems causing problem when using extern "C". Here
> is a test case:
>
> $ cat test.cpp
> #ifdef __cplusplus
> extern "C" {
> #endif
> #include 
> #ifdef __cplusplus
> }
> #endif
>
> Error:
> clang -fsyntax-only test.cpp
> In file included from test.cpp:4:
> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
> /usr/bin/../include/c++/v1/__config:593:1: error:
>   templates must have C++ linkage
> template  struct __static_assert_test;
> ^~~
> /usr/bin/../include/c++/v1/__config:594:20: error:
>   explicit specialization of non-template struct
> '__static_assert_test'
> template <> struct __static_assert_test {};
>^   ~~
> /usr/bin/../include/c++/v1/__config:595:1: error:
>   templates must have C++ linkage
> template  struct __static_assert_check {};
> ^~~
> 3 errors generated.
>
> Because the code is actually compiled in C++, the guard in the
> header failed to exclude the templates. In the meantime, I don't know 
> if
> there are ways to detect the header is in extern "C".
>

 This was supposed to work, but apparently I only tested it when
 compiling as C++11; the static_assert emulation in C++98 mode needs 
 some
 massaging to cope with this.

 Eric, Marshall: Are you OK with the attached patch? The idea is to
 make <__config> be fine to include in extern "C" or extern "C++" modes 
 (and
 likewise for the  headers). This is something that comes up 
 pretty
 often in practice (people wrap an include of a C header in 'extern 
 "C"',
 and that C header

Re: [libcxx] r249798 - Split out of .

2015-12-30 Thread Nico Weber via cfe-commits

One problem with this patch: stdio.h can be used in .c files, and when
building .c files with -gnu99 -pedantic, clang will complain about //
comments. Not only does this stdio.h have // comments, it also pulls in
some libc++ headers (__config) that have // comments as well. I suppose all
the comments in header files pulled in by C headers need to become /* */
comments?

On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier  wrote:
>
>> This change LGTM. Let's hold off on the using "_Static_assert" until we
>> understand how that would work with "-pedantic" when the macro is expanded
>> in user code.
>>
>
> Committed as r250247, thanks.
>
>
>> /Eric
>>
>> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith 
>> wrote:
>>
>>> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 I would rather not do this if possible but I understand why we need to
 do it.

 Richard is there a cost associated with the 'extern "C++"' construct?
 or by forcing the compiler to switch modes in general?

>>>
>>> Not a significant one compared to the cost of the code wrapped in the
>>> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only applies
>>> in C++98; we could further reduce the set of cases when this happens by
>>> using _Static_assert when available instead of this static_assert
>>> emulation.)
>>>
>>>
 On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith 
 wrote:

> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Hi Richard
>>
>> Your splitting seems causing problem when using extern "C". Here is a
>> test case:
>>
>> $ cat test.cpp
>> #ifdef __cplusplus
>> extern "C" {
>> #endif
>> #include 
>> #ifdef __cplusplus
>> }
>> #endif
>>
>> Error:
>> clang -fsyntax-only test.cpp
>> In file included from test.cpp:4:
>> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
>> /usr/bin/../include/c++/v1/__config:593:1: error:
>>   templates must have C++ linkage
>> template  struct __static_assert_test;
>> ^~~
>> /usr/bin/../include/c++/v1/__config:594:20: error:
>>   explicit specialization of non-template struct
>> '__static_assert_test'
>> template <> struct __static_assert_test {};
>>^   ~~
>> /usr/bin/../include/c++/v1/__config:595:1: error:
>>   templates must have C++ linkage
>> template  struct __static_assert_check {};
>> ^~~
>> 3 errors generated.
>>
>> Because the code is actually compiled in C++, the guard in the header
>> failed to exclude the templates. In the meantime, I don't know if there 
>> are
>> ways to detect the header is in extern "C".
>>
>
> This was supposed to work, but apparently I only tested it when
> compiling as C++11; the static_assert emulation in C++98 mode needs some
> massaging to cope with this.
>
> Eric, Marshall: Are you OK with the attached patch? The idea is to
> make <__config> be fine to include in extern "C" or extern "C++" modes 
> (and
> likewise for the  headers). This is something that comes up pretty
> often in practice (people wrap an include of a C header in 'extern "C"',
> and that C header includes a  file that libc++ provides).
>
>
>> Steven
>>
>>
>> > On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> >
>> > Author: rsmith
>> > Date: Thu Oct  8 20:29:09 2015
>> > New Revision: 249798
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=249798=rev
>> > Log:
>> > Split  out of .
>> >
>> > As with , skip our custom header if __need_FILE or
>> __need___FILE is defined.
>> >
>> > Added:
>> >libcxx/trunk/include/stdio.h
>> >  - copied, changed from r249736, libcxx/trunk/include/cstdio
>> > Modified:
>> >libcxx/trunk/include/cstdio
>> >libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp
>> >
>> > Modified: libcxx/trunk/include/cstdio
>> > URL:
>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798=249797=249798=diff
>> >
>> ==
>> > --- libcxx/trunk/include/cstdio (original)
>> > +++ libcxx/trunk/include/cstdio Thu Oct  8 20:29:09 2015
>> > @@ -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
>> > -
>> >

Re: [libcxx] r249798 - Split out of .

2015-12-30 Thread Richard Smith via cfe-commits

On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber  wrote:

> One problem with this patch: stdio.h can be used in .c files, and when
> building .c files with -gnu99 -pedantic,
>

Do you mean -std=gnu89?


> clang will complain about // comments. Not only does this stdio.h have //
> comments, it also pulls in some libc++ headers (__config) that have //
> comments as well. I suppose all the comments in header files pulled in by C
> headers need to become /* */ comments?
>

I suppose so too. Your configuration is probably somewhat broken if
libc++'s headers are in your include path while building C code, but it
doesn't seem unreasonable to properly support that mode, and my changes
were already trying to do so.

Eric, Marshall, what do you think about using only /*...*/-style comments
in these headers, to handle the case where libc++ is somehow in the include
path for a C89 compilation?


> On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier  wrote:
>>
>>> This change LGTM. Let's hold off on the using "_Static_assert" until we
>>> understand how that would work with "-pedantic" when the macro is expanded
>>> in user code.
>>>
>>
>> Committed as r250247, thanks.
>>
>>
>>> /Eric
>>>
>>> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith 
>>> wrote:
>>>
 On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> I would rather not do this if possible but I understand why we need to
> do it.
>
> Richard is there a cost associated with the 'extern "C++"' construct?
> or by forcing the compiler to switch modes in general?
>

 Not a significant one compared to the cost of the code wrapped in the
 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only applies
 in C++98; we could further reduce the set of cases when this happens by
 using _Static_assert when available instead of this static_assert
 emulation.)


> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith  > wrote:
>
>> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Hi Richard
>>>
>>> Your splitting seems causing problem when using extern "C". Here is
>>> a test case:
>>>
>>> $ cat test.cpp
>>> #ifdef __cplusplus
>>> extern "C" {
>>> #endif
>>> #include 
>>> #ifdef __cplusplus
>>> }
>>> #endif
>>>
>>> Error:
>>> clang -fsyntax-only test.cpp
>>> In file included from test.cpp:4:
>>> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
>>> /usr/bin/../include/c++/v1/__config:593:1: error:
>>>   templates must have C++ linkage
>>> template  struct __static_assert_test;
>>> ^~~
>>> /usr/bin/../include/c++/v1/__config:594:20: error:
>>>   explicit specialization of non-template struct
>>> '__static_assert_test'
>>> template <> struct __static_assert_test {};
>>>^   ~~
>>> /usr/bin/../include/c++/v1/__config:595:1: error:
>>>   templates must have C++ linkage
>>> template  struct __static_assert_check {};
>>> ^~~
>>> 3 errors generated.
>>>
>>> Because the code is actually compiled in C++, the guard in the
>>> header failed to exclude the templates. In the meantime, I don't know if
>>> there are ways to detect the header is in extern "C".
>>>
>>
>> This was supposed to work, but apparently I only tested it when
>> compiling as C++11; the static_assert emulation in C++98 mode needs some
>> massaging to cope with this.
>>
>> Eric, Marshall: Are you OK with the attached patch? The idea is to
>> make <__config> be fine to include in extern "C" or extern "C++" modes 
>> (and
>> likewise for the  headers). This is something that comes up pretty
>> often in practice (people wrap an include of a C header in 'extern "C"',
>> and that C header includes a  file that libc++ provides).
>>
>>
>>> Steven
>>>
>>>
>>> > On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>> >
>>> > Author: rsmith
>>> > Date: Thu Oct  8 20:29:09 2015
>>> > New Revision: 249798
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=249798=rev
>>> > Log:
>>> > Split  out of .
>>> >
>>> > As with , skip our custom header if __need_FILE or
>>> __need___FILE is defined.
>>> >
>>> > Added:
>>> >libcxx/trunk/include/stdio.h
>>> >  - copied, changed from r249736, libcxx/trunk/include/cstdio
>>> > Modified:
>>> >libcxx/trunk/include/cstdio
>>> >

Re: [libcxx] r249798 - Split out of .

2015-10-13 Thread Eric Fiselier via cfe-commits

I would rather not do this if possible but I understand why we need to do
it.

Richard is there a cost associated with the 'extern "C++"' construct? or by
forcing the compiler to switch modes in general?




On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith 
wrote:

> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Hi Richard
>>
>> Your splitting seems causing problem when using extern "C". Here is a
>> test case:
>>
>> $ cat test.cpp
>> #ifdef __cplusplus
>> extern "C" {
>> #endif
>> #include 
>> #ifdef __cplusplus
>> }
>> #endif
>>
>> Error:
>> clang -fsyntax-only test.cpp
>> In file included from test.cpp:4:
>> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
>> /usr/bin/../include/c++/v1/__config:593:1: error:
>>   templates must have C++ linkage
>> template  struct __static_assert_test;
>> ^~~
>> /usr/bin/../include/c++/v1/__config:594:20: error:
>>   explicit specialization of non-template struct
>> '__static_assert_test'
>> template <> struct __static_assert_test {};
>>^   ~~
>> /usr/bin/../include/c++/v1/__config:595:1: error:
>>   templates must have C++ linkage
>> template  struct __static_assert_check {};
>> ^~~
>> 3 errors generated.
>>
>> Because the code is actually compiled in C++, the guard in the header
>> failed to exclude the templates. In the meantime, I don't know if there are
>> ways to detect the header is in extern "C".
>>
>
> This was supposed to work, but apparently I only tested it when compiling
> as C++11; the static_assert emulation in C++98 mode needs some massaging to
> cope with this.
>
> Eric, Marshall: Are you OK with the attached patch? The idea is to make
> <__config> be fine to include in extern "C" or extern "C++" modes (and
> likewise for the  headers). This is something that comes up pretty
> often in practice (people wrap an include of a C header in 'extern "C"',
> and that C header includes a  file that libc++ provides).
>
>
>> Steven
>>
>>
>> > On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> >
>> > Author: rsmith
>> > Date: Thu Oct  8 20:29:09 2015
>> > New Revision: 249798
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=249798=rev
>> > Log:
>> > Split  out of .
>> >
>> > As with , skip our custom header if __need_FILE or
>> __need___FILE is defined.
>> >
>> > Added:
>> >libcxx/trunk/include/stdio.h
>> >  - copied, changed from r249736, libcxx/trunk/include/cstdio
>> > Modified:
>> >libcxx/trunk/include/cstdio
>> >libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp
>> >
>> > Modified: libcxx/trunk/include/cstdio
>> > URL:
>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798=249797=249798=diff
>> >
>> ==
>> > --- libcxx/trunk/include/cstdio (original)
>> > +++ libcxx/trunk/include/cstdio Thu Oct  8 20:29:09 2015
>> > @@ -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;
>> >
>> > Copied: libcxx/trunk/include/stdio.h (from r249736,
>> libcxx/trunk/include/cstdio)
>> > URL:
>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/stdio.h?p2=libcxx/trunk/include/stdio.h=libcxx/trunk/include/cstdio=249736=249798=249798=diff
>> >
>> ==
>> > --- libcxx/trunk/include/cstdio (original)
>> > +++ libcxx/trunk/include/stdio.h Thu Oct  8 20:29:09 2015
>> > @@ -1,5 +1,5 @@
>> > // -*- C++ -*-
>> > -//=== cstdio
>> --===//
>> > +//=== stdio.h
>> -===//
>> > //
>> > // The LLVM Compiler Infrastructure
>> > //
>> > @@ -8,11 +8,19 @@
>> > //
>> >
>> //===--===//
>> >
>> > -#ifndef _LIBCPP_CSTDIO
>> > -#define _LIBCPP_CSTDIO
>> > +#if defined(__need_FILE) || defined(__need___FILE)
>> > +
>> > +#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
>> > +#pragma GCC system_header
>> > +#endif
>> > +
>> > +#include_next 
>> > +
>> > +#elif !defined(_LIBCPP_STDIO_H)
>> > +#define _LIBCPP_STDIO_H
>> >
>> > /*
>> > -cstdio synopsis
>> > +stdio.h synopsis
>> >
>> > Macros:
>> >
>> > @@ -33,9 +41,6 @@ Macros:
>> > stdin
>> > stdout
>> >
>> > -namespace std
>> > -{
>> > -
>> > Types:
>> >
>> > FILE
>> > @@ -92,20 +97,23 @@ void clearerr(FILE* stream);
>> > int feof(FILE* stream);
>> > int ferror(FILE* stream);
>> > void perror(const char* s);
>> >

Re: [libcxx] r249798 - Split out of .

2015-10-13 Thread Eric Fiselier via cfe-commits

This change LGTM. Let's hold off on the using "_Static_assert" until we
understand how that would work with "-pedantic" when the macro is expanded
in user code.

/Eric

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

> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I would rather not do this if possible but I understand why we need to do
>> it.
>>
>> Richard is there a cost associated with the 'extern "C++"' construct? or
>> by forcing the compiler to switch modes in general?
>>
>
> Not a significant one compared to the cost of the code wrapped in the
> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only applies
> in C++98; we could further reduce the set of cases when this happens by
> using _Static_assert when available instead of this static_assert
> emulation.)
>
>
>> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith 
>> wrote:
>>
>>> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Hi Richard

 Your splitting seems causing problem when using extern "C". Here is a
 test case:

 $ cat test.cpp
 #ifdef __cplusplus
 extern "C" {
 #endif
 #include 
 #ifdef __cplusplus
 }
 #endif

 Error:
 clang -fsyntax-only test.cpp
 In file included from test.cpp:4:
 In file included from /usr/bin/../include/c++/v1/stdio.h:102:
 /usr/bin/../include/c++/v1/__config:593:1: error:
   templates must have C++ linkage
 template  struct __static_assert_test;
 ^~~
 /usr/bin/../include/c++/v1/__config:594:20: error:
   explicit specialization of non-template struct
 '__static_assert_test'
 template <> struct __static_assert_test {};
^   ~~
 /usr/bin/../include/c++/v1/__config:595:1: error:
   templates must have C++ linkage
 template  struct __static_assert_check {};
 ^~~
 3 errors generated.

 Because the code is actually compiled in C++, the guard in the header
 failed to exclude the templates. In the meantime, I don't know if there are
 ways to detect the header is in extern "C".

>>>
>>> This was supposed to work, but apparently I only tested it when
>>> compiling as C++11; the static_assert emulation in C++98 mode needs some
>>> massaging to cope with this.
>>>
>>> Eric, Marshall: Are you OK with the attached patch? The idea is to make
>>> <__config> be fine to include in extern "C" or extern "C++" modes (and
>>> likewise for the  headers). This is something that comes up pretty
>>> often in practice (people wrap an include of a C header in 'extern "C"',
>>> and that C header includes a  file that libc++ provides).
>>>
>>>
 Steven


 > On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:
 >
 > Author: rsmith
 > Date: Thu Oct  8 20:29:09 2015
 > New Revision: 249798
 >
 > URL: http://llvm.org/viewvc/llvm-project?rev=249798=rev
 > Log:
 > Split  out of .
 >
 > As with , skip our custom header if __need_FILE or
 __need___FILE is defined.
 >
 > Added:
 >libcxx/trunk/include/stdio.h
 >  - copied, changed from r249736, libcxx/trunk/include/cstdio
 > Modified:
 >libcxx/trunk/include/cstdio
 >libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp
 >
 > Modified: libcxx/trunk/include/cstdio
 > URL:
 http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798=249797=249798=diff
 >
 ==
 > --- libcxx/trunk/include/cstdio (original)
 > +++ libcxx/trunk/include/cstdio Thu Oct  8 20:29:09 2015
 > @@ -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;
 >
 > Copied: libcxx/trunk/include/stdio.h (from r249736,
 libcxx/trunk/include/cstdio)
 > URL:
 http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/stdio.h?p2=libcxx/trunk/include/stdio.h=libcxx/trunk/include/cstdio=249736=249798=249798=diff
 >
 ==
 > --- libcxx/trunk/include/cstdio (original)
 > +++ libcxx/trunk/include/stdio.h Thu Oct  8 20:29:09 2015
 > @@ -1,5 +1,5 @@
 > // -*- C++ -*-
 > -//=== cstdio
 --===//
 > +//=== stdio.h

Re: [libcxx] r249798 - Split out of .

2015-10-13 Thread Richard Smith via cfe-commits

On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> I would rather not do this if possible but I understand why we need to do
> it.
>
> Richard is there a cost associated with the 'extern "C++"' construct? or
> by forcing the compiler to switch modes in general?
>

Not a significant one compared to the cost of the code wrapped in the
'extern "C++"' here. (Also, this is wrapped in an #ifdef that only applies
in C++98; we could further reduce the set of cases when this happens by
using _Static_assert when available instead of this static_assert
emulation.)


> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith 
> wrote:
>
>> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Hi Richard
>>>
>>> Your splitting seems causing problem when using extern "C". Here is a
>>> test case:
>>>
>>> $ cat test.cpp
>>> #ifdef __cplusplus
>>> extern "C" {
>>> #endif
>>> #include 
>>> #ifdef __cplusplus
>>> }
>>> #endif
>>>
>>> Error:
>>> clang -fsyntax-only test.cpp
>>> In file included from test.cpp:4:
>>> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
>>> /usr/bin/../include/c++/v1/__config:593:1: error:
>>>   templates must have C++ linkage
>>> template  struct __static_assert_test;
>>> ^~~
>>> /usr/bin/../include/c++/v1/__config:594:20: error:
>>>   explicit specialization of non-template struct
>>> '__static_assert_test'
>>> template <> struct __static_assert_test {};
>>>^   ~~
>>> /usr/bin/../include/c++/v1/__config:595:1: error:
>>>   templates must have C++ linkage
>>> template  struct __static_assert_check {};
>>> ^~~
>>> 3 errors generated.
>>>
>>> Because the code is actually compiled in C++, the guard in the header
>>> failed to exclude the templates. In the meantime, I don't know if there are
>>> ways to detect the header is in extern "C".
>>>
>>
>> This was supposed to work, but apparently I only tested it when compiling
>> as C++11; the static_assert emulation in C++98 mode needs some massaging to
>> cope with this.
>>
>> Eric, Marshall: Are you OK with the attached patch? The idea is to make
>> <__config> be fine to include in extern "C" or extern "C++" modes (and
>> likewise for the  headers). This is something that comes up pretty
>> often in practice (people wrap an include of a C header in 'extern "C"',
>> and that C header includes a  file that libc++ provides).
>>
>>
>>> Steven
>>>
>>>
>>> > On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>> >
>>> > Author: rsmith
>>> > Date: Thu Oct  8 20:29:09 2015
>>> > New Revision: 249798
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=249798=rev
>>> > Log:
>>> > Split  out of .
>>> >
>>> > As with , skip our custom header if __need_FILE or
>>> __need___FILE is defined.
>>> >
>>> > Added:
>>> >libcxx/trunk/include/stdio.h
>>> >  - copied, changed from r249736, libcxx/trunk/include/cstdio
>>> > Modified:
>>> >libcxx/trunk/include/cstdio
>>> >libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp
>>> >
>>> > Modified: libcxx/trunk/include/cstdio
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798=249797=249798=diff
>>> >
>>> ==
>>> > --- libcxx/trunk/include/cstdio (original)
>>> > +++ libcxx/trunk/include/cstdio Thu Oct  8 20:29:09 2015
>>> > @@ -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;
>>> >
>>> > Copied: libcxx/trunk/include/stdio.h (from r249736,
>>> libcxx/trunk/include/cstdio)
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/stdio.h?p2=libcxx/trunk/include/stdio.h=libcxx/trunk/include/cstdio=249736=249798=249798=diff
>>> >
>>> ==
>>> > --- libcxx/trunk/include/cstdio (original)
>>> > +++ libcxx/trunk/include/stdio.h Thu Oct  8 20:29:09 2015
>>> > @@ -1,5 +1,5 @@
>>> > // -*- C++ -*-
>>> > -//=== cstdio
>>> --===//
>>> > +//=== stdio.h
>>> -===//
>>> > //
>>> > // The LLVM Compiler Infrastructure
>>> > //
>>> > @@ -8,11 +8,19 @@
>>> > //
>>> >
>>> //===--===//
>>> >
>>> > -#ifndef _LIBCPP_CSTDIO
>>> > -#define _LIBCPP_CSTDIO
>>> > +#if defined(__need_FILE) || defined(__need___FILE)
>>> > +
>>> > +#if

Re: [libcxx] r249798 - Split out of .

2015-10-13 Thread Richard Smith via cfe-commits

On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier  wrote:

> This change LGTM. Let's hold off on the using "_Static_assert" until we
> understand how that would work with "-pedantic" when the macro is expanded
> in user code.
>

Committed as r250247, thanks.


> /Eric
>
> On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith 
> wrote:
>
>> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> I would rather not do this if possible but I understand why we need to
>>> do it.
>>>
>>> Richard is there a cost associated with the 'extern "C++"' construct? or
>>> by forcing the compiler to switch modes in general?
>>>
>>
>> Not a significant one compared to the cost of the code wrapped in the
>> 'extern "C++"' here. (Also, this is wrapped in an #ifdef that only applies
>> in C++98; we could further reduce the set of cases when this happens by
>> using _Static_assert when available instead of this static_assert
>> emulation.)
>>
>>
>>> On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith 
>>> wrote:
>>>
 On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Hi Richard
>
> Your splitting seems causing problem when using extern "C". Here is a
> test case:
>
> $ cat test.cpp
> #ifdef __cplusplus
> extern "C" {
> #endif
> #include 
> #ifdef __cplusplus
> }
> #endif
>
> Error:
> clang -fsyntax-only test.cpp
> In file included from test.cpp:4:
> In file included from /usr/bin/../include/c++/v1/stdio.h:102:
> /usr/bin/../include/c++/v1/__config:593:1: error:
>   templates must have C++ linkage
> template  struct __static_assert_test;
> ^~~
> /usr/bin/../include/c++/v1/__config:594:20: error:
>   explicit specialization of non-template struct
> '__static_assert_test'
> template <> struct __static_assert_test {};
>^   ~~
> /usr/bin/../include/c++/v1/__config:595:1: error:
>   templates must have C++ linkage
> template  struct __static_assert_check {};
> ^~~
> 3 errors generated.
>
> Because the code is actually compiled in C++, the guard in the header
> failed to exclude the templates. In the meantime, I don't know if there 
> are
> ways to detect the header is in extern "C".
>

 This was supposed to work, but apparently I only tested it when
 compiling as C++11; the static_assert emulation in C++98 mode needs some
 massaging to cope with this.

 Eric, Marshall: Are you OK with the attached patch? The idea is to make
 <__config> be fine to include in extern "C" or extern "C++" modes (and
 likewise for the  headers). This is something that comes up pretty
 often in practice (people wrap an include of a C header in 'extern "C"',
 and that C header includes a  file that libc++ provides).


> Steven
>
>
> > On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Author: rsmith
> > Date: Thu Oct  8 20:29:09 2015
> > New Revision: 249798
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=249798=rev
> > Log:
> > Split  out of .
> >
> > As with , skip our custom header if __need_FILE or
> __need___FILE is defined.
> >
> > Added:
> >libcxx/trunk/include/stdio.h
> >  - copied, changed from r249736, libcxx/trunk/include/cstdio
> > Modified:
> >libcxx/trunk/include/cstdio
> >libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp
> >
> > Modified: libcxx/trunk/include/cstdio
> > URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798=249797=249798=diff
> >
> ==
> > --- libcxx/trunk/include/cstdio (original)
> > +++ libcxx/trunk/include/cstdio Thu Oct  8 20:29:09 2015
> > @@ -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;
> >
> > Copied: libcxx/trunk/include/stdio.h (from r249736,
> libcxx/trunk/include/cstdio)
> > URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/stdio.h?p2=libcxx/trunk/include/stdio.h=libcxx/trunk/include/cstdio=249736=249798=249798=diff
> >
> ==
> > --- libcxx/trunk/include/cstdio (original)
> > +++

Re: [libcxx] r249798 - Split out of .

2015-10-12 Thread Steven Wu via cfe-commits

Hi Richard

Your splitting seems causing problem when using extern "C". Here is a test case:

$ cat test.cpp 
#ifdef __cplusplus
extern "C" {
#endif
#include 
#ifdef __cplusplus
}
#endif

Error:
clang -fsyntax-only test.cpp 
In file included from test.cpp:4:
In file included from /usr/bin/../include/c++/v1/stdio.h:102:
/usr/bin/../include/c++/v1/__config:593:1: error: 
  templates must have C++ linkage
template  struct __static_assert_test;
^~~
/usr/bin/../include/c++/v1/__config:594:20: error: 
  explicit specialization of non-template struct '__static_assert_test'
template <> struct __static_assert_test {};
   ^   ~~
/usr/bin/../include/c++/v1/__config:595:1: error: 
  templates must have C++ linkage
template  struct __static_assert_check {};
^~~
3 errors generated.

Because the code is actually compiled in C++, the guard in the header failed to 
exclude the templates. In the meantime, I don't know if there are ways to 
detect the header is in extern "C".

Steven


> On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits 
>  wrote:
> 
> Author: rsmith
> Date: Thu Oct  8 20:29:09 2015
> New Revision: 249798
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=249798=rev
> Log:
> Split  out of .
> 
> As with , skip our custom header if __need_FILE or __need___FILE is 
> defined.
> 
> Added:
>libcxx/trunk/include/stdio.h
>  - copied, changed from r249736, libcxx/trunk/include/cstdio
> Modified:
>libcxx/trunk/include/cstdio
>libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp
> 
> Modified: libcxx/trunk/include/cstdio
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798=249797=249798=diff
> ==
> --- libcxx/trunk/include/cstdio (original)
> +++ libcxx/trunk/include/cstdio Thu Oct  8 20:29:09 2015
> @@ -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;
> 
> Copied: libcxx/trunk/include/stdio.h (from r249736, 
> libcxx/trunk/include/cstdio)
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/stdio.h?p2=libcxx/trunk/include/stdio.h=libcxx/trunk/include/cstdio=249736=249798=249798=diff
> ==
> --- libcxx/trunk/include/cstdio (original)
> +++ libcxx/trunk/include/stdio.h Thu Oct  8 20:29:09 2015
> @@ -1,5 +1,5 @@
> // -*- C++ -*-
> -//=== cstdio 
> --===//
> +//=== stdio.h 
> -===//
> //
> // The LLVM Compiler Infrastructure
> //
> @@ -8,11 +8,19 @@
> //
> //===--===//
> 
> -#ifndef _LIBCPP_CSTDIO
> -#define _LIBCPP_CSTDIO
> +#if defined(__need_FILE) || defined(__need___FILE)
> +
> +#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
> +#pragma GCC system_header
> +#endif
> +
> +#include_next 
> +
> +#elif !defined(_LIBCPP_STDIO_H)
> +#define _LIBCPP_STDIO_H
> 
> /*
> -cstdio synopsis
> +stdio.h synopsis
> 
> Macros:
> 
> @@ -33,9 +41,6 @@ Macros:
> stdin
> stdout
> 
> -namespace std
> -{
> -
> Types:
> 
> FILE
> @@ -92,20 +97,23 @@ void clearerr(FILE* stream);
> int feof(FILE* stream);
> int ferror(FILE* stream);
> void perror(const char* s);
> -
> -}  // std
> */
> 
> #include <__config>
> -#include 
> 
> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
> #pragma GCC system_header
> #endif
> 
> +#include_next 
> +
> +#ifdef __cplusplus
> +
> // snprintf
> #if defined(_LIBCPP_MSVCRT)
> +extern "C++" {
> #include "support/win32/support.h"
> +}
> #endif
> 
> #undef getc
> @@ -113,72 +121,7 @@ void perror(const char* s);
> #undef clearerr
> #undef feof
> #undef ferror
> -_LIBCPP_BEGIN_NAMESPACE_STD
> -
> -using ::FILE;
> -using ::fpos_t;
> -using ::size_t;
> -
> -using ::fclose;
> -using ::fflush;
> -using ::setbuf;
> -using ::setvbuf;
> -using ::fprintf;
> -using ::fscanf;
> -using ::snprintf;
> -using ::sprintf;
> -using ::sscanf;
> -#ifndef _LIBCPP_MSVCRT
> -using ::vfprintf;
> -using ::vfscanf;
> -using ::vsscanf;
> -#endif // _LIBCPP_MSVCRT
> -using ::vsnprintf;
> -using ::vsprintf;
> -using ::fgetc;
> -using ::fgets;
> -using ::fputc;
> -using ::fputs;
> -using ::getc;
> -using ::putc;
> -using ::ungetc;
> -using ::fread;
> -using ::fwrite;
> -using ::fgetpos;
> -using ::fseek;
> -using ::fsetpos;
> -using ::ftell;
> -using ::rewind;
> -using ::clearerr;
> -using ::feof;
> -using ::ferror;
> -using ::perror;
> -
> -#ifndef _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE
> -using ::fopen;
> -using ::freopen;
> -using

[libcxx] r249798 - Split out of .

2015-10-08 Thread Richard Smith via cfe-commits

Author: rsmith
Date: Thu Oct  8 20:29:09 2015
New Revision: 249798

URL: http://llvm.org/viewvc/llvm-project?rev=249798=rev
Log:
Split  out of .

As with , skip our custom header if __need_FILE or __need___FILE is 
defined.

Added:
libcxx/trunk/include/stdio.h
  - copied, changed from r249736, libcxx/trunk/include/cstdio
Modified:
libcxx/trunk/include/cstdio
libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp

Modified: libcxx/trunk/include/cstdio
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798=249797=249798=diff
==
--- libcxx/trunk/include/cstdio (original)
+++ libcxx/trunk/include/cstdio Thu Oct  8 20:29:09 2015
@@ -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;

Copied: libcxx/trunk/include/stdio.h (from r249736, libcxx/trunk/include/cstdio)
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/stdio.h?p2=libcxx/trunk/include/stdio.h=libcxx/trunk/include/cstdio=249736=249798=249798=diff
==
--- libcxx/trunk/include/cstdio (original)
+++ libcxx/trunk/include/stdio.h Thu Oct  8 20:29:09 2015
@@ -1,5 +1,5 @@
 // -*- C++ -*-
-//=== cstdio 
--===//
+//=== stdio.h 
-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -8,11 +8,19 @@
 //
 
//===--===//
 
-#ifndef _LIBCPP_CSTDIO
-#define _LIBCPP_CSTDIO
+#if defined(__need_FILE) || defined(__need___FILE)
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#include_next 
+
+#elif !defined(_LIBCPP_STDIO_H)
+#define _LIBCPP_STDIO_H
 
 /*
-cstdio synopsis
+stdio.h synopsis
 
 Macros:
 
@@ -33,9 +41,6 @@ Macros:
 stdin
 stdout
 
-namespace std
-{
-
 Types:
 
 FILE
@@ -92,20 +97,23 @@ void clearerr(FILE* stream);
 int feof(FILE* stream);
 int ferror(FILE* stream);
 void perror(const char* s);
-
-}  // std
 */
 
 #include <__config>
-#include 
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
+#include_next 
+
+#ifdef __cplusplus
+
 // snprintf
 #if defined(_LIBCPP_MSVCRT)
+extern "C++" {
 #include "support/win32/support.h"
+}
 #endif
 
 #undef getc
@@ -113,72 +121,7 @@ void perror(const char* s);
 #undef clearerr
 #undef feof
 #undef ferror
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-using ::FILE;
-using ::fpos_t;
-using ::size_t;
-
-using ::fclose;
-using ::fflush;
-using ::setbuf;
-using ::setvbuf;
-using ::fprintf;
-using ::fscanf;
-using ::snprintf;
-using ::sprintf;
-using ::sscanf;
-#ifndef _LIBCPP_MSVCRT
-using ::vfprintf;
-using ::vfscanf;
-using ::vsscanf;
-#endif // _LIBCPP_MSVCRT
-using ::vsnprintf;
-using ::vsprintf;
-using ::fgetc;
-using ::fgets;
-using ::fputc;
-using ::fputs;
-using ::getc;
-using ::putc;
-using ::ungetc;
-using ::fread;
-using ::fwrite;
-using ::fgetpos;
-using ::fseek;
-using ::fsetpos;
-using ::ftell;
-using ::rewind;
-using ::clearerr;
-using ::feof;
-using ::ferror;
-using ::perror;
-
-#ifndef _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE
-using ::fopen;
-using ::freopen;
-using ::remove;
-using ::rename;
-using ::tmpfile;
-using ::tmpnam;
-#endif
 
-#ifndef _LIBCPP_HAS_NO_STDIN
-using ::getchar;
-#if _LIBCPP_STD_VER <= 11
-using ::gets;
 #endif
-using ::scanf;
-using ::vscanf;
-#endif
-
-#ifndef _LIBCPP_HAS_NO_STDOUT
-using ::printf;
-using ::putchar;
-using ::puts;
-using ::vprintf;
-#endif
-
-_LIBCPP_END_NAMESPACE_STD
 
-#endif  // _LIBCPP_CSTDIO
+#endif  // _LIBCPP_STDIO_H

Modified: libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp?rev=249798=249797=249798=diff
==
--- libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp (original)
+++ libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp Thu Oct  8 
20:29:09 2015
@@ -13,6 +13,26 @@
 #include 
 #include "test_macros.h"
 
+#ifdef getc
+#error getc is defined
+#endif
+
+#ifdef putc
+#error putc is defined
+#endif
+
+#ifdef clearerr
+#error clearerr is defined
+#endif
+
+#ifdef feof
+#error feof is defined
+#endif
+
+#ifdef ferror
+#error ferror is defined
+#endif
+
 #ifndef BUFSIZ
 #error BUFSIZ not defined
 #endif


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

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

Re: [libcxx] r249798 - Split out of .

[libcxx] r249798 - Split out of .

14 matches

Site Navigation

Mail list logo

Footer information