[libcxx-commits] [PATCH] D102781: [libc++] Alphabetize header inclusions. NFCI
Raphael Isemann via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat May 29 14:32:57 PDT 2021
teemperor added inline comments.
================
Comment at: libcxx/include/module.modulemap:548
module __string { header "__string" export * }
+ module __threading_support { header "__threading_support" export * }
module __tree { header "__tree" export * }
----------------
Quuxplusone wrote:
> @teemperor wrote:
> > I wonder if the stds_include.sh.cpp should also use the -fmodules-local-submodule-visibility flag. The default -fmodules can't really handle top modules that include textual headers. We're using the same flag in LLVM to build since D74892 (see also rG82b47b2978405f802a33b00d046e6f18ef6a47be for why)
>
> I don't know enough modules to say whether that flag would help. However, I don't think the problem here is related to //textual// headers at all (I mean, it's not because of headers like `<cassert>`). The problem here seems to be that e.g. `<mutex>` and `<thread>` both `#include <__threading_support>`; and so the contents of header `"__threading_support"` were winding up duplicated in both `module std.mutex` and `module std.thread`. (We see random heisenbuggy duplication issues with other headers too, e.g. the partial specializations in `"__iterator/iterator_traits.h"` end up duplicated and thus ambiguous.)
>
> What we really need is a way to "share" the common helper header `__threading_support` between `mutex` and `thread` (and so on). My impression is that the only(?) tool we have right now for doing that is to make `__threading_support` a whole module in its own right.
>
> Personally I would be quite happy to replace all of `module.modulemap` with a single umbrella module `std` that simply includes everything; and then worry about "modularizing the STL" sometime down the road. This year in particular we seem to be having our hands full merely "headerizing" it! :)
Sorry, I think I should have made clear that I meant 'headers that are not in a module' when I said 'textual' (as those headers behave similar to the actual textual headers declared in a modulemap).
And yes, having all the headers (even helpers) in their own submodule would probably help a lot with the bugs caused by the duplication. I'm not familiar enough with libc++ to say how much work that would be. Just commenting as I pushed for that specific test and I don't just want to leave it to others to keep it green :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102781/new/
https://reviews.llvm.org/D102781
More information about the libcxx-commits
mailing list