The headers that include_next compiler and OS headers need to be in different top level modules in order to avoid module cycles. e.g. libc++'s stdlib.h will #include_next stdlib.h from the compiler and then the C library. Either of those are likely to include stddef.h, which will come back up to the libc++ module map and create a module cycle. Putting stdlib.h and stddef.h (and the rest of the C standard library headers) in top level modules resolves this by letting the order go cxx_stdlib_h -> os_stdlib_h -> cxx_stddef_h -> os_stddef_h.
All of those headers' dependencies then need to be moved into top level modules themselves to avoid module cycles between the new top level level cstd modules. This starts to get complicated, as the libc++ C headers, by standard, have to include many of the C++ headers, which include the private detail headers, which are intertwined. e.g. some __algorithm headers include __memory headers and vice versa.
Make top level modules for all of the libc++ headers to easily guarantee that the modules aren't cyclic.
Add enough module exports to fix check-cxx and run-buildbot generic-modules.
__stop_token/intrusive_shared_ptr.h uses __atomic/atomic.h but has no include path to it. Add that include. math.h absorbs bits/atomic_wide_counter.h on some platforms that don't have modules, work around that by including math.h in __threading_support.
<mutex> doesn't actually require threads, there are a few pieces like once_flag that work without threads. Remove the requirement from its module.
AIX is no longer able to support modular builds.
Fix a syntax error in the json file (forgot to enclose a string in quotes)
Make the module map generator script compatible with Python 3.8 (use typing.List[xxx] instead of list[xxx])
Dump stderr when the clang command is failing to get header includes
We had a meeting at Apple today including @ldionne and he's of the opinion that this script will be too difficult to maintain. We agreed to keep the static module map, but split it so that every header is a top level module. Depending on how that approach tests, especially with build performance, we might come back to the script, so I'm trying to run it through the CI just to make sure it runs properly on the builders with some older Python versions. Plus the other CI errors are from splitting the module and not necessarily from the specifics of how they end up split. I'll try to get this updated for the static module map tomorrow anyway.
No, this is only necessary when the mega module gets split. This one comes from a unit test, and I'm not sure how practical it is. I don't know how many people are trying to use libc++ with clang modules on OSes that don't have any clang modules in the OS headers. It's not really a supported configuration, clang modules have the expectation that modular headers only include modular headers.
What happens today is that libc++ has a single module, and if nothing in the OS has a module than the libc++ module will absorb all of the OS headers it includes into one huge gigantic mega module that has the whole world in it. In that case, everything can see everything else. But when the mega module breaks up, the non-modular OS headers will be randomly assigned, in this case the CI indicates that math.h is absorbing pthread.h/atomic_wide_counter.h
Thanks for all the work on this. I know this has been extremely difficult: this is the worst kind of change since all other changes conflict with it, but you kept moving forward and you're finally there.
Please merge this change as soon as possible. It is probable that other changes will conflict with it in the meantime. If someone gets here because of a failed CI or similar, please let us know but let us handle this without eagerly reverting. There is no way to merge this change but to accept that we might break the trunk for a few hours, otherwise we'd have to basically halt all other libc++ development while we wait for this to go through CI and land.
@iana So please land this ASAP and monitor this thread. I'll also keep an eye out for any issues in the next few hours.
This revision was not accepted when it landed; it landed in state Needs Review.Jul 20 2023, 12:49 PM
If someone gets here because of a failed CI or similar, please let us know but let us handle this without eagerly reverting. There is no way to merge this change but to accept that we might break the trunk for a few hours, otherwise we'd have to basically halt all other libc++ development while we wait for this to go through CI and land.
I missed it last week, but this did break the LLDB bots:
If someone gets here because of a failed CI or similar, please let us know but let us handle this without eagerly reverting. There is no way to merge this change but to accept that we might break the trunk for a few hours, otherwise we'd have to basically halt all other libc++ development while we wait for this to go through CI and land.
I missed it last week, but this did break the LLDB bots:
Has this been part of a separate patch?