[libc++] Split a few things out of <memory>

[ldionne]

Summary

These patches split std::allocator, std::get_temporary_buffer and std::auto_ptr
outside of <memory>. More patches splitting out the whole header are coming.

[• Quuxplusone]

As usual, I oppose this kind of patch unless it includes some "benefit" (e.g., simplifying the dependency graph). Right now this is just complexifying the graph.

[ldionne]

In D100216#2680139, @Quuxplusone wrote:

As usual, I oppose this kind of patch unless it includes some "benefit" (e.g., simplifying the dependency graph). Right now this is just complexifying the graph.

The benefit is that <memory> is gradually becoming smaller. Locally, where I have more patches applied, I've started discovering things like helpers that are utterly misplaced (e.g. a helper that lives in <memory> but has no business being there, since it's only used in e.g. <algorithm>). Overall, this is going to make it easier to navigate and understand the library.

[curdeius]

libcxx/include/__memory/temporary_buffer.h

29 ↗

(On Diff #336505)

Sidenote, why isn't it guarded with #if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_xyz) like auto_ptr?

libcxx/include/memory

683

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.

[ldionne]

libcxx/include/__memory/temporary_buffer.h

29 ↗

(On Diff #336505)

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.

libcxx/include/memory

683

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.

[curdeius]

LGTM. I prefer having clearly cut headers. IMO it is better to have more than not enough. That's of course subjective.

BTW. Should I understand that we settled on __memory/* naming scheme?

libcxx/include/__memory/temporary_buffer.h

29 ↗

(On Diff #336505)

Indeed, it's used in`<algorithm>`.
https://github.com/llvm/llvm-project/blob/main/libcxx/include/algorithm#L3623

[ldionne]

In D100216#2680460, @curdeius wrote:

LGTM. I prefer having clearly cut headers. IMO it is better to have more than not enough. That's of course subjective.

It's "subjective", but then anything related to readability is subjective. I still think the benefits of such a simple change will be overwhelming in the long run -- I'm already starting to see a lot of structure in the library that I didn't see before, yet I've been working on it full time for 3 years.

BTW. Should I understand that we settled on __memory/* naming scheme?

For now it does the job. I'm not entirely satisfied, for example __memory/compressed_pair.h isn't great since __compressed_pair isn't strongly tied to <memory>. I think better might be __detail/compressed_pair.h for things that are not clearly tied to a specific public header. But it'll be easy to change that in the future if we want, and git handles renames pretty well, so there shouldn't be any churn generated from changing in the future.

[Mordante]

One question other than that LGTM!

libcxx/include/__memory/allocator.h

11 ↗

(On Diff #336505)

<__string> contains a duplicate of the synopsis of char_traits. These new headers don't have a synopsis. Is that intended or an oversight?

[ldionne]

libcxx/include/__memory/allocator.h

11 ↗

(On Diff #336505)

I asked on Discord whether we wanted to keep the synopsis in the public header or split it up in the sub-headers, and we agreed to put the synopsis only in the public header. So I'll remove the synopsis from __string in a separate commit for consistency, thanks for the observation.

[Mordante]

libcxx/include/__memory/allocator.h

11 ↗

(On Diff #336505)

Ok that's fine for me as well. I think consistency is the keyword. If we tend to do more smaller headers one synopsis increases the readability.

libcxx/include/CMakeLists.txt

 set(files
   __availability
   __bit_reference
   __bits
   __bsd_locale_defaults.h
   __bsd_locale_fallbacks.h
   __config
   __debug
   __errc
   __functional_03
   __functional_base
   __functional_base_03
   __hash_table
   __libcpp_version
   __locale
   __memory/allocator_traits.h
+  __memory/auto_ptr.h
   __memory/base.h
   __memory/pointer_traits.h
   __memory/utilities.h
   __mutex_base
   __node_handle
   __nullptr
   __split_buffer
   __sso_allocator

libcxx/include/__memory/auto_ptr.h

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___MEMORY_AUTO_PTR_H
+#define _LIBCPP___MEMORY_AUTO_PTR_H
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR)
+template <class _Tp>
+struct _LIBCPP_DEPRECATED_IN_CXX11 auto_ptr_ref
+{
+    _Tp* __ptr_;
+};
+
+template<class _Tp>
+class _LIBCPP_TEMPLATE_VIS _LIBCPP_DEPRECATED_IN_CXX11 auto_ptr
+{
+private:
+    _Tp* __ptr_;
+public:
+    typedef _Tp element_type;
+
+    _LIBCPP_INLINE_VISIBILITY explicit auto_ptr(_Tp* __p = 0) _NOEXCEPT : __ptr_(__p) {}
+    _LIBCPP_INLINE_VISIBILITY auto_ptr(auto_ptr& __p) _NOEXCEPT : __ptr_(__p.release()) {}
+    template<class _Up> _LIBCPP_INLINE_VISIBILITY auto_ptr(auto_ptr<_Up>& __p) _NOEXCEPT
+        : __ptr_(__p.release()) {}
+    _LIBCPP_INLINE_VISIBILITY auto_ptr& operator=(auto_ptr& __p) _NOEXCEPT
+        {reset(__p.release()); return *this;}
+    template<class _Up> _LIBCPP_INLINE_VISIBILITY auto_ptr& operator=(auto_ptr<_Up>& __p) _NOEXCEPT
+        {reset(__p.release()); return *this;}
+    _LIBCPP_INLINE_VISIBILITY auto_ptr& operator=(auto_ptr_ref<_Tp> __p) _NOEXCEPT
+        {reset(__p.__ptr_); return *this;}
+    _LIBCPP_INLINE_VISIBILITY ~auto_ptr() _NOEXCEPT {delete __ptr_;}
+
+    _LIBCPP_INLINE_VISIBILITY _Tp& operator*() const _NOEXCEPT
+        {return *__ptr_;}
+    _LIBCPP_INLINE_VISIBILITY _Tp* operator->() const _NOEXCEPT {return __ptr_;}
+    _LIBCPP_INLINE_VISIBILITY _Tp* get() const _NOEXCEPT {return __ptr_;}
+    _LIBCPP_INLINE_VISIBILITY _Tp* release() _NOEXCEPT
+    {
+        _Tp* __t = __ptr_;
+        __ptr_ = nullptr;
+        return __t;
+    }
+    _LIBCPP_INLINE_VISIBILITY void reset(_Tp* __p = 0) _NOEXCEPT
+    {
+        if (__ptr_ != __p)
+            delete __ptr_;
+        __ptr_ = __p;
+    }
+
+    _LIBCPP_INLINE_VISIBILITY auto_ptr(auto_ptr_ref<_Tp> __p) _NOEXCEPT : __ptr_(__p.__ptr_) {}
+    template<class _Up> _LIBCPP_INLINE_VISIBILITY operator auto_ptr_ref<_Up>() _NOEXCEPT
+        {auto_ptr_ref<_Up> __t; __t.__ptr_ = release(); return __t;}
+    template<class _Up> _LIBCPP_INLINE_VISIBILITY operator auto_ptr<_Up>() _NOEXCEPT
+        {return auto_ptr<_Up>(release());}
+};
+
+template <>
+class _LIBCPP_TEMPLATE_VIS _LIBCPP_DEPRECATED_IN_CXX11 auto_ptr<void>
+{
+public:
+    typedef void element_type;
+};
+#endif
+
+_LIBCPP_END_NAMESPACE_STD
+
+_LIBCPP_POP_MACROS
+
+#endif  // _LIBCPP___MEMORY_AUTO_PTR_H

libcxx/include/memory

 #include <utility>
 #include <iterator>
 #include <__functional_base>
 #include <iosfwd>
 #include <tuple>
 #include <stdexcept>
 #include <cstring>
 #include <__memory/allocator_traits.h>
+#include <__memory/auto_ptr.h>
 #include <__memory/base.h>
 #include <__memory/pointer_traits.h>
 #include <__memory/utilities.h>
 #if !defined(_LIBCPP_HAS_NO_ATOMIC_HEADER)
 #  include <atomic>
 #endif
 #include <version>
 

 
 template <class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY
 void return_temporary_buffer(_Tp* __p) _NOEXCEPT
 {
   _VSTD::__libcpp_deallocate_unsized((void*)__p, _LIBCPP_ALIGNOF(_Tp));
 }
 
-#if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR)
-template <class _Tp>
-struct _LIBCPP_DEPRECATED_IN_CXX11 auto_ptr_ref
-{
-    _Tp* __ptr_;
-};
-
-template<class _Tp>
-class _LIBCPP_TEMPLATE_VIS _LIBCPP_DEPRECATED_IN_CXX11 auto_ptr
-{
-private:
-    _Tp* __ptr_;
-public:
-    typedef _Tp element_type;
-
-    _LIBCPP_INLINE_VISIBILITY explicit auto_ptr(_Tp* __p = 0) _NOEXCEPT : __ptr_(__p) {}
-    _LIBCPP_INLINE_VISIBILITY auto_ptr(auto_ptr& __p) _NOEXCEPT : __ptr_(__p.release()) {}
-    template<class _Up> _LIBCPP_INLINE_VISIBILITY auto_ptr(auto_ptr<_Up>& __p) _NOEXCEPT
-        : __ptr_(__p.release()) {}
-    _LIBCPP_INLINE_VISIBILITY auto_ptr& operator=(auto_ptr& __p) _NOEXCEPT
-        {reset(__p.release()); return *this;}
-    template<class _Up> _LIBCPP_INLINE_VISIBILITY auto_ptr& operator=(auto_ptr<_Up>& __p) _NOEXCEPT
-        {reset(__p.release()); return *this;}
-    _LIBCPP_INLINE_VISIBILITY auto_ptr& operator=(auto_ptr_ref<_Tp> __p) _NOEXCEPT
-        {reset(__p.__ptr_); return *this;}
-    _LIBCPP_INLINE_VISIBILITY ~auto_ptr() _NOEXCEPT {delete __ptr_;}
-
-    _LIBCPP_INLINE_VISIBILITY _Tp& operator*() const _NOEXCEPT
-        {return *__ptr_;}
-    _LIBCPP_INLINE_VISIBILITY _Tp* operator->() const _NOEXCEPT {return __ptr_;}
-    _LIBCPP_INLINE_VISIBILITY _Tp* get() const _NOEXCEPT {return __ptr_;}
-    _LIBCPP_INLINE_VISIBILITY _Tp* release() _NOEXCEPT
-    {
-        _Tp* __t = __ptr_;
-        __ptr_ = nullptr;
-        return __t;
-    }
-    _LIBCPP_INLINE_VISIBILITY void reset(_Tp* __p = 0) _NOEXCEPT
-    {
-        if (__ptr_ != __p)
-            delete __ptr_;
-        __ptr_ = __p;
-    }
-
-    _LIBCPP_INLINE_VISIBILITY auto_ptr(auto_ptr_ref<_Tp> __p) _NOEXCEPT : __ptr_(__p.__ptr_) {}
-    template<class _Up> _LIBCPP_INLINE_VISIBILITY operator auto_ptr_ref<_Up>() _NOEXCEPT
-        {auto_ptr_ref<_Up> __t; __t.__ptr_ = release(); return __t;}
-    template<class _Up> _LIBCPP_INLINE_VISIBILITY operator auto_ptr<_Up>() _NOEXCEPT
-        {return auto_ptr<_Up>(release());}
-};
-
-template <>
-class _LIBCPP_TEMPLATE_VIS _LIBCPP_DEPRECATED_IN_CXX11 auto_ptr<void>
-{
-public:
-    typedef void element_type;
-};
-#endif
-
 // Tag used to default initialize one or both of the pair's elements.
 struct __default_init_tag {};
 struct __value_init_tag {};
 
 template <class _Tp, int _Idx,
           bool _CanBeEmptyBase =
               is_empty<_Tp>::value && !__libcpp_is_final<_Tp>::value>
 struct __compressed_pair_elem {