[libcxx-commits] [PATCH] D124755: [libc++] Granularize parts of <type_traits>

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added a comment.

You can safely ignore my comments below; I don't intend to block this change.

However, I want to again state my objections for the record.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I'm going to shout my normal speech about how:

- I don't think this improves the quality of the code,
- It really muddies the blame history (which has been really really really problematic for me recently,)
- It makes our module maps more complicated,
- It creates less understandable diagnostics by exposing what should be private implementation details.
- I don't know hat it actually improves portability much; We are the only standard library to do this (at least to this extent), with the claim that exposing more names than necessary creates portability problems, but I don't think that's true for a number of reasons, one of which being that every other standard library includes all of <X> if they need one thing from <X>.

Have we done tests to see how many fewer names we actually expose to the end user for a typical real TU? I'm imaging we could do something like:

1. Produce a pre-processed header.
2. Run clang-query over it to produce a list of STL declarations.
3. Repeat for both HEAD and older versions.
4. Diff the outputs.

Further, as the complexity of our header structure increases, I find it increasingly difficult to reason about the header structure. I'm glad we have a tool to detect circular deps, because that's beyond my capabilities now.  But how are we ensuring all necessary declarations have been included *directly*, rather than transitively. AKA how do we enforce "include what you use"?

And finally, given the goal of this is to remove unnecessary declarations leaking from headers, how are we tracking our progress doing this? We should be aware exactly what breaking changes we're shipping to users, and I don't know that we are.

P.S. Also, and I have been meaning to confirm this, but I expect this to be some amount slower for non-trivial real-world C++ builds. But only when there is enough code being compiled to frequently eject the STL headers from the linux kernels file cache. That said, until I actually produce a test, it's fair to say I'm only talking out my butt.



================
Comment at: libcxx/include/module.modulemap:987
+    module conditional                { private header "__type_traits/conditional.h" }
+    module decay                      { private header "__type_traits/decay.h" }
+    module enable_if                  { private header "__type_traits/enable_if.h" }
----------------
Should these have non-reserved names?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124755