On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> 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? > 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 <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