On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber <tha...@chromium.org> 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 <e...@efcs.ca> 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 <
>>>> 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 <stdio.h>
>>>>>>> #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 <bool> 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<true> {};
>>>>>>> ^ ~~~~~~
>>>>>>> /usr/bin/../include/c++/v1/__config:595:1: error:
>>>>>>> templates must have C++ linkage
>>>>>>> template <unsigned> 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 <foo.h> 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 <foo.h> 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&view=rev
>>>>>>> > Log:
>>>>>>> > Split <stdio.h> out of <cstdio>.
>>>>>>> >
>>>>>>> > As with <stddef.h>, 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&r1=249797&r2=249798&view=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&p1=libcxx/trunk/include/cstdio&r1=249736&r2=249798&rev=249798&view=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 <stdio.h>
>>>>>>> > +
>>>>>>> > +#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 <stdio.h>
>>>>>>> >
>>>>>>> > #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
>>>>>>> > #pragma GCC system_header
>>>>>>> > #endif
>>>>>>> >
>>>>>>> > +#include_next <stdio.h>
>>>>>>> > +
>>>>>>> > +#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&r1=249797&r2=249798&view=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 <type_traits>
>>>>>>> > #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
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> 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
>>>>>
>>>>>
>>>>
>>>
>>
>> _______________________________________________
>> 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
