Commit mostly working single wrapped module map #87402
Conversation
|
We can't do this. The libc++ public headers all need to be top level modules so that they can layer with the clang and OS modules. https://reviews.llvm.org/D144322 Note that you can't use modules with only the libc++ module map, you'll need a module map to cover your libc headers and the posix headers used by libc++ as well. |
We can and have done this. I suspect the issues you're discussing arise on OS X and not Linux? I have built LLVM with this change using modules, without any special tweaks for libc or POSIX. So this change works perfectly in some configurations. If you could provide concrete examples of what breaks, I can work to fix those breakages. But without actual meaningful examples, I can't do much to help. I understand that this patch, exactly as is, might cause issues with C and POSIX headers. And that those issues may require the We also can't not do this. When was the last time you built a large project using our module map? It very quickly breaks trying to build Clang with more than one thread. So if this change breaks on a Mac, but is required on Linux and visa versa without this change, then I can come up with a separate path forward. |
|
@ian-twilightcoder It's also worth noting that this change turns a number of |
It will only work if the libc++ module map is the only module map on the system, or if you start adding Once you do have proper modules for the C headers, they quickly introduce cycles. i.e. inttypes.h includes stdint.h. If those are in the same module in libc++ then you get a module cycle: std -> c_inttypes -> std -> c_stdint. Single giant module just doesn't work, and it's not an Apple SDK specific problem, it's any system that actually supports clang modules. |
What systems support Clang modules? What can I build today, on Linux using Clang modules? Could you please try building LLVM with modules enabled I'm frustrated by your claims that differ entirely from my lived experience using Clang modules. I would like to work with you, but I'm unable to experimentally verify your claims. Please help me do that. |
Out of the box only the Apple SDK supports clang modules. Swift adds module maps to Linux and Windows (and soon Android and musl) via vfs, though admittedly some of those module maps have similar issues.
@vsapsai didn't you try this recently? I remember it came up at the team meeting a few weeks ago at least...
This change will definitely break on Apple systems, though the ones in CI are too old to exhibit the breakage. |
|
@ian-twilightcoder You can stand down on the example until I fix the test suite breakages. My apologies for asking for too much too soon. |
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Eric (EricWF) ChangesThis patch is what I use to use modules on Linux when building LLVM. Without it there are race conditions around the building of each individual module. This patch also removes two module declarations for modules which don't exist Patch is 237.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/87402.diff 1 Files Affected:
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index 22c380327f1579..f7eac09703eaf5 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -1,2101 +1,2103 @@
// Main C++ standard library interfaces
-module std_algorithm [system] {
- header "algorithm"
- export *
-}
-module std_any [system] {
- header "any"
- export *
-}
-module std_array [system] {
- header "array"
- export *
-}
-module std_atomic [system] {
- header "atomic"
- export *
-}
-module std_barrier [system] {
- header "barrier"
- export *
-}
-module std_bit [system] {
- header "bit"
- export *
-}
-module std_bitset [system] {
- header "bitset"
- export *
-}
-module std_charconv [system] {
- header "charconv"
- export *
-}
-module std_chrono [system] {
- header "chrono"
- export *
-}
-module std_codecvt [system] {
- header "codecvt"
- export *
-}
-module std_compare [system] {
- header "compare"
- export *
-}
-module std_complex [system] {
- header "complex"
- export *
-}
-module std_concepts [system] {
- header "concepts"
- export *
-}
-module std_condition_variable [system] {
- header "condition_variable"
- export *
-}
-module std_coroutine [system] {
- header "coroutine"
- export *
-}
-module std_deque [system] {
- header "deque"
- export *
-}
-module std_exception [system] {
- header "exception"
- export *
-}
-module std_execution [system] {
- header "execution"
- export *
-}
-module std_expected [system] {
- header "expected"
- export *
-}
-module std_filesystem [system] {
- header "filesystem"
- export *
-}
-module std_format [system] {
- header "format"
- export *
-}
-module std_forward_list [system] {
- header "forward_list"
- export *
-}
-module std_fstream [system] {
- header "fstream"
- export *
-}
-module std_functional [system] {
- header "functional"
- export *
-}
-module std_future [system] {
- header "future"
- export *
-}
-module std_initializer_list [system] {
- header "initializer_list"
- export *
-}
-module std_iomanip [system] {
- header "iomanip"
- export *
-}
-module std_ios [system] {
- header "ios"
- export *
-}
-module std_iosfwd [system] {
- header "iosfwd"
- export *
-}
-module std_iostream [system] {
- header "iostream"
- export *
-}
-module std_istream [system] {
- header "istream"
- export *
-}
-module std_iterator [system] {
- header "iterator"
- export *
-}
-module std_latch [system] {
- header "latch"
- export *
-}
-module std_limits [system] {
- header "limits"
- export *
-}
-module std_list [system] {
- header "list"
- export *
-}
-module std_locale [system] {
- header "locale"
- export *
-}
-module std_map [system] {
- header "map"
- export *
-}
-module std_mdspan [system] {
- header "mdspan"
- export *
-}
-module std_memory [system] {
- header "memory"
- export *
-}
-module std_memory_resource [system] {
- header "memory_resource"
- export *
-}
-module std_mutex [system] {
- header "mutex"
- export *
-}
-module std_new [system] {
- header "new"
- export *
-}
-module std_numbers [system] {
- header "numbers"
- export *
-}
-module std_numeric [system] {
- header "numeric"
- export *
-}
-module std_optional [system] {
- header "optional"
- export *
-}
-module std_ostream [system] {
- header "ostream"
- export *
-}
-module std_print [system] {
- header "print"
- export *
-}
-module std_queue [system] {
- header "queue"
- export *
-}
-module std_random [system] {
- header "random"
- export *
-}
-module std_ranges [system] {
- header "ranges"
- export *
-}
-module std_ratio [system] {
- header "ratio"
- export *
-}
-module std_regex [system] {
- header "regex"
- export *
-}
-module std_scoped_allocator [system] {
- header "scoped_allocator"
- export *
-}
-module std_semaphore [system] {
- header "semaphore"
- export *
-}
-module std_set [system] {
- header "set"
- export *
-}
-module std_shared_mutex [system] {
- header "shared_mutex"
- export std_version
-}
-module std_source_location [system] {
- header "source_location"
- export *
-}
-module std_span [system] {
- header "span"
- export std_private_ranges_enable_borrowed_range
- export std_version
- export std_private_span_span_fwd
-}
-module std_sstream [system] {
- header "sstream"
- export *
-}
-module std_stack [system] {
- header "stack"
- export *
-}
-module std_stdexcept [system] {
- header "stdexcept"
- export *
-}
-module std_stop_token {
- header "stop_token"
- export *
-}
-module std_streambuf [system] {
- header "streambuf"
- export *
-}
-module std_string [system] {
- header "string"
- export *
-}
-module std_string_view [system] {
- header "string_view"
- export *
-}
-module std_strstream [system] {
- header "strstream"
- export *
-}
-module std_syncstream [system] {
- header "syncstream"
- export *
-}
-module std_system_error [system] {
- header "system_error"
- export *
-}
-module std_thread [system] {
- header "thread"
- export *
-}
-module std_tuple [system] {
- header "tuple"
- export *
-}
-module std_type_traits [system] {
- header "type_traits"
- export *
-}
-module std_typeindex [system] {
- header "typeindex"
- export *
-}
-module std_typeinfo [system] {
- header "typeinfo"
- export *
-}
-module std_unordered_map [system] {
- header "unordered_map"
- export *
-}
-module std_unordered_set [system] {
- header "unordered_set"
- export *
-}
-module std_utility [system] {
- header "utility"
- export *
-}
-module std_valarray [system] {
- header "valarray"
- export *
-}
-module std_variant [system] {
- header "variant"
- export *
-}
-module std_vector [system] {
- header "vector"
- export *
-}
-module std_version [system] {
- header "version"
- export *
+module std_config [system] [extern_c] {
+ header "__config"
}
-// C standard library interface wrappers
-module std_cassert [system] {
- // <cassert>'s use of NDEBUG requires textual inclusion.
- textual header "cassert"
-}
-module std_ccomplex [system] {
- header "ccomplex"
- export *
-}
-module std_cctype [system] {
- header "cctype"
- export *
-}
-module std_cerrno [system] {
- header "cerrno"
- export *
-}
-module std_cfenv [system] {
- header "cfenv"
- export *
-}
-module std_cfloat [system] {
- header "cfloat"
- export *
-}
-module std_cinttypes [system] {
- header "cinttypes"
- export *
-}
-module std_ciso646 [system] {
- header "ciso646"
- export *
-}
-module std_climits [system] {
- header "climits"
- export *
-}
-module std_clocale [system] {
- header "clocale"
- export *
-}
-module std_cmath [system] {
- header "cmath"
- export *
-}
-module std_csetjmp [system] {
- header "csetjmp"
- export *
-}
-module std_csignal [system] {
- header "csignal"
- export *
-}
-// FIXME: <cstdalign> is missing.
-module std_cstdarg [system] {
- header "cstdarg"
- export *
-}
-module std_cstdbool [system] {
- header "cstdbool"
- export *
-}
-module std_cstddef [system] {
- header "cstddef"
- export *
-}
-module std_cstdint [system] {
- header "cstdint"
- export *
-}
-module std_cstdio [system] {
- header "cstdio"
- export *
-}
-module std_cstdlib [system] {
- header "cstdlib"
- export *
-}
-module std_cstring [system] {
- header "cstring"
- export *
-}
-module std_ctgmath [system] {
- header "ctgmath"
- export *
-}
-module std_ctime [system] {
- header "ctime"
- export *
-}
-module std_cuchar [system] {
- header "cuchar"
- export *
-}
-module std_cwchar [system] {
- header "cwchar"
- export *
-}
-module std_cwctype [system] {
- header "cwctype"
- export *
-}
+module std [system] {
+ module std_algorithm [system] {
+ header "algorithm"
+ export *
+ }
+ module std_any [system] {
+ header "any"
+ export *
+ }
+ module std_array [system] {
+ header "array"
+ export *
+ }
+ module std_atomic [system] {
+ header "atomic"
+ export *
+ }
+ module std_barrier [system] {
+ header "barrier"
+ export *
+ }
+ module std_bit [system] {
+ header "bit"
+ export *
+ }
+ module std_bitset [system] {
+ header "bitset"
+ export *
+ }
+ module std_charconv [system] {
+ header "charconv"
+ export *
+ }
+ module std_chrono [system] {
+ header "chrono"
+ export *
+ }
+ module std_codecvt [system] {
+ header "codecvt"
+ export *
+ }
+ module std_compare [system] {
+ header "compare"
+ export *
+ }
+ module std_complex [system] {
+ header "complex"
+ export *
+ }
+ module std_concepts [system] {
+ header "concepts"
+ export *
+ }
+ module std_condition_variable [system] {
+ header "condition_variable"
+ export *
+ }
+ module std_coroutine [system] {
+ header "coroutine"
+ export *
+ }
+ module std_deque [system] {
+ header "deque"
+ export *
+ }
+ module std_exception [system] {
+ header "exception"
+ export *
+ }
+ module std_execution [system] {
+ header "execution"
+ export *
+ }
+ module std_expected [system] {
+ header "expected"
+ export *
+ }
+ module std_filesystem [system] {
+ header "filesystem"
+ export *
+ }
+ module std_format [system] {
+ header "format"
+ export *
+ }
+ module std_forward_list [system] {
+ header "forward_list"
+ export *
+ }
+ module std_fstream [system] {
+ header "fstream"
+ export *
+ }
+ module std_functional [system] {
+ header "functional"
+ export *
+ }
+ module std_future [system] {
+ header "future"
+ export *
+ }
+ module std_initializer_list [system] {
+ header "initializer_list"
+ export *
+ }
+ module std_iomanip [system] {
+ header "iomanip"
+ export *
+ }
+ module std_ios [system] {
+ header "ios"
+ export *
+ }
+ module std_iosfwd [system] {
+ header "iosfwd"
+ export *
+ }
+ module std_iostream [system] {
+ header "iostream"
+ export *
+ }
+ module std_istream [system] {
+ header "istream"
+ export *
+ }
+ module std_iterator [system] {
+ header "iterator"
+ export *
+ }
+ module std_latch [system] {
+ header "latch"
+ export *
+ }
+ module std_limits [system] {
+ header "limits"
+ export *
+ }
+ module std_list [system] {
+ header "list"
+ export *
+ }
+ module std_locale [system] {
+ header "locale"
+ export *
+ }
+ module std_map [system] {
+ header "map"
+ export *
+ }
+ module std_mdspan [system] {
+ header "mdspan"
+ export *
+ }
+ module std_memory [system] {
+ header "memory"
+ export *
+ }
+ module std_memory_resource [system] {
+ header "memory_resource"
+ export *
+ }
+ module std_mutex [system] {
+ header "mutex"
+ export *
+ }
+ module std_new [system] {
+ header "new"
+ export *
+ }
+ module std_numbers [system] {
+ header "numbers"
+ export *
+ }
+ module std_numeric [system] {
+ header "numeric"
+ export *
+ }
+ module std_optional [system] {
+ header "optional"
+ export *
+ }
+ module std_ostream [system] {
+ header "ostream"
+ export *
+ }
+ module std_print [system] {
+ header "print"
+ export *
+ }
+ module std_queue [system] {
+ header "queue"
+ export *
+ }
+ module std_random [system] {
+ header "random"
+ export *
+ }
+ module std_ranges [system] {
+ header "ranges"
+ export *
+ }
+ module std_ratio [system] {
+ header "ratio"
+ export *
+ }
+ module std_regex [system] {
+ header "regex"
+ export *
+ }
+ module std_scoped_allocator [system] {
+ header "scoped_allocator"
+ export *
+ }
+ module std_semaphore [system] {
+ header "semaphore"
+ export *
+ }
+ module std_set [system] {
+ header "set"
+ export *
+ }
+ module std_shared_mutex [system] {
+ header "shared_mutex"
+ export std_version
+ }
+ module std_source_location [system] {
+ header "source_location"
+ export *
+ }
+ module std_span [system] {
+ header "span"
+ export std_private_ranges_enable_borrowed_range
+ export std_version
+ export std_private_span_span_fwd
+ }
+ module std_sstream [system] {
+ header "sstream"
+ export *
+ }
+ module std_stack [system] {
+ header "stack"
+ export *
+ }
+ module std_stdexcept [system] {
+ header "stdexcept"
+ export *
+ }
+ module std_stop_token {
+ header "stop_token"
+ export *
+ }
+ module std_streambuf [system] {
+ header "streambuf"
+ export *
+ }
+ module std_string [system] {
+ header "string"
+ export *
+ }
+ module std_string_view [system] {
+ header "string_view"
+ export *
+ }
+ module std_strstream [system] {
+ header "strstream"
+ export *
+ }
+ module std_syncstream [system] {
+ header "syncstream"
+ export *
+ }
+ module std_system_error [system] {
+ header "system_error"
+ export *
+ }
+ module std_thread [system] {
+ header "thread"
+ export *
+ }
+ module std_tuple [system] {
+ header "tuple"
+ export *
+ }
+ module std_type_traits [system] {
+ header "type_traits"
+ export *
+ }
+ module std_typeindex [system] {
+ header "typeindex"
+ export *
+ }
+ module std_typeinfo [system] {
+ header "typeinfo"
+ export *
+ }
+ module std_unordered_map [system] {
+ header "unordered_map"
+ export *
+ }
+ module std_unordered_set [system] {
+ header "unordered_set"
+ export *
+ }
+ module std_utility [system] {
+ header "utility"
+ export *
+ }
+ module std_valarray [system] {
+ header "valarray"
+ export *
+ }
+ module std_variant [system] {
+ header "variant"
+ export *
+ }
+ module std_vector [system] {
+ header "vector"
+ export *
+ }
+ module std_version [system] {
+ header "version"
+ export *
+ }
-// C standard library interfaces augmented/replaced in C++
-// <assert.h> provided by C library.
-module std_complex_h [system] {
- header "complex.h"
- export *
-}
-module std_ctype_h [system] {
- header "ctype.h"
- export *
-}
-module std_errno_h [system] {
- header "errno.h"
- export *
-}
-module std_fenv_h [system] {
- header "fenv.h"
- export *
-}
-module std_float_h [system] {
- header "float.h"
- export *
-}
-module std_inttypes_h [system] {
- header "inttypes.h"
- export *
-}
-// <iso646.h> provided by compiler.
-module std_locale_h [system] {
- header "locale.h"
- export *
-}
-module std_math_h [system] {
- header "math.h"
- export *
-}
-// <setjmp.h> provided by C library.
-// <signal.h> provided by C library.
-// FIXME: <stdalign.h> is missing.
-// <stdarg.h> provided by compiler.
-module std_stdatomic_h [system] {
- header "stdatomic.h"
- export *
-}
-module std_stdbool_h [system] {
- // <stdbool.h>'s __bool_true_false_are_defined macro requires textual inclusion.
- textual header "stdbool.h"
- export *
-}
-module std_stddef_h [system] {
- // <stddef.h>'s __need_* macros require textual inclusion.
- textual header "stddef.h"
- export *
-}
-module std_stdint_h [system] {
- header "stdint.h"
- export *
-}
-module std_stdio_h [system] {
- // <stdio.h>'s __need_* macros require textual inclusion.
- textual header "stdio.h"
- export *
-}
-module std_stdlib_h [system] {
- // <stdlib.h>'s __need_* macros require textual inclusion.
- textual header "stdlib.h"
- export *
-}
-module std_string_h [system] {
- header "string.h"
- export *
-}
-module std_tgmath_h [system] {
- header "tgmath.h"
- export *
-}
-module std_uchar_h [system] {
- header "uchar.h"
- export *
-}
-// <time.h> provided by C library.
-module std_wchar_h [system] {
- // <wchar.h>'s __need_* macros require textual inclusion.
- textual header "wchar.h"
- export *
-}
-module std_wctype_h [system] {
- header "wctype.h"
- export *
-}
+ // C standard library interface wrappers
+ module std_cassert [system] {
+ // <cassert>'s use of NDEBUG requires textual inclusion.
+ textual header "cassert"
+ }
+ module std_ccomplex [system] {
+ header "ccomplex"
+ export *
+ }
+ module std_cctype [system] {
+ header "cctype"
+ export *
+ }
+ module std_cerrno [system] {
+ header "cerrno"
+ export *
+ }
+ module std_cfenv [system] {
+ header "cfenv"
+ export *
+ }
+ module std_cfloat [system] {
+ header "cfloat"
+ export *
+ }
+ module std_cinttypes [system] {
+ header "cinttypes"
+ export *
+ }
+ module std_ciso646 [system] {
+ header "ciso646"
+ export *
+ }
+ module std_climits [system] {
+ header "climits"
+ export *
+ }
+ module std_clocale [system] {
+ header "clocale"
+ export *
+ }
+ module std_cmath [system] {
+ header "cmath"
+ export *
+ }
+ module std_csetjmp [system] {
+ header "csetjmp"
+ export *
+ }
+ module std_csignal [system] {
+ header "csignal"
+ export *
+ }
+ // FIXME: <cstdalign> is missing.
+ module std_cstdarg [system] {
+ header "cstdarg"
+ export *
+ }
+ module std_cstdbool [system] {
+ header "cstdbool"
+ export *
+ }
+ module std_cstddef [system] {
+ header "cstddef"
+ export *
+ }
+ module std_cstdint [system] {
+ header "cstdint"
+ export *
+ }
+ module std_cstdio [system] {
+ header "cstdio"
+ export *
+ }
+ module std_cstdlib [system] {
+ header "cstdlib"
+ export *
+ }
+ module std_cstring [system] {
+ header "cstring"
+ export *
+ }
+ module std_ctgmath [system] {
+ header "ctgmath"
+ export *
+ }
+ module std_ctime [system] {
+ header "ctime"
+ export *
+ }
+ module std_cuchar [system] {
+ header "cuchar"
+ export *
+ }
+ module std_cwchar [system] {
+ header "cwchar"
+ export *
+ }
+ module std_cwctype [system] {
+ header "cwctype"
+ export *
+ }
-// Experimental C++ standard library interfaces
-module std_experimental [system] {
- module iterator {
- header "experimental/iterator"
- export *
- }
- module memory {
- header "experimental/memory"
- export *
- }
- module propagate_const {
- header "experimental/propagate_const"
- export *
- }
- module simd {
- module aligned_tag { private header "experimental/__simd/aligned_tag.h" }
- module declaration { private header "experimental/__simd/declaration.h" }
- module reference { private header "experimental/__simd/reference.h" }
- module scalar { private header "experimental/__simd/scalar.h" }
- module simd { private header "experimental/__simd/simd.h" }
- module simd_mask { private header "experimental/__simd/simd_mask.h" }
- module traits { private header "experimental/__simd/traits.h" }
- module utility { private header "experimental/__simd/utility.h" }
- module vec_ext { private header "experimental/__simd/vec_ext.h" }
+ // C standard library interfaces augmented/replaced in C++
+ // <assert.h> provided by C library.
+ module std_complex_h [system] {
+ header "complex.h"
+ export std_ccomplex
+ export *
+ }
+ module std_ctype_h [system] {
+ header "ctype.h"
+ export *
+ }
+ module std_errno_h [system] {
+ header "errno.h"
+ export *
+ }
+ module std_fenv_h [system] {
+ header "fenv.h"
+ export *
+ }
+ module std_float_h [system] {
+ header "float.h"
+ export *
+ }
+ module std_inttypes_h [system] {
+ header "inttypes.h"
+ export *
+ }
+ // <iso646.h> provided by compiler.
+ module std_locale_h [system] {
+ header "locale.h"
+ export *
+ }
+ module std_math_h [system] {
+ header "math.h"
+ export *
+ }
+ // <s...
[truncated]
|
|
I think we may be talking past each other. I think we can likely find a solution that works for everyone. |
Let me discuss with some of the other modules engineers at Apple and see how we think this is supposed to work. |
Talked to @Bigcheese about it some. I think we need to have an overview of your build environment, is this for Google? Should we try to meed some time with @dwblaikie too and figure out how to best support what you need to do? |
|
I'm following only vaguely - I don't think there's any particular/immediate use/plan at Google for this work, but I might be mistaken. We don't generally use pre-written module maps, but build rules that generate module maps. @ilya-biryukov might have interest. |
Not particularly recently. I believe |
This is not at Google, it's me at home trying to save cycles on my LLVM compiles. The good news is that I haven't been able to reproduce my normal breakages when trying today to create a reproducer :-) There's been a bunch of work on modules since I last tried this, and I suspect some of it resolved the issues I was seeing. The less fortunate news is that it appears the current approach, without the top level I'll do some more digging and report back. |
Generally one needs to modularize from the leaves/bottom-up - anything less than that will produce a lot of duplication in the intermediate modules due to the leaves not being modularized. (or creating something more like a PCH - a single module that includes a bunch of non-modular code, but then all modular code that needs any of the non-modular code always goes through that single module (essentially any situation where significant amounts of (especially C++) non-modular code is depended on by more than one module is a bad thing, and should be avoided)) |
This is not a hack. It is quite well explained what that does in ed84df008f609.
That is quite incorrect. Our framework for large scale data analysis ROOT, supports modules on Linux and OSX since 2017 and everything worked pretty well before these changes...
That's very abstract. Could you please give us something to work with?
Our infrastructure at CERN depends on a single module of STL for performance, distribution and integration reasons on OSX. I'd like to pursue this patch or anything else that brings back the old behavior. I am not sure what the concerns of @ian-twilightcoder are concretely. I'd be happy to help if we can see a reproducible setup. |
|
@vgvassilev We're on the same page. We also want to see a sane modulemap with as few top-level modules as possible. However, the approach I am suggesting to get there is to gradually modularize libc++ the way it should always have evolved, that is by following the directory structure of the library and making sure that said directory structure is module friendly. As opposed to taking the modulemap soup we currently have and putting it under an umbrella. We also have reasons to want to improve this situation beyond simple code cleanup, and I plan to work on improving that situation after the LLVM 19 branch has been created. |
|
@ldionne, that sounds awesome!! |
|
Closing since we are making progress on an iterative simplification of the modulemap. Ideally we'll end up with a small number of top-level modules (we need the |
This patch is what I use to use modules on Linux when building LLVM.
Without it there are race conditions around the building of each individual module.
I have not validated this on Mac, or with the various submodule visibilitys
This patch also removes two module declarations for modules which don't exist