[libcxx-commits] [PATCH] D115059: [libcxx][modularisation] completes <memory> modularisation

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/memory:679
 #include <__memory/unique_ptr.h>
 #include <__memory/uses_allocator.h>
 #include <compare>
----------------
ldionne wrote:
> Quuxplusone wrote:
> > I think we still haven't made a consistent decision about whether `<foo>` should include //all// `<__foo/bar.h>` subheaders (like the module does), or just the "public-relevant" ones. However, for consistency with how //this specific file// currently includes `<__memory/allocation_guard.h>`, please `#include` all your new headers in here too.
> > 
> > ...Except for `<__vector/construct.h>`, I suppose, since that's now part of `<vector>`. (It's basically as if we cut-and-pasted those components from here to there, in addition to the NFC parts of this patch. I think that's totally fine, because we're within our rights to shuffle around internal implementation details however often we like.)
> Yeah, let's include all of the `__memory/foo.h` headers since that's what we've been doing. Agreed on not including `__vector/construct.h`, since it is not part of `__memory/`.
There's precedent where we don't do this from `<functional>`. I'm also not in favour of exporting implementation details, which is what this would enable (i.e. I'd prefer to go and make a CL that deletes the detail headers in other user-facing headers).

A lot of the stuff in this CL would be better off in some `__container` directory anyway, since they're common to containers headers, rather than stuff that genuinely is in `<memory>`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115059