> On Oct 14, 2015, at 11:37 AM, Adrian Prantl via cfe-commits
> <cfe-commits@lists.llvm.org> wrote:
>
>>
>> On Oct 13, 2015, at 7:43 PM, Richard Smith <rich...@metafoo.co.uk
>> <mailto: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?
To answer my previous question, removing the type_traits module allows us to
finish all the cmake checks, but it then fails when building the LLVM_Utils
module:
While building module 'LLVM_Utils' imported from ../lib/Support/APFloat.cpp:15:
In file included from <module-includes>:1:
In file included from ../include/llvm/ADT/APFloat.h:20:
In file included from ../include/llvm/ADT/APInt.h:19:
In file included from ../include/llvm/ADT/ArrayRef.h:14:
In file included from ../include/llvm/ADT/SmallVector.h:17:
../include/llvm/ADT/iterator_range.h:63:36: error: declaration of 'declval'
must be imported from module 'Darwin.C.tgmath' before it is required
iterator_range<decltype(begin(std::declval<T>()))> drop_begin(T &&t, int n) {
^
/Users/buildslave/adrian/llvm.org/_build.ninja.release/bin/../include/c++/v1/type_traits:700:1:
note: previous declaration is here
declval() _NOEXCEPT;
^
While building module 'LLVM_Utils' imported from ../lib/Support/APFloat.cpp:15:
In file included from <module-includes>:1:
In file included from ../include/llvm/ADT/APFloat.h:20:
In file included from ../include/llvm/ADT/APInt.h:19:
In file included from ../include/llvm/ADT/ArrayRef.h:14:
In file included from ../include/llvm/ADT/SmallVector.h:20:
In file included from ../include/llvm/Support/MathExtras.h:21:
/Users/buildslave/adrian/llvm.org/_build.ninja.release/bin/../include/c++/v1/type_traits:220:8:
error: redefinition of '__void_t'
struct __void_t { typedef void type; };
^
/Users/buildslave/adrian/llvm.org/_build.ninja.release/bin/../include/c++/v1/type_traits:220:8:
note: previous definition is here
struct __void_t { typedef void type; };
^
If there is a way to prevent Clang from going into std when building Darwin, it
looks like that’d be the way to go.
-- 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 <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
