/sub
On Wed, Jan 20, 2016 at 4:45 AM, Nico Weber via cfe-commits <cfe-commits@lists.llvm.org> 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" <tha...@chromium.org> wrote: >> >> 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 > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits