[libcxx-commits] [PATCH] D133252: [libc++][random] Removes transitive includes.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D133252#3772246 <https://reviews.llvm.org/D133252#3772246>, @ldionne wrote:

> This change is a natural step in our ongoing effort to clean up incidental transitive includes. It just happened to surface the fact that our `modulemap` is written in an extremely lazy way (by using `export *`), which leads to our headers sometimes providing stuff implicitly when they shouldn't.
>
> To be clear:
>
> - I don't think this will break any more code than the other incidental-transitive-includes removals we've been doing (and which are guarded behind the standard version for now). Specifically, this one is a NFC for users that don't build with modules enabled, and it is a potential breaking change for users that build with `-fmodules -std=c++23`. I think it's reasonable to expect that those users can fix their includes, that's likely one of the reason they are using modules in the first place.
> - Just to reiterate the motivation for this whole effort, removing transitive includes is necessary to implement some C++ features (otherwise we run into circular dependencies), and it also leads to shorter compilation times as @philnik measured.
>
> IMO what we should do to tackle this issue once and for all is to fix our modulemap so that each header properly exports what it should be exporting, and nothing more. However, I think that is likely a ton of work, it's not fun work, and it's not clear that the ROI is that great given C++20 modules, so perhaps we should not actually do it, and only fix our modulemap as issues come up.

The modulemap works actually pretty well with the granularized includes. With the current style everything defined inside a header in `__public_header/` gets re-exported and anything else doesn't. So IMO we should just continue with granularizing all the headers instead of making the modulemap super complex.



================
Comment at: libcxx/include/module.modulemap.in:923
+        private header "__random/discrete_distribution.h"
+        export vector
+      }
----------------
Why do you have to re-export `vector`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133252/new/

https://reviews.llvm.org/D133252