[libcxx-commits] [PATCH] D100216: [libc++] Split a few things out of <memory>

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked 2 inline comments as done.
ldionne added inline comments.


================
Comment at: libcxx/include/__memory/temporary_buffer.h:29
+pair<_Tp*, ptrdiff_t>
+get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT
+{
----------------
curdeius wrote:
> Sidenote, why isn't it guarded with `	#if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_xyz)` like `auto_ptr`?
I suspect this is simply an oversight, honestly. I think moving away from gigantic headers will make it easier to reason and spot this sort of stuff, too.

Thinking twice, it may be that we're using `get_temporary_buffer()` to implement some algorithms so we can't actually remove it. Of course we could move to an internal name and stop providing the public name.

I'm not changing any code whatsoever in this patch, just moving stuff around.


================
Comment at: libcxx/include/memory:685
 #include <__memory/allocator_traits.h>
+#include <__memory/auto_ptr.h>
 #include <__memory/base.h>
----------------
curdeius wrote:
> IMO there would be a "real" benefit of splitting those headers if their inclusion were guarded by `	#if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_xyz)`. WDYT?
> That cannot apply to allocator of course as not all of it was removed in C++20.
> 
> By real, I mean more than just going down with the line count.
Yes, I fully agree, and in fact this is how I had done it originally. But I decided to stage it into multiple changes to avoid making any sort of code changes in the patches where I'm moving stuff around. Subtle mistakes are so easy to make.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100216