On Thu, Dec 31, 2015 at 10:52 AM, 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?
>>
>
> 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?
>
Ah, -ansi just means -std=c89, so passing both -std=gnu99 and -ansi doesn't
make a lot of sense. So not 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@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
