> On Oct 13, 2015, at 7:43 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > > On Tue, Oct 13, 2015 at 6:54 PM, Sean Silva <chisophu...@gmail.com > <mailto:chisophu...@gmail.com>> wrote: > On Tue, Oct 13, 2015 at 6:14 PM, Richard Smith <rich...@metafoo.co.uk > <mailto:rich...@metafoo.co.uk>> wrote: > On Tue, Oct 13, 2015 at 5:31 PM, Sean Silva <chisophu...@gmail.com > <mailto:chisophu...@gmail.com>> wrote: > On Tue, Oct 13, 2015 at 3:17 PM, Richard Smith via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: > On Tue, Oct 13, 2015 at 2:10 PM, Adrian Prantl via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@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 > > <http://llvm.org/_build.ninja.release/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 > > <http://llvm.org/_build.ninja.release/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 > > <http://llvm.org/_build.ninja.release/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 > > <http://llvm.org/_build.ninja.release/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 > > <http://llvm.org/_build.ninja.release/include/c++/v1/algorithm:624>: > > In file included from > > /Users/buildslave/adrian/llvm.org/_build.ninja.release/bin/../include/c++/v1/initializer_list:47 > > <http://llvm.org/_build.ninja.release/include/c++/v1/initializer_list:47>: > > /Users/buildslave/adrian/llvm.org/_build.ninja.release/bin/../include/c++/v1/cstddef:39:10 > > <http://llvm.org/_build.ninja.release/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 > > <http://llvm.org/_build.ninja.release/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: > > That's an include of <cstddef>. We'd need to apply this workaround to that > header too (but I think the buck stops there).
After commenting out the cstddef module as well, the test case compiles again. Should I commit that and the type_traits change to libcxx, or are you working on a better fix? -- adrian > > 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? > > I was thinking: if we found the module map for a module in some include path, > then we should build that module with only the directories from the header > search path at or before that directory in the list of include dirs. That's > probably a fairly simple change. > > -- 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@lists.llvm.org <mailto:cfe-commits@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 > > <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 > > > > <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 > > > > <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 > > > > <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@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > <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
