diff --git a/libcxx/include/__memory/allocation_guard.h b/libcxx/include/__memory/allocation_guard.h --- a/libcxx/include/__memory/allocation_guard.h +++ b/libcxx/include/__memory/allocation_guard.h @@ -11,6 +11,7 @@ #define _LIBCPP___MEMORY_ALLOCATION_GUARD_H #include <__config> +#include <__memory/addressof.h> #include <__memory/allocator_traits.h> #include <__utility/move.h> #include @@ -58,9 +59,29 @@ _LIBCPP_HIDE_FROM_ABI ~__allocation_guard() _NOEXCEPT { - if (__ptr_ != nullptr) { - allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, __n_); + __destroy(); + } + + _LIBCPP_HIDE_FROM_ABI __allocation_guard(const __allocation_guard&) = delete; + _LIBCPP_HIDE_FROM_ABI __allocation_guard(__allocation_guard&& __other) _NOEXCEPT + : __alloc_(std::move(__other.__alloc_)) + , __n_(__other.__n_) + , __ptr_(__other.__ptr_) { + __other.__ptr_ = nullptr; + } + + _LIBCPP_HIDE_FROM_ABI __allocation_guard& operator=(const __allocation_guard& __other) = delete; + _LIBCPP_HIDE_FROM_ABI __allocation_guard& operator=(__allocation_guard&& __other) _NOEXCEPT { + if (std::addressof(__other) != this) { + __destroy(); + + __alloc_ = std::move(__other.__alloc_); + __n_ = __other.__n_; + __ptr_ = __other.__ptr_; + __other.__ptr_ = nullptr; } + + return *this; } _LIBCPP_HIDE_FROM_ABI @@ -76,6 +97,13 @@ } private: + _LIBCPP_HIDE_FROM_ABI + void __destroy() _NOEXCEPT { + if (__ptr_ != nullptr) { + allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, __n_); + } + } + _Alloc __alloc_; _Size __n_; _Pointer __ptr_; diff --git a/libcxx/include/forward_list b/libcxx/include/forward_list --- a/libcxx/include/forward_list +++ b/libcxx/include/forward_list @@ -195,6 +195,7 @@ #include <__iterator/move_iterator.h> #include <__iterator/next.h> #include <__memory/addressof.h> +#include <__memory/allocation_guard.h> #include <__memory/allocator.h> #include <__memory/allocator_destructor.h> #include <__memory/allocator_traits.h> @@ -292,6 +293,7 @@ pointer __next_; _LIBCPP_INLINE_VISIBILITY __forward_begin_node() : __next_(nullptr) {} + _LIBCPP_INLINE_VISIBILITY explicit __forward_begin_node(pointer __n) : __next_(__n) {} _LIBCPP_INLINE_VISIBILITY __begin_node_pointer __next_as_begin() const { @@ -307,8 +309,13 @@ : public __begin_node_of<_Tp, _VoidPtr> { typedef _Tp value_type; + typedef __begin_node_of<_Tp, _VoidPtr> _Base; + typedef typename _Base::pointer _NodePtr; value_type __value_; + + _LIBCPP_HIDE_FROM_ABI __forward_list_node() = default; + _LIBCPP_HIDE_FROM_ABI __forward_list_node(const value_type& __v, _NodePtr __next) : _Base(__next), __value_(__v) {} }; @@ -1247,14 +1254,20 @@ forward_list<_Tp, _Alloc>::insert_after(const_iterator __p, size_type __n, const value_type& __v) { + using _Guard = __allocation_guard<__node_allocator>; + __begin_node_pointer __r = __p.__get_begin(); if (__n > 0) { __node_allocator& __a = base::__alloc(); - typedef __allocator_destructor<__node_allocator> _Dp; - unique_ptr<__node, _Dp> __h(__node_traits::allocate(__a, 1), _Dp(__a, 1)); - __node_traits::construct(__a, _VSTD::addressof(__h->__value_), __v); - __node_pointer __first = __h.release(); + + __node_pointer __first = nullptr; + { + _Guard __h(__a, 1); + __node_traits::construct(__a, std::addressof(__h.__get()->__value_), __v); + __h.__get()->__next_ = nullptr; + __first = __h.__release_ptr(); + } __node_pointer __last = __first; #ifndef _LIBCPP_HAS_NO_EXCEPTIONS try @@ -1262,9 +1275,10 @@ #endif // _LIBCPP_HAS_NO_EXCEPTIONS for (--__n; __n != 0; --__n, __last = __last->__next_) { - __h.reset(__node_traits::allocate(__a, 1)); - __node_traits::construct(__a, _VSTD::addressof(__h->__value_), __v); - __last->__next_ = __h.release(); + _Guard __h(__a, 1); + __node_traits::construct(__a, std::addressof(__h.__get()->__value_), __v); + __h.__get()->__next_ = nullptr; + __last->__next_ = __h.__release_ptr(); } #ifndef _LIBCPP_HAS_NO_EXCEPTIONS } @@ -1293,14 +1307,19 @@ forward_list<_Tp, _Alloc>::insert_after(const_iterator __p, _InputIterator __f, _InputIterator __l) { + using _Guard = __allocation_guard<__node_allocator>; + __begin_node_pointer __r = __p.__get_begin(); if (__f != __l) { __node_allocator& __a = base::__alloc(); - typedef __allocator_destructor<__node_allocator> _Dp; - unique_ptr<__node, _Dp> __h(__node_traits::allocate(__a, 1), _Dp(__a, 1)); - __node_traits::construct(__a, _VSTD::addressof(__h->__value_), *__f); - __node_pointer __first = __h.release(); + __node_pointer __first = nullptr; + { + _Guard __h(__a, 1); + __node_traits::construct(__a, std::addressof(__h.__get()->__value_), *__f); + __h.__get()->__next_ = nullptr; + __first = __h.__release_ptr(); + } __node_pointer __last = __first; #ifndef _LIBCPP_HAS_NO_EXCEPTIONS try @@ -1308,9 +1327,10 @@ #endif // _LIBCPP_HAS_NO_EXCEPTIONS for (++__f; __f != __l; ++__f, ((void)(__last = __last->__next_))) { - __h.reset(__node_traits::allocate(__a, 1)); - __node_traits::construct(__a, _VSTD::addressof(__h->__value_), *__f); - __last->__next_ = __h.release(); + _Guard __h(__a, 1); + __node_traits::construct(__a, std::addressof(__h.__get()->__value_), *__f); + __h.__get()->__next_ = nullptr; + __last->__next_ = __h.__release_ptr(); } #ifndef _LIBCPP_HAS_NO_EXCEPTIONS } diff --git a/libcxx/test/libcxx/memory/allocation_guard.pass.cpp b/libcxx/test/libcxx/memory/allocation_guard.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/libcxx/memory/allocation_guard.pass.cpp @@ -0,0 +1,192 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +// + +// + +// To allow checking that self-move works correctly. +// ADDITIONAL_COMPILE_FLAGS: -Wno-self-move + +// template +// struct __allocation_guard; + +#include +#include +#include +#include + +#include "test_allocator.h" + +using A = test_allocator; + +// A trimmed-down version of `test_allocator` that is copy-assignable (in general allocators don't have to support copy +// assignment). +template +struct AssignableAllocator { + using size_type = unsigned; + using difference_type = int; + using value_type = T; + using pointer = value_type*; + using const_pointer = const value_type*; + using reference = typename std::add_lvalue_reference::type; + using const_reference = typename std::add_lvalue_reference::type; + + template + struct rebind { + using other = test_allocator; + }; + + test_allocator_statistics* stats_ = nullptr; + + explicit AssignableAllocator(test_allocator_statistics& stats) : stats_(&stats) { + ++stats_->count; + } + + TEST_CONSTEXPR_CXX14 AssignableAllocator(const AssignableAllocator& rhs) TEST_NOEXCEPT + : stats_(rhs.stats_) { + if (stats_ != nullptr) { + ++stats_->count; + ++stats_->copied; + } + } + + TEST_CONSTEXPR_CXX14 AssignableAllocator& operator=(const AssignableAllocator& rhs) TEST_NOEXCEPT { + stats_ = rhs.stats_; + if (stats_ != nullptr) { + ++stats_->count; + ++stats_->copied; + } + + return *this; + } + + TEST_CONSTEXPR_CXX14 pointer allocate(size_type n, const void* = nullptr) { + if (stats_ != nullptr) { + ++stats_->alloc_count; + } + return std::allocator().allocate(n); + } + + TEST_CONSTEXPR_CXX14 void deallocate(pointer p, size_type s) { + if (stats_ != nullptr) { + --stats_->alloc_count; + } + std::allocator().deallocate(p, s); + } + + TEST_CONSTEXPR size_type max_size() const TEST_NOEXCEPT { return UINT_MAX / sizeof(T); } + + template + TEST_CONSTEXPR_CXX20 void construct(pointer p, U&& val) { + if (stats_ != nullptr) + ++stats_->construct_count; +#if TEST_STD_VER > 17 + std::construct_at(std::to_address(p), std::forward(val)); +#else + ::new (static_cast(p)) T(std::forward(val)); +#endif + } + + TEST_CONSTEXPR_CXX14 void destroy(pointer p) { + if (stats_ != nullptr) { + ++stats_->destroy_count; + } + p->~T(); + } +}; + +// Move-only. +static_assert(!std::is_copy_constructible >::value); +static_assert(std::is_move_constructible >::value); +static_assert(!std::is_copy_assignable >::value); +static_assert(std::is_move_assignable >::value); + +int main(int, char**) { + const int size = 42; + + { // The constructor allocates using the given allocator. + test_allocator_statistics stats; + std::__allocation_guard guard(A(&stats), size); + assert(stats.alloc_count == 1); + assert(guard.__get() != nullptr); + } + + { // The destructor deallocates using the given allocator. + test_allocator_statistics stats; + { + std::__allocation_guard guard(A(&stats), size); + assert(stats.alloc_count == 1); + } + assert(stats.alloc_count == 0); + } + + { // `__release_ptr` prevents deallocation. + test_allocator_statistics stats; + A alloc(&stats); + int* ptr = nullptr; + { + std::__allocation_guard guard(alloc, size); + assert(stats.alloc_count == 1); + ptr = guard.__release_ptr(); + } + assert(stats.alloc_count == 1); + alloc.deallocate(ptr, size); + } + + { // Using the move constructor doesn't lead to double deletion. + test_allocator_statistics stats; + { + std::__allocation_guard guard1(A(&stats), size); + assert(stats.alloc_count == 1); + auto* ptr1 = guard1.__get(); + + std::__allocation_guard guard2 = std::move(guard1); + assert(stats.alloc_count == 1); + assert(guard1.__get() == nullptr); + assert(guard2.__get() == ptr1); + } + assert(stats.alloc_count == 0); + } + + { // Using the move assignment operator doesn't lead to double deletion. + using A2 = AssignableAllocator; + + test_allocator_statistics stats; + { + std::__allocation_guard guard1(A2(stats), size); + assert(stats.alloc_count == 1); + std::__allocation_guard guard2(A2(stats), size); + assert(stats.alloc_count == 2); + auto* ptr1 = guard1.__get(); + + guard2 = std::move(guard1); + assert(stats.alloc_count == 1); + assert(guard1.__get() == nullptr); + assert(guard2.__get() == ptr1); + } + assert(stats.alloc_count == 0); + } + + { // Self-assignment is a no-op. + using A2 = AssignableAllocator; + + test_allocator_statistics stats; + { + std::__allocation_guard guard(A2(stats), size); + assert(stats.alloc_count == 1); + auto* ptr = guard.__get(); + + guard = std::move(guard); + assert(stats.alloc_count == 1); + assert(guard.__get() == ptr); + } + assert(stats.alloc_count == 0); + } + + return 0; +} diff --git a/libcxx/test/std/containers/container.adaptors/from_range_container_adaptors.h b/libcxx/test/std/containers/container.adaptors/from_range_container_adaptors.h --- a/libcxx/test/std/containers/container.adaptors/from_range_container_adaptors.h +++ b/libcxx/test/std/containers/container.adaptors/from_range_container_adaptors.h @@ -17,6 +17,7 @@ #include #include +#include "../exception_safety_helpers.h" #include "../from_range_helpers.h" #include "MoveOnly.h" #include "almost_satisfies_types.h" @@ -192,18 +193,11 @@ template