<p dir="ltr">Eric, Marshall: another ping. This should be fixed on the 3.8 branch, so it needs to be resolved soon.</p>
<div class="gmail_quote">On Jan 5, 2016 5:25 PM, "Nico Weber" <<a href="mailto:thakis@chromium.org">thakis@chromium.org</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 30, 2015 at 8:28 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Wed, Dec 30, 2015 at 1:17 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">One problem with this patch: stdio.h can be used in .c files, and when building .c files with -gnu99 -pedantic,</div></blockquote><div><br></div></span><div>Do you mean -std=gnu89?</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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?</div></blockquote><div><br></div></span><div>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.</div><div><br></div><div>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?</div></div></div></div></blockquote><div><br></div><div>Eric, Marshall: Ping ^</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div class="gmail_extra"><div class="gmail_quote">On Tue, Oct 13, 2015 at 7:34 PM, Richard Smith via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Tue, Oct 13, 2015 at 3:26 PM, Eric Fiselier <span dir="ltr"><<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">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.</div></blockquote><div><br></div></span><div>Committed as r250247, thanks.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><span><font color="#888888"><div>/Eric</div></font></span></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 13, 2015 at 4:19 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Tue, Oct 13, 2015 at 2:12 PM, Eric Fiselier via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">I would rather not do this if possible but I understand why we need to do it.<div><br></div><div>Richard is there a cost associated with the 'extern "C++"' construct? or by forcing the compiler to switch modes in general?</div></div></blockquote><div><br></div></span><div>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.)</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 12, 2015 at 12:27 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div>On Mon, Oct 12, 2015 at 9:41 AM, Steven Wu via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Richard<br>
<br>
Your splitting seems causing problem when using extern "C". Here is a test case:<br>
<br>
$ cat test.cpp<br>
#ifdef __cplusplus<br>
extern "C" {<br>
#endif<br>
#include <stdio.h><br>
#ifdef __cplusplus<br>
}<br>
#endif<br>
<br>
Error:<br>
clang -fsyntax-only test.cpp<br>
In file included from test.cpp:4:<br>
In file included from /usr/bin/../include/c++/v1/stdio.h:102:<br>
/usr/bin/../include/c++/v1/__config:593:1: error:<br>
      templates must have C++ linkage<br>
template <bool> struct __static_assert_test;<br>
^~~~~~~~~~~~~~~<br>
/usr/bin/../include/c++/v1/__config:594:20: error:<br>
      explicit specialization of non-template struct '__static_assert_test'<br>
template <> struct __static_assert_test<true> {};<br>
                   ^                   ~~~~~~<br>
/usr/bin/../include/c++/v1/__config:595:1: error:<br>
      templates must have C++ linkage<br>
template <unsigned> struct __static_assert_check {};<br>
^~~~~~~~~~~~~~~~~~~<br>
3 errors generated.<br>
<br>
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".<br></blockquote><div><br></div></div></div><div>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.</div><div><br></div><div>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).</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Steven<br>
<div><div><br>
<br>
> On Oct 8, 2015, at 6:29 PM, Richard Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Author: rsmith<br>
> Date: Thu Oct  8 20:29:09 2015<br>
> New Revision: 249798<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=249798&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=249798&view=rev</a><br>
> Log:<br>
> Split <stdio.h> out of <cstdio>.<br>
><br>
> As with <stddef.h>, skip our custom header if __need_FILE or __need___FILE is defined.<br>
><br>
> Added:<br>
>    libcxx/trunk/include/stdio.h<br>
>      - copied, changed from r249736, libcxx/trunk/include/cstdio<br>
> Modified:<br>
>    libcxx/trunk/include/cstdio<br>
>    libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp<br>
><br>
> Modified: libcxx/trunk/include/cstdio<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798&r1=249797&r2=249798&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cstdio?rev=249798&r1=249797&r2=249798&view=diff</a><br>
> ==============================================================================<br>
> --- libcxx/trunk/include/cstdio (original)<br>
> +++ libcxx/trunk/include/cstdio Thu Oct  8 20:29:09 2015<br>
> @@ -103,16 +103,6 @@ void perror(const char* s);<br>
> #pragma GCC system_header<br>
> #endif<br>
><br>
> -// snprintf<br>
> -#if defined(_LIBCPP_MSVCRT)<br>
> -#include "support/win32/support.h"<br>
> -#endif<br>
> -<br>
> -#undef getc<br>
> -#undef putc<br>
> -#undef clearerr<br>
> -#undef feof<br>
> -#undef ferror<br>
> _LIBCPP_BEGIN_NAMESPACE_STD<br>
><br>
> using ::FILE;<br>
><br>
> Copied: libcxx/trunk/include/stdio.h (from r249736, libcxx/trunk/include/cstdio)<br>
> URL: <a href="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" rel="noreferrer" target="_blank">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</a><br>
> ==============================================================================<br>
> --- libcxx/trunk/include/cstdio (original)<br>
> +++ libcxx/trunk/include/stdio.h Thu Oct  8 20:29:09 2015<br>
> @@ -1,5 +1,5 @@<br>
> // -*- C++ -*-<br>
> -//===---------------------------- cstdio ----------------------------------===//<br>
> +//===---------------------------- stdio.h ---------------------------------===//<br>
> //<br>
> //                     The LLVM Compiler Infrastructure<br>
> //<br>
> @@ -8,11 +8,19 @@<br>
> //<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> -#ifndef _LIBCPP_CSTDIO<br>
> -#define _LIBCPP_CSTDIO<br>
> +#if defined(__need_FILE) || defined(__need___FILE)<br>
> +<br>
> +#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)<br>
> +#pragma GCC system_header<br>
> +#endif<br>
> +<br>
> +#include_next <stdio.h><br>
> +<br>
> +#elif !defined(_LIBCPP_STDIO_H)<br>
> +#define _LIBCPP_STDIO_H<br>
><br>
> /*<br>
> -    cstdio synopsis<br>
> +    stdio.h synopsis<br>
><br>
> Macros:<br>
><br>
> @@ -33,9 +41,6 @@ Macros:<br>
>     stdin<br>
>     stdout<br>
><br>
> -namespace std<br>
> -{<br>
> -<br>
> Types:<br>
><br>
> FILE<br>
> @@ -92,20 +97,23 @@ void clearerr(FILE* stream);<br>
> int feof(FILE* stream);<br>
> int ferror(FILE* stream);<br>
> void perror(const char* s);<br>
> -<br>
> -}  // std<br>
> */<br>
><br>
> #include <__config><br>
> -#include <stdio.h><br>
><br>
> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)<br>
> #pragma GCC system_header<br>
> #endif<br>
><br>
> +#include_next <stdio.h><br>
> +<br>
> +#ifdef __cplusplus<br>
> +<br>
> // snprintf<br>
> #if defined(_LIBCPP_MSVCRT)<br>
> +extern "C++" {<br>
> #include "support/win32/support.h"<br>
> +}<br>
> #endif<br>
><br>
> #undef getc<br>
> @@ -113,72 +121,7 @@ void perror(const char* s);<br>
> #undef clearerr<br>
> #undef feof<br>
> #undef ferror<br>
> -_LIBCPP_BEGIN_NAMESPACE_STD<br>
> -<br>
> -using ::FILE;<br>
> -using ::fpos_t;<br>
> -using ::size_t;<br>
> -<br>
> -using ::fclose;<br>
> -using ::fflush;<br>
> -using ::setbuf;<br>
> -using ::setvbuf;<br>
> -using ::fprintf;<br>
> -using ::fscanf;<br>
> -using ::snprintf;<br>
> -using ::sprintf;<br>
> -using ::sscanf;<br>
> -#ifndef _LIBCPP_MSVCRT<br>
> -using ::vfprintf;<br>
> -using ::vfscanf;<br>
> -using ::vsscanf;<br>
> -#endif // _LIBCPP_MSVCRT<br>
> -using ::vsnprintf;<br>
> -using ::vsprintf;<br>
> -using ::fgetc;<br>
> -using ::fgets;<br>
> -using ::fputc;<br>
> -using ::fputs;<br>
> -using ::getc;<br>
> -using ::putc;<br>
> -using ::ungetc;<br>
> -using ::fread;<br>
> -using ::fwrite;<br>
> -using ::fgetpos;<br>
> -using ::fseek;<br>
> -using ::fsetpos;<br>
> -using ::ftell;<br>
> -using ::rewind;<br>
> -using ::clearerr;<br>
> -using ::feof;<br>
> -using ::ferror;<br>
> -using ::perror;<br>
> -<br>
> -#ifndef _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE<br>
> -using ::fopen;<br>
> -using ::freopen;<br>
> -using ::remove;<br>
> -using ::rename;<br>
> -using ::tmpfile;<br>
> -using ::tmpnam;<br>
> -#endif<br>
><br>
> -#ifndef _LIBCPP_HAS_NO_STDIN<br>
> -using ::getchar;<br>
> -#if _LIBCPP_STD_VER <= 11<br>
> -using ::gets;<br>
> #endif<br>
> -using ::scanf;<br>
> -using ::vscanf;<br>
> -#endif<br>
> -<br>
> -#ifndef _LIBCPP_HAS_NO_STDOUT<br>
> -using ::printf;<br>
> -using ::putchar;<br>
> -using ::puts;<br>
> -using ::vprintf;<br>
> -#endif<br>
> -<br>
> -_LIBCPP_END_NAMESPACE_STD<br>
><br>
> -#endif  // _LIBCPP_CSTDIO<br>
> +#endif  // _LIBCPP_STDIO_H<br>
><br>
> Modified: libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp<br>
> URL: <a href="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" rel="noreferrer" target="_blank">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</a><br>
> ==============================================================================<br>
> --- libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp (original)<br>
> +++ libcxx/trunk/test/std/depr/depr.c.headers/stdio_h.pass.cpp Thu Oct  8 20:29:09 2015<br>
> @@ -13,6 +13,26 @@<br>
> #include <type_traits><br>
> #include "test_macros.h"<br>
><br>
> +#ifdef getc<br>
> +#error getc is defined<br>
> +#endif<br>
> +<br>
> +#ifdef putc<br>
> +#error putc is defined<br>
> +#endif<br>
> +<br>
> +#ifdef clearerr<br>
> +#error clearerr is defined<br>
> +#endif<br>
> +<br>
> +#ifdef feof<br>
> +#error feof is defined<br>
> +#endif<br>
> +<br>
> +#ifdef ferror<br>
> +#error ferror is defined<br>
> +#endif<br>
> +<br>
> #ifndef BUFSIZ<br>
> #error BUFSIZ not defined<br>
> #endif<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>
</blockquote></div>