[libcxx] r249798 - Split <stdio.h> out of <cstdio>.
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 31 07:52:37 PST 2015
On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith <richard at metafoo.co.uk>
wrote:
> On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber <thakis at 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?
>
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 at lists.llvm.org> wrote:
>>
>>> On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier <eric at 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 <richard at metafoo.co.uk>
>>>> wrote:
>>>>
>>>>> On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <
>>>>> cfe-commits at 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 <
>>>>>> richard at metafoo.co.uk> wrote:
>>>>>>
>>>>>>> On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <
>>>>>>> cfe-commits at 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 at 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 at lists.llvm.org
>>>>>>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> cfe-commits mailing list
>>>>>>>> cfe-commits at lists.llvm.org
>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151231/20b5ab27/attachment-0001.html>
More information about the cfe-commits
mailing list