[libcxx-commits] [PATCH] D119439: [libc++] Remove __functional_base

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__functional_base:22-26
-#include <exception>
-#include <new>
-#include <type_traits>
-#include <typeinfo>
-#include <utility>
----------------
Quuxplusone wrote:
> If you're marking this as NFC, then please make sure that each top-level header that //previously// transitively included `<__functional_base>` still (directly) includes each of these headers. E.g. if some user is doing
> ```
> #include <bitset>
> auto x = typeid(int);  // depends on <typeinfo>
> ```
> we don't want to break them. You can mark any `#include` lines added in this way with a comment like `// TODO: remove this`.
> 
> IMHO we don't need to add direct inclusions to `<format>` or `<ranges>`, but I'd recommend doing it for all the others. (And by doing it for `<iterator>`, you'll pick up `<ranges>` for free anyway.)
This ^

Removing includes is a very common source of breakage. Unfortunately, people don't include what they use all the time, and as a result they can be broken when we remove a transitive include. Removing headers is OK, but it has to be called out explicitly and release-noted, and concretely we have to be aware that it's going to trigger some work on the vendor side (they will see failures downstream when rebuilding code and they will have to fix the projects that didn't have the correct includes).

Ideally, this commit would remove `__functional_base` while still making sure we include all the stuff we used to include. And in a separate commit, we can remove those includes and release-note it. I can also run a build and try to see what the impact of the removal would be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119439