[libcxx-commits] [PATCH] D93074: [libc++] Split allocator_traits and pointer_traits out of <memory>

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__memory/allocator_traits.h:14
+#include <__config>
+#include <__memory/base.h>
+#include <__memory/pointer_traits.h>
----------------
Quuxplusone wrote:
> ldionne wrote:
> > miscco wrote:
> > > Should this also be __base.h to make clear that these are purely internal headers?
> > I've put the headers in a directory that had `__` in it to make it clear it was internal (and to avoid conflicting with user-provided directories as much as possible). I don't think it's necessary to have the file contain `__` too IMO.
> FWIW, I just saw this today, and I wish the level of directory structure weren't there; I'd rather have seen `<__memory_base>`, `<__memory_pointer_traits>`, `<__memory_allocator_traits>`. I don't even know what's in `<__memory/utilities.h>`, so I probably wouldn't have split it out — just combine it into `<__memory_base>` if it's needed by lots of things, and otherwise leave it in `<memory>`.
Worse, I have just noticed that this moves `addressof` out of `<type_traits>` and into `base.h`, which //depends on// `<type_traits>`; so my `<type_traits>` can no longer use `addressof` to implement `swap` for trivially relocatable types. In my branch, I'm moving `addressof` back into `<type_traits>`.

I do have some vague ideas re a new `<__type_traits_base>`, to contain `declval` and `forward` and `move` and `addressof` and `swap` and the helpers like `_EnableIf` and whatever type-traits seem most used (e.g. `is_*_constructible`, `is_integral`, `is_signed`). **But**, I don't think that's really worth pursuing, because libc++'s current philosophy is "`<type_traits>` goes at the root"; there's no point in trying to make other headers //not// include `<type_traits>` because everyone is going to include `<type_traits>` at some point anyway.

I agree with trying to extract a `<__memory_base>`. Also, in C++20 we'll need an `<__algorithm_base>` to avoid dragging all the Ranges stuff into `<vector>` and `<string>`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93074