[libcxx-commits] [PATCH] D132284: [libc++] Reduces the number of transitive includes.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/docs/DesignDocs/HeaderRemovalPolicy.rst:32-33
+that users will have to fix their missing includes in order to upgrade to a
+newer version of the Standard. Libc++ also reserves the right to remove
+transitive includes at any other time, however new language versions will be
+used as a convenient way to perform bulk removals of
----------------
Mordante wrote:
> philnik wrote:
> > Would the plan still be to remove transitive includes at some point? If not the includes will probably end up in a huge mess.
> I'm not sure. We surely need to remove them when it breaks us, like I did for `<chrono>`.
> The big issue of removal is that may break the packagers who then need to fix it.
> If it was only developers I would have less of an issue with the breakage.
> 
> We can still remove them without repercussions from C++23 and I want to look at some more TODO removals and use the granularized headers in more places. Some header include `concepts` or `type_traits` which can be minimized now.
> 
> I'm not sure whether we would get in a huge mess with the current header method.
> Do you have any concrete concerns with this approach?
> 
> 
> 
I'm fine with it if they're removed in a release where we don't get that much done that we can ship stable. LLVM 16 will definitely contain ranges and probably format, `std::pmr` (PRs coming soon), maybe the PSTL (@ldionne friendly reminder to work on D103198) and more. So that's most likely not a good option, but I don't think that there will be huge additions in LLVM 17. LLVM 15 would actually have been a really good release in hind-sight. The main reason we didn't do it then was because of ranges, but that didn't actually happen.

TL;DR I don't think it would be a huge problem if people don't have a really good reason to update (not that you shouldn't keep your tools up-to-date).

My main concern is that we will have to move stuff around to implement the rest of C++17 and C++20. Having
```
#if !defined(_LIBCPP_REMOVE_TRANSITIVE_INCLUDES) && _LIBCPP_STD_VER <= 14
...
#endif

#if !defined(_LIBCPP_REMOVE_TRANSITIVE_INCLUDES) && _LIBCPP_STD_VER <= 17
...
#endif


#if !defined(_LIBCPP_REMOVE_TRANSITIVE_INCLUDES) && _LIBCPP_STD_VER <= 20
...
#endif
```
seems to me like a bad idea. Adding a few includes also isn't that huge of a deal. I'm not saying we should do it right now, but it might be a good idea to say: We'll remove all transitive includes in LLVM 17 (maybe?), so, vendors, please enable `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` for your internal testing and file bugs against any libraries/programs that break. That would give vendors lots of time to resolve any issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132284