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

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

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

LGTM % comments!



================
Comment at: libcxx/include/__vector/construct.h:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
I'm not thrilled by the name of this file, although I can't come up with anything super better, so the current name is not unacceptable AFAIC. //Possibly// `<__vector/construct_range.h>` or `<__vector/construct_helpers.h>` would be better?


================
Comment at: libcxx/include/memory:679
 #include <__memory/unique_ptr.h>
 #include <__memory/uses_allocator.h>
 #include <compare>
----------------
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.)


================
Comment at: libcxx/include/memory:704
 
-template <class _Alloc, class _Ptr>
-_LIBCPP_INLINE_VISIBILITY
-void __construct_forward_with_exception_guarantees(_Alloc& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __begin2) {
-    static_assert(__is_cpp17_move_insertable<_Alloc>::value,
-        "The specified type does not meet the requirements of Cpp17MoveInsertable");
-    typedef allocator_traits<_Alloc> _Traits;
-    for (; __begin1 != __end1; ++__begin1, (void)++__begin2) {
-        _Traits::construct(__a, _VSTD::__to_address(__begin2),
-#ifdef _LIBCPP_NO_EXCEPTIONS
-            _VSTD::move(*__begin1)
-#else
-            _VSTD::move_if_noexcept(*__begin1)
-#endif
-        );
-    }
-}
-
-template <class _Alloc, class _Tp, typename enable_if<
-    (__is_default_allocator<_Alloc>::value || !__has_construct<_Alloc, _Tp*, _Tp>::value) &&
-    is_trivially_move_constructible<_Tp>::value
->::type>
-_LIBCPP_INLINE_VISIBILITY
-void __construct_forward_with_exception_guarantees(_Alloc&, _Tp* __begin1, _Tp* __end1, _Tp*& __begin2) {
-    ptrdiff_t _Np = __end1 - __begin1;
-    if (_Np > 0) {
-        _VSTD::memcpy(__begin2, __begin1, _Np * sizeof(_Tp));
-        __begin2 += _Np;
-    }
-}
-
-template <class _Alloc, class _Iter, class _Ptr>
-_LIBCPP_INLINE_VISIBILITY
-void __construct_range_forward(_Alloc& __a, _Iter __begin1, _Iter __end1, _Ptr& __begin2) {
-    typedef allocator_traits<_Alloc> _Traits;
-    for (; __begin1 != __end1; ++__begin1, (void) ++__begin2) {
-        _Traits::construct(__a, _VSTD::__to_address(__begin2), *__begin1);
-    }
-}
-
-template <class _Alloc, class _Source, class _Dest,
-          class _RawSource = typename remove_const<_Source>::type,
-          class _RawDest = typename remove_const<_Dest>::type,
-          class =
-    typename enable_if<
-        is_trivially_copy_constructible<_Dest>::value &&
-        is_same<_RawSource, _RawDest>::value &&
-        (__is_default_allocator<_Alloc>::value || !__has_construct<_Alloc, _Dest*, _Source&>::value)
-    >::type>
-_LIBCPP_INLINE_VISIBILITY
-void __construct_range_forward(_Alloc&, _Source* __begin1, _Source* __end1, _Dest*& __begin2) {
-    ptrdiff_t _Np = __end1 - __begin1;
-    if (_Np > 0) {
-        _VSTD::memcpy(const_cast<_RawDest*>(__begin2), __begin1, _Np * sizeof(_Dest));
-        __begin2 += _Np;
-    }
-}
-
-template <class _Alloc, class _Ptr>
-_LIBCPP_INLINE_VISIBILITY
-void __construct_backward_with_exception_guarantees(_Alloc& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __end2) {
-    static_assert(__is_cpp17_move_insertable<_Alloc>::value,
-        "The specified type does not meet the requirements of Cpp17MoveInsertable");
-    typedef allocator_traits<_Alloc> _Traits;
-    while (__end1 != __begin1) {
-        _Traits::construct(__a, _VSTD::__to_address(__end2 - 1),
-#ifdef _LIBCPP_NO_EXCEPTIONS
-            _VSTD::move(*--__end1)
-#else
-            _VSTD::move_if_noexcept(*--__end1)
-#endif
-        );
-        --__end2;
-    }
-}
-
-template <class _Alloc, class _Tp, class = typename enable_if<
-    (__is_default_allocator<_Alloc>::value || !__has_construct<_Alloc, _Tp*, _Tp>::value) &&
-    is_trivially_move_constructible<_Tp>::value
->::type>
-_LIBCPP_INLINE_VISIBILITY
-void __construct_backward_with_exception_guarantees(_Alloc&, _Tp* __begin1, _Tp* __end1, _Tp*& __end2) {
-    ptrdiff_t _Np = __end1 - __begin1;
-    __end2 -= _Np;
-    if (_Np > 0)
-        _VSTD::memcpy(static_cast<void*>(__end2), static_cast<void const*>(__begin1), _Np * sizeof(_Tp));
-}
-
-struct __destruct_n
-{
-private:
-    size_t __size_;
-
-    template <class _Tp>
-    _LIBCPP_INLINE_VISIBILITY void __process(_Tp* __p, false_type) _NOEXCEPT
-        {for (size_t __i = 0; __i < __size_; ++__i, ++__p) __p->~_Tp();}
-
-    template <class _Tp>
-    _LIBCPP_INLINE_VISIBILITY void __process(_Tp*, true_type) _NOEXCEPT
-        {}
-
-    _LIBCPP_INLINE_VISIBILITY void __incr(false_type) _NOEXCEPT
-        {++__size_;}
-    _LIBCPP_INLINE_VISIBILITY void __incr(true_type) _NOEXCEPT
-        {}
-
-    _LIBCPP_INLINE_VISIBILITY void __set(size_t __s, false_type) _NOEXCEPT
-        {__size_ = __s;}
-    _LIBCPP_INLINE_VISIBILITY void __set(size_t, true_type) _NOEXCEPT
-        {}
-public:
-    _LIBCPP_INLINE_VISIBILITY explicit __destruct_n(size_t __s) _NOEXCEPT
-        : __size_(__s) {}
-
-    template <class _Tp>
-    _LIBCPP_INLINE_VISIBILITY void __incr() _NOEXCEPT
-        {__incr(integral_constant<bool, is_trivially_destructible<_Tp>::value>());}
-
-    template <class _Tp>
-    _LIBCPP_INLINE_VISIBILITY void __set(size_t __s, _Tp*) _NOEXCEPT
-        {__set(__s, integral_constant<bool, is_trivially_destructible<_Tp>::value>());}
-
-    template <class _Tp>
-    _LIBCPP_INLINE_VISIBILITY void operator()(_Tp* __p) _NOEXCEPT
-        {__process(__p, integral_constant<bool, is_trivially_destructible<_Tp>::value>());}
-};
-
 _LIBCPP_FUNC_VIS void* align(size_t __align, size_t __sz, void*& __ptr, size_t& __space);
 
----------------
This should move to `<__memory/align.h>`. It's just a one-liner, but it lets us make `<memory>` a true top-level header with no extraneous bits. (And no libc++ header will ever need to resort to `#include <memory>` for any reason except backward compatibility.)

(You'll also get rid of `_LIBCPP_BEGIN_NAMESPACE_STD` / `_LIBCPP_END_NAMESPACE_STD` at that point.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115059