Skip to content

Commit

Permalink
[libc++][modules] Rewrite the modulemap to have fewer top-level modul…
Browse files Browse the repository at this point in the history
…es (llvm#107638)

This is a re-application of bc6bd3b which was reverted in f11abac
because it broke the Clang pre-commit CI.

Original commit message:

This patch rewrites the modulemap to have fewer top-level modules.
Previously, our modulemap had one top level module for each header in
the library, including private headers. This had the well-known problem
of making compilation times terrible, in addition to being somewhat
against the design principles of Clang modules.

This patch provides almost an order of magnitude compilation time
improvement when building modularized code (certainly subject to
variations). For example, including <ccomplex> without a module cache
went from 22.4 seconds to 1.6 seconds, a 14x improvement.

To achieve this, one might be tempted to simply put all the headers in a
single top-level module. Unfortunately, this doesn't work because libc++
provides C compatibility headers (e.g. stdlib.h) which create cycles
when the C Standard Library headers are modularized too. This is
especially tricky since base systems are usually not modularized: as far
as I know, only Xcode 16 beta contains a modularized SDK that makes this
issue visible. To understand it, imagine we have the following setup:

   // in libc++'s include/c++/v1/module.modulemap
   module std {
      header stddef.h
      header stdlib.h
   }

   // in the C library's include/module.modulemap
   module clib {
      header stddef.h
      header stdlib.h
   }

Now, imagine that the C library's <stdlib.h> includes <stddef.h>,
perhaps as an implementation detail. When building the `std` module,
libc++'s <stdlib.h> header does `#include_next <stdlib.h>` to get the C
library's <stdlib.h>, so libc++ depends on the `clib` module.

However, remember that the C library's <stdlib.h> header includes
<stddef.h> as an implementation detail. Since the header search paths
for libc++ are (and must be) before the search paths for the C library,
the C library ends up including libc++'s <stddef.h>, which means it
depends on the `std` module. That's a cycle.

To solve this issue, this patch creates one top-level module for each C
compatibility header. The rest of the libc++ headers are located in a
single top-level `std` module, with two main exceptions. First, the
module containing configuration headers (e.g. <__config>) has its own
top-level module too, because those headers are included by the C
compatibility headers.

Second, we create a top-level std_core module that contains several
dependency-free utilities used (directly or indirectly) from the __math
subdirectory. This is needed because __math pulls in a bunch of stuff,
and __math is used from the C compatibility header <math.h>.

As a direct benefit of this change, we don't need to generate an
artificial __std_clang_module header anymore to provide a monolithic
`std` module, since our modulemap does it naturally by construction.

A next step after this change would be to look into whether math.h
really needs to include the contents of __math, and if so, whether
libc++'s math.h truly needs to include the C library's math.h header.
Removing either dependency would break this annoying cycle.

Thanks to Eric Fiselier for pointing out this approach during a recent
meeting. This wasn't viable before some recent refactoring, but wrapping
everything (except the C headers) in a large module is by far the
simplest and the most effective way of doing this.

Fixes llvm#86193
  • Loading branch information
ldionne committed Sep 30, 2024
1 parent 18df9d2 commit 114cc54
Show file tree
Hide file tree
Showing 8 changed files with 2,184 additions and 2,350 deletions.
1 change: 0 additions & 1 deletion libcxx/include/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,6 @@ set(files
__ranges/views.h
__ranges/zip_view.h
__split_buffer
__std_clang_module
__std_mbstate_t.h
__stop_token/atomic_unique_lock.h
__stop_token/intrusive_list_view.h
Expand Down
1 change: 1 addition & 0 deletions libcxx/include/__format/formatter_integral.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <__type_traits/make_unsigned.h>
#include <__utility/unreachable.h>
#include <array>
#include <cstdint>
#include <limits>
#include <string>
#include <string_view>
Expand Down
193 changes: 0 additions & 193 deletions libcxx/include/__std_clang_module

This file was deleted.

Loading

0 comments on commit 114cc54

Please sign in to comment.