[libcxx] r249738 - Split <ctype.h> out of <cctype>.
Sean Silva via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 13 19:00:12 PDT 2015
On Tue, Oct 13, 2015 at 6:54 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>
> On Tue, Oct 13, 2015 at 6:14 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Tue, Oct 13, 2015 at 5:31 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>> On Tue, Oct 13, 2015 at 3:17 PM, Richard Smith via cfe-commits <
>>> cfe-commits at lists.llvm.org> wrote:
>>>
>>>> On Tue, Oct 13, 2015 at 2:10 PM, Adrian Prantl via cfe-commits <
>>>> cfe-commits at lists.llvm.org> wrote:
>>>>
>>>>> Hi Richard,
>>>>>
>>>>> this commit appears to break the module self-host on Darwin.
>>>>>
>>>>> When compiling the following program generated by clang’s own cmake
>>>>> script:
>>>>>
>>>>> > #undef NDEBUG
>>>>> > #include <cassert>
>>>>> > #define NDEBUG
>>>>> > #include <cassert>
>>>>> > int main() { assert(this code is not compiled); }
>>>>>
>>>>> with clang++ -std=c++11 -fmodules -fcxx-modules test.cpp
>>>>>
>>>>
>>>> (You don't need -fcxx-modules here.)
>>>>
>>>>
>>>>> I get
>>>>>
>>>>> > While building module 'std' imported from /Users/buildslave/adrian/
>>>>> llvm.org/_build.ninja.release/bin/../include/c++/v1/cassert:20:
>>>>> > While building module 'Darwin' imported from
>>>>> /Users/buildslave/adrian/
>>>>> llvm.org/_build.ninja.release/bin/../include/c++/v1/cstddef:39:
>>>>> > In file included from <module-includes>:98:
>>>>> > In file included from
>>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/wchar.h:92:
>>>>> > In file included from
>>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/_wctype.h:57:
>>>>
>>>> > /Users/buildslave/adrian/
>>>>> llvm.org/_build.ninja.release/bin/../include/c++/v1/ctype.h:33:10:
>>>>> fatal error: cyclic dependency in module 'std': std -> Darwin -> std
>>>>> > #include <__config>
>>>>> > ^
>>>>> > While building module 'std' imported from /Users/buildslave/adrian/
>>>>> llvm.org/_build.ninja.release/bin/../include/c++/v1/cassert:20:
>>>>> > In file included from <module-includes>:1:
>>>>> > In file included from /Users/buildslave/adrian/
>>>>> llvm.org/_build.ninja.release/bin/../include/c++/v1/algorithm:624:
>>>>> > In file included from /Users/buildslave/adrian/
>>>>> llvm.org/_build.ninja.release/bin/../include/c++/v1/initializer_list:47
>>>>> :
>>>>> > /Users/buildslave/adrian/
>>>>> llvm.org/_build.ninja.release/bin/../include/c++/v1/cstddef:39:10:
>>>>> fatal error: could not build module 'Darwin'
>>>>> > #include <stddef.h>
>>>>> > ~~~~~~~~^
>>>>> > In file included from test.cpp:2:
>>>>> > /Users/buildslave/adrian/
>>>>> llvm.org/_build.ninja.release/bin/../include/c++/v1/cassert:20:10:
>>>>> fatal error: could not build module 'std'
>>>>> > #include <__config>
>>>>> > ~~~~~~~~^
>>>>> > 3 errors generated.
>>>>>
>>>>> Let me know how I can help in diagnosing what’s going on here.
>>>>>
>>>>
>>>> OK, I see what's wrong. Is this working any better for you in r250236?
>>>>
>>>
>>> We're still seeing:
>>>
>>> While building module 'Darwin' imported from /usr/include/assert.h:42:
>>> While building module 'std' imported from
>>> /path/to/build_cmake/stage1/bin/../include/c++/v1/math.h:309:
>>>
>>
>> Argh, this is including <type_traits>, which is in the std module. Can
>> you try removing the header "type_traits" line from the libc++ module map
>> and see if that helps?
>>
>
> I've run into this issue in the past, and I don't think that will fix it
> (see below). Just to be sure, here is the output with type_traits removed
> from the module map::
>
> While building module 'Darwin' imported from /usr/include/assert.h:42:
> While building module 'std' imported from
> /path/to/build_cmake/stage1/bin/../include/c++/v1/type_traits:211:
> In file included from <module-includes>:1:
> In file included from
> /path/to/build_cmake/stage1/bin/../include/c++/v1/algorithm:624:
> In file included from
> /path/to/build_cmake/stage1/bin/../include/c++/v1/initializer_list:47:
> In file included from
> /path/to/build_cmake/stage1/bin/../include/c++/v1/cstddef:43:
> /path/to/build_cmake/stage1/bin/../include/c++/v1/stddef.h:46:15: fatal
> error: cyclic dependency in module 'Darwin': Darwin -> std -> Darwin
> #include_next <stddef.h>
> ^
> While building module 'Darwin' imported from /usr/include/assert.h:42:
> In file included from <module-includes>:80:
> In file included from
> /path/to/build_cmake/stage1/bin/../lib/clang/3.8.0/include/tgmath.h:29:
> In file included from
> /path/to/build_cmake/stage1/bin/../include/c++/v1/math.h:309:
> /path/to/build_cmake/stage1/bin/../include/c++/v1/type_traits:211:10:
> fatal error: could not build module 'std'
> #include <cstddef>
> ~~~~~~~~^
> In file included from modules.cpp:2:
> In file included from
> /path/to/build_cmake/stage1/bin/../include/c++/v1/cassert:21:
> /usr/include/assert.h:42:10: fatal error: could not build module 'Darwin'
> #include <sys/cdefs.h>
> ~~~~~~~~^
> 3 errors generated.
>
>
>
>> What we really need here is a way to get the Darwin blah.h headers to
>> only include each other, and not find the libc++ headers. Do you need
>> something that works with new libc++ and old Clang, or would a Clang
>> feature to prevent the Darwin module from finding the std module's headers
>> work for you?
>>
>
> I've run into this issue in the wild with the modularized PS4 SDK, so I
> think that a clang feature that prevents this is the best fit. I think I
> may have already mentioned this to you at one of the socials but just to
> have it in writing here for everybody else, the root cause of the issue is
> that `-Imylib` will cause mylib/assert.h to be selected for `#include
> <assert.h>`, even when building the system module. Hence some random user
> header will end up as being part of the system module and the seed for
> problems has been planted.
>
> If the random user header then ends up including code from a module that
> depends on the system module, then clang will go off and build this other
> module, which itself (say) depends on the system module, which triggers the
> recursive dependency error.
>
> There's nothing special about system headers for this issue, but due to
> their position at the root of the dependency tree they seem to be the ones
> affected in practice. The fundamental problem is that some random header
> outside the module is interposing for the header listed in the module map.
> I think the feature basically needs to be about making the module more
> "hermetically sealed" in this scenario. Off the top of my head, maybe
> something like putting the module map's directory at the front of the
> search path for includes? That seems sort of hacky. Any ideas?
>
Just noticed that I had somehow overlooked the first sentence of your last
paragraph. Apologies for the redundancy (although maybe the extra
explanation will be helpful for onlookers).
-- Sean Silva
>
> -- Sean Silva
>
>
>
>>
>>
>>> In file included from <module-includes>:1:
>>> In file included from
>>> /path/to/build_cmake/stage1/bin/../include/c++/v1/algorithm:624:
>>> In file included from
>>> /path/to/build_cmake/stage1/bin/../include/c++/v1/initializer_list:47:
>>> In file included from
>>> /path/to/build_cmake/stage1/bin/../include/c++/v1/cstddef:43:
>>> /path/to/build_cmake/stage1/bin/../include/c++/v1/stddef.h:46:15: fatal
>>> error: cyclic dependency in module 'Darwin': Darwin -> std -> Darwin
>>> #include_next <stddef.h>
>>> ^
>>> While building module 'Darwin' imported from /usr/include/assert.h:42:
>>> In file included from <module-includes>:80:
>>> In file included from
>>> /path/to/build_cmake/stage1/bin/../lib/clang/3.8.0/include/tgmath.h:29:
>>> /path/to/build_cmake/stage1/bin/../include/c++/v1/math.h:309:10: fatal
>>> error: could not build module 'std'
>>> #include <type_traits>
>>> ~~~~~~~~^
>>> In file included from modules.cpp:2:
>>> In file included from
>>> /path/to/build_cmake/stage1/bin/../include/c++/v1/cassert:21:
>>> /usr/include/assert.h:42:10: fatal error: could not build module 'Darwin'
>>> #include <sys/cdefs.h>
>>> ~~~~~~~~^
>>> 3 errors generated.
>>>
>>>
>>> Looks like the system headers are being interposed.
>>>
>>> -- Sean Silva
>>>
>>>
>>>>
>>>>
>>>>> Once this works, I’d like to to set up a green dragon bot that builds
>>>>> clang with LLVM_ENABLE_MODULES to catch similar problems earlier.
>>>>>
>>>>> -- adrian
>>>>>
>>>>> > On Oct 8, 2015, at 1:36 PM, Richard Smith via cfe-commits <
>>>>> cfe-commits at lists.llvm.org> wrote:
>>>>> >
>>>>> > Author: rsmith
>>>>> > Date: Thu Oct 8 15:36:30 2015
>>>>> > New Revision: 249738
>>>>> >
>>>>> > URL: http://llvm.org/viewvc/llvm-project?rev=249738&view=rev
>>>>> > Log:
>>>>> > Split <ctype.h> out of <cctype>.
>>>>> >
>>>>> > Added:
>>>>> > libcxx/trunk/include/ctype.h
>>>>> > - copied, changed from r249736, libcxx/trunk/include/cctype
>>>>> > Modified:
>>>>> > libcxx/trunk/include/cctype
>>>>> > libcxx/trunk/test/std/strings/c.strings/cctype.pass.cpp
>>>>> >
>>>>> > Modified: libcxx/trunk/include/cctype
>>>>> > URL:
>>>>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/cctype?rev=249738&r1=249737&r2=249738&view=diff
>>>>> >
>>>>> ==============================================================================
>>>>> > --- libcxx/trunk/include/cctype (original)
>>>>> > +++ libcxx/trunk/include/cctype Thu Oct 8 15:36:30 2015
>>>>> > @@ -37,10 +37,6 @@ int toupper(int c);
>>>>> >
>>>>> > #include <__config>
>>>>> > #include <ctype.h>
>>>>> > -#if defined(_LIBCPP_MSVCRT)
>>>>> > -#include "support/win32/support.h"
>>>>> > -#include "support/win32/locale_win32.h"
>>>>> > -#endif // _LIBCPP_MSVCRT
>>>>> >
>>>>> > #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
>>>>> > #pragma GCC system_header
>>>>> > @@ -48,33 +44,19 @@ int toupper(int c);
>>>>> >
>>>>> > _LIBCPP_BEGIN_NAMESPACE_STD
>>>>> >
>>>>> > -#undef isalnum
>>>>> > using ::isalnum;
>>>>> > -#undef isalpha
>>>>> > using ::isalpha;
>>>>> > -#undef isblank
>>>>> > using ::isblank;
>>>>> > -#undef iscntrl
>>>>> > using ::iscntrl;
>>>>> > -#undef isdigit
>>>>> > using ::isdigit;
>>>>> > -#undef isgraph
>>>>> > using ::isgraph;
>>>>> > -#undef islower
>>>>> > using ::islower;
>>>>> > -#undef isprint
>>>>> > using ::isprint;
>>>>> > -#undef ispunct
>>>>> > using ::ispunct;
>>>>> > -#undef isspace
>>>>> > using ::isspace;
>>>>> > -#undef isupper
>>>>> > using ::isupper;
>>>>> > -#undef isxdigit
>>>>> > using ::isxdigit;
>>>>> > -#undef tolower
>>>>> > using ::tolower;
>>>>> > -#undef toupper
>>>>> > using ::toupper;
>>>>> >
>>>>> > _LIBCPP_END_NAMESPACE_STD
>>>>> >
>>>>> > Copied: libcxx/trunk/include/ctype.h (from r249736,
>>>>> libcxx/trunk/include/cctype)
>>>>> > URL:
>>>>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/ctype.h?p2=libcxx/trunk/include/ctype.h&p1=libcxx/trunk/include/cctype&r1=249736&r2=249738&rev=249738&view=diff
>>>>> >
>>>>> ==============================================================================
>>>>> > --- libcxx/trunk/include/cctype (original)
>>>>> > +++ libcxx/trunk/include/ctype.h Thu Oct 8 15:36:30 2015
>>>>> > @@ -1,5 +1,5 @@
>>>>> > // -*- C++ -*-
>>>>> > -//===---------------------------- cctype
>>>>> ----------------------------------===//
>>>>> > +//===---------------------------- ctype.h
>>>>> ---------------------------------===//
>>>>> > //
>>>>> > // The LLVM Compiler Infrastructure
>>>>> > //
>>>>> > @@ -8,14 +8,11 @@
>>>>> > //
>>>>> >
>>>>> //===----------------------------------------------------------------------===//
>>>>> >
>>>>> > -#ifndef _LIBCPP_CCTYPE
>>>>> > -#define _LIBCPP_CCTYPE
>>>>> > +#ifndef _LIBCPP_CTYPE_H
>>>>> > +#define _LIBCPP_CTYPE_H
>>>>> >
>>>>> > /*
>>>>> > - cctype synopsis
>>>>> > -
>>>>> > -namespace std
>>>>> > -{
>>>>> > + ctype.h synopsis
>>>>> >
>>>>> > int isalnum(int c);
>>>>> > int isalpha(int c);
>>>>> > @@ -31,52 +28,41 @@ int isupper(int c);
>>>>> > int isxdigit(int c);
>>>>> > int tolower(int c);
>>>>> > int toupper(int c);
>>>>> > -
>>>>> > -} // std
>>>>> > */
>>>>> >
>>>>> > #include <__config>
>>>>> > -#include <ctype.h>
>>>>> > -#if defined(_LIBCPP_MSVCRT)
>>>>> > -#include "support/win32/support.h"
>>>>> > -#include "support/win32/locale_win32.h"
>>>>> > -#endif // _LIBCPP_MSVCRT
>>>>> > +#include_next <ctype.h>
>>>>> >
>>>>> > #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
>>>>> > #pragma GCC system_header
>>>>> > #endif
>>>>> >
>>>>> > -_LIBCPP_BEGIN_NAMESPACE_STD
>>>>> > +#ifdef __cplusplus
>>>>> > +
>>>>> > +#if defined(_LIBCPP_MSVCRT)
>>>>> > +// We support including .h headers inside 'extern "C"' contexts, so
>>>>> switch
>>>>> > +// back to C++ linkage before including these C++ headers.
>>>>> > +extern "C++" {
>>>>> > + #include "support/win32/support.h"
>>>>> > + #include "support/win32/locale_win32.h"
>>>>> > +}
>>>>> > +#endif // _LIBCPP_MSVCRT
>>>>> >
>>>>> > #undef isalnum
>>>>> > -using ::isalnum;
>>>>> > #undef isalpha
>>>>> > -using ::isalpha;
>>>>> > #undef isblank
>>>>> > -using ::isblank;
>>>>> > #undef iscntrl
>>>>> > -using ::iscntrl;
>>>>> > #undef isdigit
>>>>> > -using ::isdigit;
>>>>> > #undef isgraph
>>>>> > -using ::isgraph;
>>>>> > #undef islower
>>>>> > -using ::islower;
>>>>> > #undef isprint
>>>>> > -using ::isprint;
>>>>> > #undef ispunct
>>>>> > -using ::ispunct;
>>>>> > #undef isspace
>>>>> > -using ::isspace;
>>>>> > #undef isupper
>>>>> > -using ::isupper;
>>>>> > #undef isxdigit
>>>>> > -using ::isxdigit;
>>>>> > #undef tolower
>>>>> > -using ::tolower;
>>>>> > #undef toupper
>>>>> > -using ::toupper;
>>>>> >
>>>>> > -_LIBCPP_END_NAMESPACE_STD
>>>>> > +#endif
>>>>> >
>>>>> > -#endif // _LIBCPP_CCTYPE
>>>>> > +#endif // _LIBCPP_CTYPE_H
>>>>> >
>>>>> > Modified: libcxx/trunk/test/std/strings/c.strings/cctype.pass.cpp
>>>>> > URL:
>>>>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/c.strings/cctype.pass.cpp?rev=249738&r1=249737&r2=249738&view=diff
>>>>> >
>>>>> ==============================================================================
>>>>> > --- libcxx/trunk/test/std/strings/c.strings/cctype.pass.cpp
>>>>> (original)
>>>>> > +++ libcxx/trunk/test/std/strings/c.strings/cctype.pass.cpp Thu Oct
>>>>> 8 15:36:30 2015
>>>>> > @@ -86,18 +86,18 @@ int main()
>>>>> > static_assert((std::is_same<decltype(std::tolower(0)),
>>>>> int>::value), "");
>>>>> > static_assert((std::is_same<decltype(std::toupper(0)),
>>>>> int>::value), "");
>>>>> >
>>>>> > - assert(isalnum('a'));
>>>>> > - assert(isalpha('a'));
>>>>> > - assert(isblank(' '));
>>>>> > - assert(!iscntrl(' '));
>>>>> > - assert(!isdigit('a'));
>>>>> > - assert(isgraph('a'));
>>>>> > - assert(islower('a'));
>>>>> > - assert(isprint('a'));
>>>>> > - assert(!ispunct('a'));
>>>>> > - assert(!isspace('a'));
>>>>> > - assert(!isupper('a'));
>>>>> > - assert(isxdigit('a'));
>>>>> > - assert(tolower('A') == 'a');
>>>>> > - assert(toupper('a') == 'A');
>>>>> > + assert(std::isalnum('a'));
>>>>> > + assert(std::isalpha('a'));
>>>>> > + assert(std::isblank(' '));
>>>>> > + assert(!std::iscntrl(' '));
>>>>> > + assert(!std::isdigit('a'));
>>>>> > + assert(std::isgraph('a'));
>>>>> > + assert(std::islower('a'));
>>>>> > + assert(std::isprint('a'));
>>>>> > + assert(!std::ispunct('a'));
>>>>> > + assert(!std::isspace('a'));
>>>>> > + assert(!std::isupper('a'));
>>>>> > + assert(std::isxdigit('a'));
>>>>> > + assert(std::tolower('A') == 'a');
>>>>> > + assert(std::toupper('a') == 'A');
>>>>> > }
>>>>> >
>>>>> >
>>>>> > _______________________________________________
>>>>> > 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/20151013/c032a753/attachment-0001.html>
More information about the cfe-commits
mailing list