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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added a comment.
This revision now requires changes to proceed.

Basically LGTM, % significant comment about Hyrum's Law.



================
Comment at: libcxx/include/__functional_base:22-26
-#include <exception>
-#include <new>
-#include <type_traits>
-#include <typeinfo>
-#include <utility>
----------------
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.)


================
Comment at: libcxx/test/std/ranges/range.factories/range.iota.view/iterator/minus.pass.cpp:20
+#include <cstdint>
+#include <ranges>
 
----------------
Please leave `<ranges>` first; historically this is our style for tests that test a particular header (although I admit I don't see much point to it).


================
Comment at: libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/deduct.pass.cpp:22
 
+#include <functional>
 #include <memory>
----------------
This doesn't look needed. Why would we need this here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119439