There's a potential memory leak in the allocator-extended move constructor of forward_list, if the allocator parameter is not equal to the rvalue list's allocator then new list nodes must be allocated, but if doing so throws then the already-allocated nodes are not freed.
Rather than adding a try-catch block this moves the allocations out of the _Fwd_list_base constructor and into the derived constructor, so that if an exception is thrown the base class will clean everything up. This also splits out the default constructor from the one taking an allocator, so the default constructor is not explicit (as per the resolution of LWG 2193) and can be conditionally-noexcept (as per LWG 2455) but the one taking an allocator is explicit and noexcept (because copying allocators can't throw). There are also some small optimisations: The base constructors that take an allocator can take it by rvalue reference and move from it, because we always have to construct a temporary _Node_alloc_type from the user-supplied allocator. The call to std::swap in _M_move_assign can be replaced with two assignments, because we know one of the pointers is null so there's no need to use a temporary variable to preserve its value. Tested powerpc64le-linux, committed to trunk.
commit 2de95f71039003d692934065799fd3fc66155d2f Author: Jonathan Wakely <jwak...@redhat.com> Date: Wed Jun 17 16:48:59 2015 +0100 * include/bits/forward_list.h (_Fwd_list_base(const _Node_alloc_type&)): Change parameter to rvalue-reference. (_Fwd_list_base(_Fwd_list_base&&, const _Node_alloc_type&)): Likewise. (forward_list(const _Alloc&)): Split default constructor out to separate function. (forward_list(forward_list&&, const _Alloc&)): Move elements if base class didn't do so. (forward_list::_M_move_assign(forward_list&&, true_type)): Replace swap call with two assignments. * include/bits/forward_list.tcc (_Fwd_list_base(_Fwd_list_base&&, const _Node_alloc_type&)): Don't move elements when allocators are not equal. * include/debug/forward_list (forward_list(const allocator_type&)): Split default constructor out to separate function. * include/profile/forward_list (forward_list(const _Alloc&)): Likewise. diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h index d611ade..57c2d7b 100644 --- a/libstdc++-v3/include/bits/forward_list.h +++ b/libstdc++-v3/include/bits/forward_list.h @@ -314,10 +314,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _Fwd_list_base() : _M_impl() { } - _Fwd_list_base(const _Node_alloc_type& __a) - : _M_impl(__a) { } + _Fwd_list_base(_Node_alloc_type&& __a) + : _M_impl(std::move(__a)) { } - _Fwd_list_base(_Fwd_list_base&& __lst, const _Node_alloc_type& __a); + _Fwd_list_base(_Fwd_list_base&& __lst, _Node_alloc_type&& __a); _Fwd_list_base(_Fwd_list_base&& __lst) : _M_impl(std::move(__lst._M_get_Node_allocator())) @@ -435,13 +435,22 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER /** * @brief Creates a %forward_list with no elements. + */ + forward_list() + noexcept(is_nothrow_default_constructible<_Node_alloc_type>::value) + : _Base() + { } + + /** + * @brief Creates a %forward_list with no elements. * @param __al An allocator object. */ explicit - forward_list(const _Alloc& __al = _Alloc()) + forward_list(const _Alloc& __al) noexcept : _Base(_Node_alloc_type(__al)) { } + /** * @brief Copy constructor with allocator argument. * @param __list Input list to copy. @@ -459,7 +468,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER forward_list(forward_list&& __list, const _Alloc& __al) noexcept(_Node_alloc_traits::_S_always_equal()) : _Base(std::move(__list), _Node_alloc_type(__al)) - { } + { + // If __list is not empty it means its allocator is not equal to __a, + // so we need to move from each element individually. + insert_after(cbefore_begin(), + std::__make_move_if_noexcept_iterator(__list.begin()), + std::__make_move_if_noexcept_iterator(__list.end())); + } /** * @brief Creates a %forward_list with default constructed elements. @@ -1248,8 +1263,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _M_move_assign(forward_list&& __list, std::true_type) noexcept { clear(); - std::swap(this->_M_impl._M_head._M_next, - __list._M_impl._M_head._M_next); + this->_M_impl._M_head._M_next = __list._M_impl._M_head._M_next; + __list._M_impl._M_head._M_next = nullptr; std::__alloc_on_move(this->_M_get_Node_allocator(), __list._M_get_Node_allocator()); } diff --git a/libstdc++-v3/include/bits/forward_list.tcc b/libstdc++-v3/include/bits/forward_list.tcc index 1e1a02e..472020c 100644 --- a/libstdc++-v3/include/bits/forward_list.tcc +++ b/libstdc++-v3/include/bits/forward_list.tcc @@ -36,28 +36,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template<typename _Tp, typename _Alloc> _Fwd_list_base<_Tp, _Alloc>:: - _Fwd_list_base(_Fwd_list_base&& __lst, const _Node_alloc_type& __a) - : _M_impl(__a) + _Fwd_list_base(_Fwd_list_base&& __lst, _Node_alloc_type&& __a) + : _M_impl(std::move(__a)) { - if (__lst._M_get_Node_allocator() == __a) + if (__lst._M_get_Node_allocator() == _M_get_Node_allocator()) { this->_M_impl._M_head._M_next = __lst._M_impl._M_head._M_next; __lst._M_impl._M_head._M_next = 0; } else - { - this->_M_impl._M_head._M_next = 0; - _Fwd_list_node_base* __to = &this->_M_impl._M_head; - _Node* __curr = static_cast<_Node*>(__lst._M_impl._M_head._M_next); - - while (__curr) - { - __to->_M_next = - _M_create_node(std::move_if_noexcept(*__curr->_M_valptr())); - __to = __to->_M_next; - __curr = static_cast<_Node*>(__curr->_M_next); - } - } + this->_M_impl._M_head._M_next = 0; } template<typename _Tp, typename _Alloc> diff --git a/libstdc++-v3/include/debug/forward_list b/libstdc++-v3/include/debug/forward_list index d22c22f..2b42a3f 100644 --- a/libstdc++-v3/include/debug/forward_list +++ b/libstdc++-v3/include/debug/forward_list @@ -204,8 +204,11 @@ namespace __debug typedef typename _Base::const_pointer const_pointer; // 23.2.3.1 construct/copy/destroy: + + forward_list() = default; + explicit - forward_list(const allocator_type& __al = allocator_type()) + forward_list(const allocator_type& __al) noexcept : _Base(__al) { } forward_list(const forward_list& __list, const allocator_type& __al) diff --git a/libstdc++-v3/include/profile/forward_list b/libstdc++-v3/include/profile/forward_list index 295c4a5..ab7ed04 100644 --- a/libstdc++-v3/include/profile/forward_list +++ b/libstdc++-v3/include/profile/forward_list @@ -51,8 +51,11 @@ namespace __profile typedef typename _Base::const_iterator const_iterator; // 23.2.3.1 construct/copy/destroy: + + forward_list() = default; + explicit - forward_list(const _Alloc& __al = _Alloc()) + forward_list(const _Alloc& __al) noexcept : _Base(__al) { } forward_list(const forward_list& __list, const _Alloc& __al)