* std::forward_list optim for always equal allocator
@ 2017-07-17 20:10 François Dumont
2017-07-17 20:14 ` Daniel Krügler
2017-08-28 20:39 ` François Dumont
0 siblings, 2 replies; 13+ messages in thread
From: François Dumont @ 2017-07-17 20:10 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]
Hi
Here is the patch to implement the always equal alloc optimization
for forward_list. With this version there is no abi issue.
I also prefer to implement the _Fwd_list_node_base move operator
for consistency with the move constructor and used it where applicable.
* include/bits/forward_list.h
(_Fwd_list_node_base& operator=(_Fwd_list_node_base&&)): Implement.
(_Fwd_list_impl(_Fwd_list_impl&&, _Node_alloc_type&&)): New.
(_Fwd_list_base(_Fwd_list_base&&, _Node_alloc_type&&, std::true_type)):
New, use latter.
(forward_list(forward_list&&, _Node_alloc_type&&, std::false_type)):
New.
(forward_list(forward_list&&, _Node_alloc_type&&, std::true_type)):
New.
(forward_list(forward_list&&, const _Alloc&)): Adapt to use latters.
* include/bits/forward_list.tcc
(_Fwd_list_base(_Fwd_list_base&&, _Node_alloc_type&&)): Adapt to use
_M_impl._M_head move assignment.
(forward_list<>::merge(forward_list<>&&, _Comp)): Likewise.
Tested under Linux x86_64, ok to commit ?
François
[-- Attachment #2: forward_list.patch --]
[-- Type: text/x-patch, Size: 7763 bytes --]
diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h
index 9ddbcb2..dec91ea 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -60,7 +60,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_Fwd_list_node_base(const _Fwd_list_node_base&) = delete;
_Fwd_list_node_base& operator=(const _Fwd_list_node_base&) = delete;
- _Fwd_list_node_base& operator=(_Fwd_list_node_base&&) = delete;
+
+ _Fwd_list_node_base&
+ operator=(_Fwd_list_node_base&& __x)
+ {
+ _M_next = __x._M_next;
+ __x._M_next = nullptr;
+ return *this;
+ }
_Fwd_list_node_base* _M_next = nullptr;
@@ -75,7 +82,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__end->_M_next = _M_next;
}
else
- __begin->_M_next = 0;
+ __begin->_M_next = nullptr;
_M_next = __keep;
return __end;
}
@@ -180,7 +187,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
if (_M_node)
return _Fwd_list_iterator(_M_node->_M_next);
else
- return _Fwd_list_iterator(0);
+ return _Fwd_list_iterator(nullptr);
}
_Fwd_list_node_base* _M_node;
@@ -251,7 +258,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
if (this->_M_node)
return _Fwd_list_const_iterator(_M_node->_M_next);
else
- return _Fwd_list_const_iterator(0);
+ return _Fwd_list_const_iterator(nullptr);
}
const _Fwd_list_node_base* _M_node;
@@ -298,6 +305,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_Fwd_list_impl(_Fwd_list_impl&&) = default;
+ _Fwd_list_impl(_Fwd_list_impl&& __fl, _Node_alloc_type&& __a)
+ : _Node_alloc_type(std::move(__a)), _M_head(std::move(__fl._M_head))
+ { }
+
_Fwd_list_impl(_Node_alloc_type&& __a)
: _Node_alloc_type(std::move(__a))
{ }
@@ -323,15 +334,21 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_Fwd_list_base(_Node_alloc_type&& __a)
: _M_impl(std::move(__a)) { }
+ // When allocators are always equal.
+ _Fwd_list_base(_Fwd_list_base&& __lst, _Node_alloc_type&& __a,
+ std::true_type)
+ : _M_impl(std::move(__lst._M_impl), std::move(__a))
+ { }
+
+ // When allocators are not always equal.
_Fwd_list_base(_Fwd_list_base&& __lst, _Node_alloc_type&& __a);
_Fwd_list_base(_Fwd_list_base&&) = default;
~_Fwd_list_base()
- { _M_erase_after(&_M_impl._M_head, 0); }
+ { _M_erase_after(&_M_impl._M_head, nullptr); }
protected:
-
_Node*
_M_get_node()
{
@@ -448,7 +465,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _Base(_Node_alloc_type(__al))
{ }
-
/**
* @brief Copy constructor with allocator argument.
* @param __list Input list to copy.
@@ -458,14 +474,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _Base(_Node_alloc_type(__al))
{ _M_range_initialize(__list.begin(), __list.end()); }
- /**
- * @brief Move constructor with allocator argument.
- * @param __list Input list to move.
- * @param __al An allocator object.
- */
- forward_list(forward_list&& __list, const _Alloc& __al)
- noexcept(_Node_alloc_traits::_S_always_equal())
- : _Base(std::move(__list), _Node_alloc_type(__al))
+ private:
+ forward_list(forward_list&& __list, _Node_alloc_type&& __al,
+ std::false_type)
+ : _Base(std::move(__list), std::move(__al))
{
// If __list is not empty it means its allocator is not equal to __a,
// so we need to move from each element individually.
@@ -474,6 +486,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
std::__make_move_if_noexcept_iterator(__list.end()));
}
+ forward_list(forward_list&& __list, _Node_alloc_type&& __al,
+ std::true_type)
+ noexcept
+ : _Base(std::move(__list), _Node_alloc_type(__al), std::true_type{})
+ { }
+
+ public:
+ /**
+ * @brief Move constructor with allocator argument.
+ * @param __list Input list to move.
+ * @param __al An allocator object.
+ */
+ forward_list(forward_list&& __list, const _Alloc& __al)
+ noexcept(_Node_alloc_traits::_S_always_equal())
+ : forward_list(std::move(__list), _Node_alloc_type(__al),
+ typename _Node_alloc_traits::is_always_equal{})
+ { }
+
/**
* @brief Creates a %forward_list with default constructed elements.
* @param __n The number of elements to initially create.
@@ -702,7 +732,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
iterator
end() noexcept
- { return iterator(0); }
+ { return iterator(nullptr); }
/**
* Returns a read-only iterator that points one past the last
@@ -711,7 +741,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_iterator
end() const noexcept
- { return const_iterator(0); }
+ { return const_iterator(nullptr); }
/**
* Returns a read-only (constant) iterator that points to the
@@ -738,7 +768,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_iterator
cend() const noexcept
- { return const_iterator(0); }
+ { return const_iterator(nullptr); }
/**
* Returns true if the %forward_list is empty. (Thus begin() would
@@ -746,7 +776,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
bool
empty() const noexcept
- { return this->_M_impl._M_head._M_next == 0; }
+ { return this->_M_impl._M_head._M_next == nullptr; }
/**
* Returns the largest possible number of elements of %forward_list.
@@ -1051,7 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
void
clear() noexcept
- { this->_M_erase_after(&this->_M_impl._M_head, 0); }
+ { this->_M_erase_after(&this->_M_impl._M_head, nullptr); }
// 23.3.4.6 forward_list operations:
diff --git a/libstdc++-v3/include/bits/forward_list.tcc b/libstdc++-v3/include/bits/forward_list.tcc
index b7c906e..f13e959 100644
--- a/libstdc++-v3/include/bits/forward_list.tcc
+++ b/libstdc++-v3/include/bits/forward_list.tcc
@@ -41,10 +41,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _M_impl(std::move(__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;
- }
+ this->_M_impl._M_head = std::move(__lst._M_impl._M_head);
}
template<typename _Tp, typename _Alloc>
@@ -362,11 +359,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__list._M_impl._M_head._M_next);
__node = __node->_M_next;
}
+
if (__list._M_impl._M_head._M_next)
- {
- __node->_M_next = __list._M_impl._M_head._M_next;
- __list._M_impl._M_head._M_next = 0;
- }
+ *__node = std::move(__list._M_impl._M_head);
}
template<typename _Tp, typename _Alloc>
@@ -397,7 +392,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
forward_list<_Tp, _Alloc>::
sort(_Comp __comp)
{
- // If `next' is 0, return immediately.
+ // If `next' is nullptr, return immediately.
_Node* __list = static_cast<_Node*>(this->_M_impl._M_head._M_next);
if (!__list)
return;
@@ -407,8 +402,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
while (1)
{
_Node* __p = __list;
- __list = 0;
- _Node* __tail = 0;
+ __list = nullptr;
+ _Node* __tail = nullptr;
// Count number of merges we do in this pass.
unsigned long __nmerges = 0;
@@ -476,7 +471,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
// Now p has stepped `insize' places along, and q has too.
__p = __q;
}
- __tail->_M_next = 0;
+ __tail->_M_next = nullptr;
// If we have done only one merge, we're finished.
// Allow for nmerges == 0, the empty list case.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-07-17 20:10 std::forward_list optim for always equal allocator François Dumont
@ 2017-07-17 20:14 ` Daniel Krügler
2017-08-28 20:39 ` François Dumont
1 sibling, 0 replies; 13+ messages in thread
From: Daniel Krügler @ 2017-07-17 20:14 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
2017-07-17 22:10 GMT+02:00 François Dumont <frs.dumont@gmail.com>:
> Hi
>
> Here is the patch to implement the always equal alloc optimization for
> forward_list. With this version there is no abi issue.
>
> I also prefer to implement the _Fwd_list_node_base move operator for
> consistency with the move constructor and used it where applicable.
>
>
> * include/bits/forward_list.h
> (_Fwd_list_node_base& operator=(_Fwd_list_node_base&&)): Implement.
> (_Fwd_list_impl(_Fwd_list_impl&&, _Node_alloc_type&&)): New.
> (_Fwd_list_base(_Fwd_list_base&&, _Node_alloc_type&&, std::true_type)):
> New, use latter.
> (forward_list(forward_list&&, _Node_alloc_type&&, std::false_type)):
> New.
> (forward_list(forward_list&&, _Node_alloc_type&&, std::true_type)):
> New.
> (forward_list(forward_list&&, const _Alloc&)): Adapt to use latters.
> * include/bits/forward_list.tcc
> (_Fwd_list_base(_Fwd_list_base&&, _Node_alloc_type&&)): Adapt to use
> _M_impl._M_head move assignment.
> (forward_list<>::merge(forward_list<>&&, _Comp)): Likewise.
>
> Tested under Linux x86_64, ok to commit ?
Out of curiosity: Shouldn't
_Fwd_list_node_base&
operator=(_Fwd_list_node_base&& __x);
be declared noexcept?
Thanks,
- Daniel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-07-17 20:10 std::forward_list optim for always equal allocator François Dumont
2017-07-17 20:14 ` Daniel Krügler
@ 2017-08-28 20:39 ` François Dumont
2017-09-08 16:19 ` Jonathan Wakely
2017-11-23 21:49 ` François Dumont
1 sibling, 2 replies; 13+ messages in thread
From: François Dumont @ 2017-08-28 20:39 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]
Hi
Any news for this patch ?
It does remove a constructor:
- _Fwd_list_impl(const _Node_alloc_type& __a)
- : _Node_alloc_type(__a), _M_head()
It was already unused before the patch. Do you think it has ever
been used and so do I need to restore it ?
I eventually restore the _M_head() in _Fwd_list_impl constructors
cause IMO it is the inline init of _M_next in _Fwd_list_node_base which
should be removed. But I remember Jonathan that you didn't want to do so
because gcc was not good enough in detecting usage of uninitialized
variables, is it still true ?
François
On 17/07/2017 22:10, François Dumont wrote:
> Hi
>
> Here is the patch to implement the always equal alloc optimization
> for forward_list. With this version there is no abi issue.
>
> I also prefer to implement the _Fwd_list_node_base move operator
> for consistency with the move constructor and used it where applicable.
>
>
> * include/bits/forward_list.h
> (_Fwd_list_node_base& operator=(_Fwd_list_node_base&&)): Implement.
> (_Fwd_list_impl(_Fwd_list_impl&&, _Node_alloc_type&&)): New.
> (_Fwd_list_base(_Fwd_list_base&&, _Node_alloc_type&&,
> std::true_type)):
> New, use latter.
> (forward_list(forward_list&&, _Node_alloc_type&&, std::false_type)):
> New.
> (forward_list(forward_list&&, _Node_alloc_type&&, std::true_type)):
> New.
> (forward_list(forward_list&&, const _Alloc&)): Adapt to use latters.
> * include/bits/forward_list.tcc
> (_Fwd_list_base(_Fwd_list_base&&, _Node_alloc_type&&)): Adapt to use
> _M_impl._M_head move assignment.
> (forward_list<>::merge(forward_list<>&&, _Comp)): Likewise.
>
> Tested under Linux x86_64, ok to commit ?
>
> François
>
[-- Attachment #2: forward_list.patch --]
[-- Type: text/x-patch, Size: 12183 bytes --]
diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h
index 9d86fcc..772e9a0 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -54,6 +54,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
struct _Fwd_list_node_base
{
_Fwd_list_node_base() = default;
+ _Fwd_list_node_base(_Fwd_list_node_base&& __x) noexcept
+ : _M_next(__x._M_next)
+ { __x._M_next = nullptr; }
+
+ _Fwd_list_node_base(const _Fwd_list_node_base&) = delete;
+ _Fwd_list_node_base& operator=(const _Fwd_list_node_base&) = delete;
+
+ _Fwd_list_node_base&
+ operator=(_Fwd_list_node_base&& __x) noexcept
+ {
+ _M_next = __x._M_next;
+ __x._M_next = nullptr;
+ return *this;
+ }
_Fwd_list_node_base* _M_next = nullptr;
@@ -68,7 +82,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__end->_M_next = _M_next;
}
else
- __begin->_M_next = 0;
+ __begin->_M_next = nullptr;
_M_next = __keep;
return __end;
}
@@ -173,7 +187,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
if (_M_node)
return _Fwd_list_iterator(_M_node->_M_next);
else
- return _Fwd_list_iterator(0);
+ return _Fwd_list_iterator(nullptr);
}
_Fwd_list_node_base* _M_node;
@@ -244,7 +258,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
if (this->_M_node)
return _Fwd_list_const_iterator(_M_node->_M_next);
else
- return _Fwd_list_const_iterator(0);
+ return _Fwd_list_const_iterator(nullptr);
}
const _Fwd_list_node_base* _M_node;
@@ -285,11 +299,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_Fwd_list_node_base _M_head;
_Fwd_list_impl()
+ noexcept( noexcept(_Node_alloc_type()) )
: _Node_alloc_type(), _M_head()
{ }
- _Fwd_list_impl(const _Node_alloc_type& __a)
- : _Node_alloc_type(__a), _M_head()
+ _Fwd_list_impl(_Fwd_list_impl&&) = default;
+
+ _Fwd_list_impl(_Fwd_list_impl&& __fl, _Node_alloc_type&& __a)
+ : _Node_alloc_type(std::move(__a)), _M_head(std::move(__fl._M_head))
{ }
_Fwd_list_impl(_Node_alloc_type&& __a)
@@ -312,26 +329,26 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_M_get_Node_allocator() const noexcept
{ return this->_M_impl; }
- _Fwd_list_base()
- : _M_impl() { }
+ _Fwd_list_base() = default;
_Fwd_list_base(_Node_alloc_type&& __a)
: _M_impl(std::move(__a)) { }
+ // When allocators are always equal.
+ _Fwd_list_base(_Fwd_list_base&& __lst, _Node_alloc_type&& __a,
+ std::true_type)
+ : _M_impl(std::move(__lst._M_impl), std::move(__a))
+ { }
+
+ // When allocators are not always equal.
_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()))
- {
- this->_M_impl._M_head._M_next = __lst._M_impl._M_head._M_next;
- __lst._M_impl._M_head._M_next = 0;
- }
+ _Fwd_list_base(_Fwd_list_base&&) = default;
~_Fwd_list_base()
- { _M_erase_after(&_M_impl._M_head, 0); }
+ { _M_erase_after(&_M_impl._M_head, nullptr); }
protected:
-
_Node*
_M_get_node()
{
@@ -437,10 +454,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/**
* @brief Creates a %forward_list with no elements.
*/
- forward_list()
- noexcept(is_nothrow_default_constructible<_Node_alloc_type>::value)
- : _Base()
- { }
+ forward_list() = default;
/**
* @brief Creates a %forward_list with no elements.
@@ -451,7 +465,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _Base(_Node_alloc_type(__al))
{ }
-
/**
* @brief Copy constructor with allocator argument.
* @param __list Input list to copy.
@@ -461,14 +474,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _Base(_Node_alloc_type(__al))
{ _M_range_initialize(__list.begin(), __list.end()); }
- /**
- * @brief Move constructor with allocator argument.
- * @param __list Input list to move.
- * @param __al An allocator object.
- */
- forward_list(forward_list&& __list, const _Alloc& __al)
- noexcept(_Node_alloc_traits::_S_always_equal())
- : _Base(std::move(__list), _Node_alloc_type(__al))
+ private:
+ forward_list(forward_list&& __list, _Node_alloc_type&& __al,
+ std::false_type)
+ : _Base(std::move(__list), std::move(__al))
{
// If __list is not empty it means its allocator is not equal to __a,
// so we need to move from each element individually.
@@ -477,6 +486,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
std::__make_move_if_noexcept_iterator(__list.end()));
}
+ forward_list(forward_list&& __list, _Node_alloc_type&& __al,
+ std::true_type)
+ noexcept
+ : _Base(std::move(__list), _Node_alloc_type(__al), std::true_type{})
+ { }
+
+ public:
+ /**
+ * @brief Move constructor with allocator argument.
+ * @param __list Input list to move.
+ * @param __al An allocator object.
+ */
+ forward_list(forward_list&& __list, const _Alloc& __al)
+ noexcept(_Node_alloc_traits::_S_always_equal())
+ : forward_list(std::move(__list), _Node_alloc_type(__al),
+ typename _Node_alloc_traits::is_always_equal{})
+ { }
+
/**
* @brief Creates a %forward_list with default constructed elements.
* @param __n The number of elements to initially create.
@@ -533,15 +560,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/**
* @brief The %forward_list move constructor.
- * @param __list A %forward_list of identical element and allocator
- * types.
+ * @param A %forward_list of identical element and allocator types.
*
- * The newly-created %forward_list contains the exact contents of @a
- * __list. The contents of @a __list are a valid, but unspecified
- * %forward_list.
+ * The newly-created %forward_list contains the exact contents of the
+ * moved instance. The contents of the moved instance are a valid, but
+ * unspecified %forward_list.
*/
- forward_list(forward_list&& __list) noexcept
- : _Base(std::move(__list)) { }
+ forward_list(forward_list&&) = default;
/**
* @brief Builds a %forward_list from an initializer_list
@@ -707,7 +732,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
iterator
end() noexcept
- { return iterator(0); }
+ { return iterator(nullptr); }
/**
* Returns a read-only iterator that points one past the last
@@ -716,7 +741,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_iterator
end() const noexcept
- { return const_iterator(0); }
+ { return const_iterator(nullptr); }
/**
* Returns a read-only (constant) iterator that points to the
@@ -743,7 +768,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_iterator
cend() const noexcept
- { return const_iterator(0); }
+ { return const_iterator(nullptr); }
/**
* Returns true if the %forward_list is empty. (Thus begin() would
@@ -751,7 +776,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
bool
empty() const noexcept
- { return this->_M_impl._M_head._M_next == 0; }
+ { return this->_M_impl._M_head._M_next == nullptr; }
/**
* Returns the largest possible number of elements of %forward_list.
@@ -1056,7 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
void
clear() noexcept
- { this->_M_erase_after(&this->_M_impl._M_head, 0); }
+ { this->_M_erase_after(&this->_M_impl._M_head, nullptr); }
// 23.3.4.6 forward_list operations:
diff --git a/libstdc++-v3/include/bits/forward_list.tcc b/libstdc++-v3/include/bits/forward_list.tcc
index 64bd9c4..f13e959 100644
--- a/libstdc++-v3/include/bits/forward_list.tcc
+++ b/libstdc++-v3/include/bits/forward_list.tcc
@@ -41,12 +41,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _M_impl(std::move(__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;
+ this->_M_impl._M_head = std::move(__lst._M_impl._M_head);
}
template<typename _Tp, typename _Alloc>
@@ -364,11 +359,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__list._M_impl._M_head._M_next);
__node = __node->_M_next;
}
+
if (__list._M_impl._M_head._M_next)
- {
- __node->_M_next = __list._M_impl._M_head._M_next;
- __list._M_impl._M_head._M_next = 0;
- }
+ *__node = std::move(__list._M_impl._M_head);
}
template<typename _Tp, typename _Alloc>
@@ -399,7 +392,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
forward_list<_Tp, _Alloc>::
sort(_Comp __comp)
{
- // If `next' is 0, return immediately.
+ // If `next' is nullptr, return immediately.
_Node* __list = static_cast<_Node*>(this->_M_impl._M_head._M_next);
if (!__list)
return;
@@ -409,8 +402,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
while (1)
{
_Node* __p = __list;
- __list = 0;
- _Node* __tail = 0;
+ __list = nullptr;
+ _Node* __tail = nullptr;
// Count number of merges we do in this pass.
unsigned long __nmerges = 0;
@@ -478,7 +471,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
// Now p has stepped `insize' places along, and q has too.
__p = __q;
}
- __tail->_M_next = 0;
+ __tail->_M_next = nullptr;
// If we have done only one merge, we're finished.
// Allow for nmerges == 0, the empty list case.
@@ -498,4 +491,3 @@ _GLIBCXX_END_NAMESPACE_VERSION
} // namespace std
#endif /* _FORWARD_LIST_TCC */
-
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/allocator/default_init.cc b/libstdc++-v3/testsuite/23_containers/forward_list/allocator/default_init.cc
new file mode 100644
index 0000000..b56c9cc
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/allocator/default_init.cc
@@ -0,0 +1,67 @@
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++11 } }
+// { dg-options "-O0" }
+// { dg-xfail-run-if "PR c++/65816" { *-*-* } }
+
+#include <forward_list>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = int;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+ typedef default_init_allocator<T> alloc_type;
+ typedef std::forward_list<T, alloc_type> test_type;
+
+ __gnu_cxx::__aligned_buffer<test_type> buf;
+ __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+ test_type *tmp = ::new(buf._M_addr()) test_type;
+
+ VERIFY( tmp->get_allocator().state == 0 );
+
+ tmp->~test_type();
+}
+
+void test02()
+{
+ typedef default_init_allocator<T> alloc_type;
+ typedef std::forward_list<T, alloc_type> test_type;
+
+ __gnu_cxx::__aligned_buffer<test_type> buf;
+ __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+ test_type *tmp = ::new(buf._M_addr()) test_type();
+
+ VERIFY( tmp->get_allocator().state == 0 );
+
+ tmp->~test_type();
+}
+
+int main()
+{
+ test01();
+ test02();
+ return 0;
+}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-08-28 20:39 ` François Dumont
@ 2017-09-08 16:19 ` Jonathan Wakely
2017-09-11 5:12 ` François Dumont
2017-11-23 21:49 ` François Dumont
1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2017-09-08 16:19 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 28/08/17 21:09 +0200, François Dumont wrote:
>Hi
>
> Any news for this patch ?
>
> It does remove a constructor:
>- _Fwd_list_impl(const _Node_alloc_type& __a)
>- : _Node_alloc_type(__a), _M_head()
>
> It was already unused before the patch. Do you think it has ever
>been used and so do I need to restore it ?
>
> I eventually restore the _M_head() in _Fwd_list_impl constructors
>cause IMO it is the inline init of _M_next in _Fwd_list_node_base
>which should be removed. But I remember Jonathan that you didn't want
>to do so because gcc was not good enough in detecting usage of
>uninitialized variables, is it still true ?
Why should it be removed?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-09-08 16:19 ` Jonathan Wakely
@ 2017-09-11 5:12 ` François Dumont
2017-09-11 5:44 ` Daniel Krügler
0 siblings, 1 reply; 13+ messages in thread
From: François Dumont @ 2017-09-11 5:12 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
On 08/09/2017 18:19, Jonathan Wakely wrote:
> On 28/08/17 21:09 +0200, François Dumont wrote:
>> Hi
>>
>> Any news for this patch ?
>>
>> It does remove a constructor:
>> - _Fwd_list_impl(const _Node_alloc_type& __a)
>> - : _Node_alloc_type(__a), _M_head()
>>
>> It was already unused before the patch. Do you think it has ever
>> been used and so do I need to restore it ?
>>
>> I eventually restore the _M_head() in _Fwd_list_impl constructors
>> cause IMO it is the inline init of _M_next in _Fwd_list_node_base
>> which should be removed. But I remember Jonathan that you didn't want
>> to do so because gcc was not good enough in detecting usage of
>> uninitialized variables, is it still true ?
>
> Why should it be removed?
>
>
>
When user declare a container iterator like that:
std::forward_list<int>::iterator it;
There is no reason to initialize it with a null node pointer. It is just
an uninitialized iterator which is invalid to use except to initialize it.
I once proposed to do the same simplification for the unordered
containers _Hash_node_base. But you said that detection of the usage of
uninitialized variable is not good enough in gcc to leave variables
uninitialized this way.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-09-11 5:12 ` François Dumont
@ 2017-09-11 5:44 ` Daniel Krügler
2017-09-11 12:11 ` Jonathan Wakely
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Krügler @ 2017-09-11 5:44 UTC (permalink / raw)
To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches
2017-09-11 7:12 GMT+02:00 François Dumont <frs.dumont@gmail.com>:
> When user declare a container iterator like that:
>
> std::forward_list<int>::iterator it;
>
> There is no reason to initialize it with a null node pointer. It is just an
> uninitialized iterator which is invalid to use except to initialize it.
While that is correct, for every forward iterator (and
std::forward_list<int>::iterator meets these requirements), it is also
required that a value-initialized iterator can be compared against
other initialized iterators, so this reduces the amount of freedom to
define a default constructor for such iterators even when used to
default-initialize. This is not meant as a showstopper argument, since
I have not fully understood of what you are planning, but just a
reminder.
- Daniel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-09-11 5:44 ` Daniel Krügler
@ 2017-09-11 12:11 ` Jonathan Wakely
2017-09-11 20:36 ` François Dumont
0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2017-09-11 12:11 UTC (permalink / raw)
To: Daniel Krügler; +Cc: François Dumont, libstdc++, gcc-patches
On 11/09/17 07:44 +0200, Daniel Krügler wrote:
>2017-09-11 7:12 GMT+02:00 François Dumont <frs.dumont@gmail.com>:
>> When user declare a container iterator like that:
>>
>> std::forward_list<int>::iterator it;
>>
>> There is no reason to initialize it with a null node pointer. It is just an
>> uninitialized iterator which is invalid to use except to initialize it.
>
>While that is correct, for every forward iterator (and
>std::forward_list<int>::iterator meets these requirements), it is also
>required that a value-initialized iterator can be compared against
>other initialized iterators, so this reduces the amount of freedom to
>define a default constructor for such iterators even when used to
>default-initialize. This is not meant as a showstopper argument, since
>I have not fully understood of what you are planning, but just a
>reminder.
Right, which means that
std::forward_list<int>::iterator it = {};
must initialize the node pointer to nullptr. If we remove the
initialization of _Fwd_list_iterator<T>::_M_node from the default
constructor then it would be left uninitialized.
But I'm confused, François was talking about removing the
initialization of _Fwd_list_node_base::_M_next, what has that got to
do with forward_list<T>::iterator? Thee is no node-base in the
iterator.
So I'm still wondering why the initialization of _M_next should be
removed.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-09-11 12:11 ` Jonathan Wakely
@ 2017-09-11 20:36 ` François Dumont
2017-09-11 20:39 ` Daniel Krügler
0 siblings, 1 reply; 13+ messages in thread
From: François Dumont @ 2017-09-11 20:36 UTC (permalink / raw)
To: Jonathan Wakely, Daniel Krügler; +Cc: libstdc++, gcc-patches
On 11/09/2017 14:11, Jonathan Wakely wrote:
> On 11/09/17 07:44 +0200, Daniel Krügler wrote:
>> 2017-09-11 7:12 GMT+02:00 François Dumont <frs.dumont@gmail.com>:
>>> When user declare a container iterator like that:
>>>
>>> std::forward_list<int>::iterator it;
>>>
>>> There is no reason to initialize it with a null node pointer. It is
>>> just an
>>> uninitialized iterator which is invalid to use except to initialize it.
>>
>> While that is correct, for every forward iterator (and
>> std::forward_list<int>::iterator meets these requirements), it is also
>> required that a value-initialized iterator can be compared against
>> other initialized iterators, so this reduces the amount of freedom to
>> define a default constructor for such iterators even when used to
>> default-initialize. This is not meant as a showstopper argument, since
>> I have not fully understood of what you are planning, but just a
>> reminder.
>
> Right, which means that
> std::forward_list<int>::iterator it = {};
>
> must initialize the node pointer to nullptr. If we remove the
> initialization of _Fwd_list_iterator<T>::_M_node from the default
> constructor then it would be left uninitialized.
>
> But I'm confused, François was talking about removing the
> initialization of _Fwd_list_node_base::_M_next, what has that got to
> do with forward_list<T>::iterator? Thee is no node-base in the
> iterator.
>
> So I'm still wondering why the initialization of _M_next should be
> removed.
>
Indeed, the iterator contains a _Fwd_list_node_base*.
So my remark was rather for the:
_Fwd_list_iterator() noexcept
: _M_node() { }
that could simply be
_Fwd_list_iterator() = default;
no ?
François
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-09-11 20:36 ` François Dumont
@ 2017-09-11 20:39 ` Daniel Krügler
2017-09-11 22:10 ` Jonathan Wakely
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Krügler @ 2017-09-11 20:39 UTC (permalink / raw)
To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches
2017-09-11 22:36 GMT+02:00 François Dumont <frs.dumont@gmail.com>:
[..]
> So my remark was rather for the:
>
> _Fwd_list_iterator() noexcept
> : _M_node() { }
>
> that could simply be
>
> _Fwd_list_iterator() = default;
>
> no ?
Yes, that should be fine.
- Daniel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-09-11 20:39 ` Daniel Krügler
@ 2017-09-11 22:10 ` Jonathan Wakely
2017-09-12 20:41 ` François Dumont
0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2017-09-11 22:10 UTC (permalink / raw)
To: Daniel Krügler; +Cc: François Dumont, libstdc++, gcc-patches
On 11/09/17 22:39 +0200, Daniel Krügler wrote:
>2017-09-11 22:36 GMT+02:00 François Dumont <frs.dumont@gmail.com>:
>[..]
>> So my remark was rather for the:
>>
>> _Fwd_list_iterator() noexcept
>> : _M_node() { }
>>
>> that could simply be
>>
>> _Fwd_list_iterator() = default;
>>
>> no ?
>
>Yes, that should be fine.
I'm not sure there's much benefit to that change.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-09-11 22:10 ` Jonathan Wakely
@ 2017-09-12 20:41 ` François Dumont
0 siblings, 0 replies; 13+ messages in thread
From: François Dumont @ 2017-09-12 20:41 UTC (permalink / raw)
To: Jonathan Wakely, Daniel Krügler; +Cc: libstdc++, gcc-patches
On 12/09/2017 00:10, Jonathan Wakely wrote:
> On 11/09/17 22:39 +0200, Daniel Krügler wrote:
>> 2017-09-11 22:36 GMT+02:00 François Dumont <frs.dumont@gmail.com>:
>> [..]
>>> So my remark was rather for the:
>>>
>>> _Fwd_list_iterator() noexcept
>>> : _M_node() { }
>>>
>>> that could simply be
>>>
>>> _Fwd_list_iterator() = default;
>>>
>>> no ?
>>
>> Yes, that should be fine.
>
> I'm not sure there's much benefit to that change
Sure, it would be a minor change.
Which is moreover not part of this patch proposal. Is the patch ok to
commit ?
François
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-08-28 20:39 ` François Dumont
2017-09-08 16:19 ` Jonathan Wakely
@ 2017-11-23 21:49 ` François Dumont
2018-01-08 14:07 ` Jonathan Wakely
1 sibling, 1 reply; 13+ messages in thread
From: François Dumont @ 2017-11-23 21:49 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2687 bytes --]
Gentle reminder for this patch.
I looked when the constructor got unused and I think it is back in June
2015 in git commit:
commit debb6aabb771ed02cb7256a7719555e5fbd7d3f7
Author: redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:Â Â Wed Jun 17 17:45:45 2015 +0000
       * include/bits/forward_list.h
       (_Fwd_list_base(const _Node_alloc_type&)): Change parameter to
       rvalue-reference.
If you fear abi breaking change I can restore it in a
!_GLIBCXX_INLINE_VERSION section.
François
On 28/08/2017 21:09, François Dumont wrote:
> Hi
>
> Â Â Â Any news for this patch ?
>
> Â Â Â It does remove a constructor:
> -Â Â Â Â Â Â Â _Fwd_list_impl(const _Node_alloc_type& __a)
> -Â Â Â Â Â Â Â : _Node_alloc_type(__a), _M_head()
>
> Â Â Â Â It was already unused before the patch. Do you think it has ever
> been used and so do I need to restore it ?
>
> Â Â Â I eventually restore the _M_head() in _Fwd_list_impl constructors
> cause IMO it is the inline init of _M_next in _Fwd_list_node_base
> which should be removed. But I remember Jonathan that you didn't want
> to do so because gcc was not good enough in detecting usage of
> uninitialized variables, is it still true ?
>
> François
>
>
> On 17/07/2017 22:10, François Dumont wrote:
>> Hi
>>
>> Â Â Â Here is the patch to implement the always equal alloc
>> optimization for forward_list. With this version there is no abi issue.
>>
>> Â Â Â I also prefer to implement the _Fwd_list_node_base move operator
>> for consistency with the move constructor and used it where applicable.
>>
>>
>> Â Â Â * include/bits/forward_list.h
>> Â Â Â (_Fwd_list_node_base& operator=(_Fwd_list_node_base&&)): Implement.
>> Â Â Â (_Fwd_list_impl(_Fwd_list_impl&&, _Node_alloc_type&&)): New.
>> Â Â Â (_Fwd_list_base(_Fwd_list_base&&, _Node_alloc_type&&,
>> std::true_type)):
>> Â Â Â New, use latter.
>> Â Â Â (forward_list(forward_list&&, _Node_alloc_type&&, std::false_type)):
>> Â Â Â New.
>> Â Â Â (forward_list(forward_list&&, _Node_alloc_type&&, std::true_type)):
>> Â Â Â New.
>> Â Â Â (forward_list(forward_list&&, const _Alloc&)): Adapt to use latters.
>> Â Â Â * include/bits/forward_list.tcc
>> Â Â Â (_Fwd_list_base(_Fwd_list_base&&, _Node_alloc_type&&)): Adapt to use
>> Â Â Â _M_impl._M_head move assignment.
>> Â Â Â (forward_list<>::merge(forward_list<>&&, _Comp)): Likewise.
>>
>> Tested under Linux x86_64, ok to commit ?
>>
>> François
>>
>
[-- Attachment #2: forward_list.patch --]
[-- Type: text/x-patch, Size: 12182 bytes --]
diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h
index 9d86fcc..772e9a0 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -54,6 +54,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
struct _Fwd_list_node_base
{
_Fwd_list_node_base() = default;
+ _Fwd_list_node_base(_Fwd_list_node_base&& __x) noexcept
+ : _M_next(__x._M_next)
+ { __x._M_next = nullptr; }
+
+ _Fwd_list_node_base(const _Fwd_list_node_base&) = delete;
+ _Fwd_list_node_base& operator=(const _Fwd_list_node_base&) = delete;
+
+ _Fwd_list_node_base&
+ operator=(_Fwd_list_node_base&& __x) noexcept
+ {
+ _M_next = __x._M_next;
+ __x._M_next = nullptr;
+ return *this;
+ }
_Fwd_list_node_base* _M_next = nullptr;
@@ -68,7 +82,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__end->_M_next = _M_next;
}
else
- __begin->_M_next = 0;
+ __begin->_M_next = nullptr;
_M_next = __keep;
return __end;
}
@@ -173,7 +187,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
if (_M_node)
return _Fwd_list_iterator(_M_node->_M_next);
else
- return _Fwd_list_iterator(0);
+ return _Fwd_list_iterator(nullptr);
}
_Fwd_list_node_base* _M_node;
@@ -244,7 +258,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
if (this->_M_node)
return _Fwd_list_const_iterator(_M_node->_M_next);
else
- return _Fwd_list_const_iterator(0);
+ return _Fwd_list_const_iterator(nullptr);
}
const _Fwd_list_node_base* _M_node;
@@ -285,11 +299,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_Fwd_list_node_base _M_head;
_Fwd_list_impl()
+ noexcept( noexcept(_Node_alloc_type()) )
: _Node_alloc_type(), _M_head()
{ }
- _Fwd_list_impl(const _Node_alloc_type& __a)
- : _Node_alloc_type(__a), _M_head()
+ _Fwd_list_impl(_Fwd_list_impl&&) = default;
+
+ _Fwd_list_impl(_Fwd_list_impl&& __fl, _Node_alloc_type&& __a)
+ : _Node_alloc_type(std::move(__a)), _M_head(std::move(__fl._M_head))
{ }
_Fwd_list_impl(_Node_alloc_type&& __a)
@@ -312,26 +329,26 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_M_get_Node_allocator() const noexcept
{ return this->_M_impl; }
- _Fwd_list_base()
- : _M_impl() { }
+ _Fwd_list_base() = default;
_Fwd_list_base(_Node_alloc_type&& __a)
: _M_impl(std::move(__a)) { }
+ // When allocators are always equal.
+ _Fwd_list_base(_Fwd_list_base&& __lst, _Node_alloc_type&& __a,
+ std::true_type)
+ : _M_impl(std::move(__lst._M_impl), std::move(__a))
+ { }
+
+ // When allocators are not always equal.
_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()))
- {
- this->_M_impl._M_head._M_next = __lst._M_impl._M_head._M_next;
- __lst._M_impl._M_head._M_next = 0;
- }
+ _Fwd_list_base(_Fwd_list_base&&) = default;
~_Fwd_list_base()
- { _M_erase_after(&_M_impl._M_head, 0); }
+ { _M_erase_after(&_M_impl._M_head, nullptr); }
protected:
-
_Node*
_M_get_node()
{
@@ -437,10 +454,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/**
* @brief Creates a %forward_list with no elements.
*/
- forward_list()
- noexcept(is_nothrow_default_constructible<_Node_alloc_type>::value)
- : _Base()
- { }
+ forward_list() = default;
/**
* @brief Creates a %forward_list with no elements.
@@ -451,7 +465,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _Base(_Node_alloc_type(__al))
{ }
-
/**
* @brief Copy constructor with allocator argument.
* @param __list Input list to copy.
@@ -461,14 +474,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _Base(_Node_alloc_type(__al))
{ _M_range_initialize(__list.begin(), __list.end()); }
- /**
- * @brief Move constructor with allocator argument.
- * @param __list Input list to move.
- * @param __al An allocator object.
- */
- forward_list(forward_list&& __list, const _Alloc& __al)
- noexcept(_Node_alloc_traits::_S_always_equal())
- : _Base(std::move(__list), _Node_alloc_type(__al))
+ private:
+ forward_list(forward_list&& __list, _Node_alloc_type&& __al,
+ std::false_type)
+ : _Base(std::move(__list), std::move(__al))
{
// If __list is not empty it means its allocator is not equal to __a,
// so we need to move from each element individually.
@@ -477,6 +486,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
std::__make_move_if_noexcept_iterator(__list.end()));
}
+ forward_list(forward_list&& __list, _Node_alloc_type&& __al,
+ std::true_type)
+ noexcept
+ : _Base(std::move(__list), _Node_alloc_type(__al), std::true_type{})
+ { }
+
+ public:
+ /**
+ * @brief Move constructor with allocator argument.
+ * @param __list Input list to move.
+ * @param __al An allocator object.
+ */
+ forward_list(forward_list&& __list, const _Alloc& __al)
+ noexcept(_Node_alloc_traits::_S_always_equal())
+ : forward_list(std::move(__list), _Node_alloc_type(__al),
+ typename _Node_alloc_traits::is_always_equal{})
+ { }
+
/**
* @brief Creates a %forward_list with default constructed elements.
* @param __n The number of elements to initially create.
@@ -533,15 +560,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/**
* @brief The %forward_list move constructor.
- * @param __list A %forward_list of identical element and allocator
- * types.
+ * @param A %forward_list of identical element and allocator types.
*
- * The newly-created %forward_list contains the exact contents of @a
- * __list. The contents of @a __list are a valid, but unspecified
- * %forward_list.
+ * The newly-created %forward_list contains the exact contents of the
+ * moved instance. The contents of the moved instance are a valid, but
+ * unspecified %forward_list.
*/
- forward_list(forward_list&& __list) noexcept
- : _Base(std::move(__list)) { }
+ forward_list(forward_list&&) = default;
/**
* @brief Builds a %forward_list from an initializer_list
@@ -707,7 +732,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
iterator
end() noexcept
- { return iterator(0); }
+ { return iterator(nullptr); }
/**
* Returns a read-only iterator that points one past the last
@@ -716,7 +741,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_iterator
end() const noexcept
- { return const_iterator(0); }
+ { return const_iterator(nullptr); }
/**
* Returns a read-only (constant) iterator that points to the
@@ -743,7 +768,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_iterator
cend() const noexcept
- { return const_iterator(0); }
+ { return const_iterator(nullptr); }
/**
* Returns true if the %forward_list is empty. (Thus begin() would
@@ -751,7 +776,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
bool
empty() const noexcept
- { return this->_M_impl._M_head._M_next == 0; }
+ { return this->_M_impl._M_head._M_next == nullptr; }
/**
* Returns the largest possible number of elements of %forward_list.
@@ -1056,7 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
void
clear() noexcept
- { this->_M_erase_after(&this->_M_impl._M_head, 0); }
+ { this->_M_erase_after(&this->_M_impl._M_head, nullptr); }
// 23.3.4.6 forward_list operations:
diff --git a/libstdc++-v3/include/bits/forward_list.tcc b/libstdc++-v3/include/bits/forward_list.tcc
index 64bd9c4..f13e959 100644
--- a/libstdc++-v3/include/bits/forward_list.tcc
+++ b/libstdc++-v3/include/bits/forward_list.tcc
@@ -41,12 +41,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _M_impl(std::move(__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;
+ this->_M_impl._M_head = std::move(__lst._M_impl._M_head);
}
template<typename _Tp, typename _Alloc>
@@ -364,11 +359,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__list._M_impl._M_head._M_next);
__node = __node->_M_next;
}
+
if (__list._M_impl._M_head._M_next)
- {
- __node->_M_next = __list._M_impl._M_head._M_next;
- __list._M_impl._M_head._M_next = 0;
- }
+ *__node = std::move(__list._M_impl._M_head);
}
template<typename _Tp, typename _Alloc>
@@ -399,7 +392,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
forward_list<_Tp, _Alloc>::
sort(_Comp __comp)
{
- // If `next' is 0, return immediately.
+ // If `next' is nullptr, return immediately.
_Node* __list = static_cast<_Node*>(this->_M_impl._M_head._M_next);
if (!__list)
return;
@@ -409,8 +402,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
while (1)
{
_Node* __p = __list;
- __list = 0;
- _Node* __tail = 0;
+ __list = nullptr;
+ _Node* __tail = nullptr;
// Count number of merges we do in this pass.
unsigned long __nmerges = 0;
@@ -478,7 +471,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
// Now p has stepped `insize' places along, and q has too.
__p = __q;
}
- __tail->_M_next = 0;
+ __tail->_M_next = nullptr;
// If we have done only one merge, we're finished.
// Allow for nmerges == 0, the empty list case.
@@ -498,4 +491,3 @@ _GLIBCXX_END_NAMESPACE_VERSION
} // namespace std
#endif /* _FORWARD_LIST_TCC */
-
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/allocator/default_init.cc b/libstdc++-v3/testsuite/23_containers/forward_list/allocator/default_init.cc
new file mode 100644
index 0000000..b56c9cc
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/allocator/default_init.cc
@@ -0,0 +1,67 @@
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++11 } }
+// { dg-options "-O0" }
+// { dg-xfail-run-if "PR c++/65816" { *-*-* } }
+
+#include <forward_list>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = int;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+ typedef default_init_allocator<T> alloc_type;
+ typedef std::forward_list<T, alloc_type> test_type;
+
+ __gnu_cxx::__aligned_buffer<test_type> buf;
+ __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+ test_type *tmp = ::new(buf._M_addr()) test_type;
+
+ VERIFY( tmp->get_allocator().state == 0 );
+
+ tmp->~test_type();
+}
+
+void test02()
+{
+ typedef default_init_allocator<T> alloc_type;
+ typedef std::forward_list<T, alloc_type> test_type;
+
+ __gnu_cxx::__aligned_buffer<test_type> buf;
+ __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+ test_type *tmp = ::new(buf._M_addr()) test_type();
+
+ VERIFY( tmp->get_allocator().state == 0 );
+
+ tmp->~test_type();
+}
+
+int main()
+{
+ test01();
+ test02();
+ return 0;
+}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: std::forward_list optim for always equal allocator
2017-11-23 21:49 ` François Dumont
@ 2018-01-08 14:07 ` Jonathan Wakely
0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Wakely @ 2018-01-08 14:07 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 23/11/17 22:22 +0100, François Dumont wrote:
>Gentle reminder for this patch.
>
>I looked when the constructor got unused and I think it is back in
>June 2015 in git commit:
>
>commit debb6aabb771ed02cb7256a7719555e5fbd7d3f7
>Author: redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4>
>Date:Â Â Wed Jun 17 17:45:45 2015 +0000
>
>Â Â Â Â Â Â Â * include/bits/forward_list.h
>Â Â Â Â Â Â Â (_Fwd_list_base(const _Node_alloc_type&)): Change parameter to
>Â Â Â Â Â Â Â rvalue-reference.
Hmm, I should have put that same change on the gcc-5-branch too.
>If you fear abi breaking change I can restore it in a
>!_GLIBCXX_INLINE_VERSION section.
I think if there was a problem here my June 2015 change would already
have caused it (when I changed the _Fwd_list_base constructor
signatures).
So let's assume it's OK to remove the constructor.
>@@ -533,15 +560,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>
> /**
> * @brief The %forward_list move constructor.
>- * @param __list A %forward_list of identical element and allocator
>- * types.
>+ * @param A %forward_list of identical element and allocator types.
This change is wrong, you can't just remove the parameter name,
because now Doxygen will document a parameter called "A" (and complain
that there is no such parameter).
It would be better to leave the name __list there and just get the
warning.
Otherwise the patch is OK for trunk (please ensure to update the
Copyright dates in the test files to 2018).
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-01-08 13:53 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-17 20:10 std::forward_list optim for always equal allocator François Dumont
2017-07-17 20:14 ` Daniel Krügler
2017-08-28 20:39 ` François Dumont
2017-09-08 16:19 ` Jonathan Wakely
2017-09-11 5:12 ` François Dumont
2017-09-11 5:44 ` Daniel Krügler
2017-09-11 12:11 ` Jonathan Wakely
2017-09-11 20:36 ` François Dumont
2017-09-11 20:39 ` Daniel Krügler
2017-09-11 22:10 ` Jonathan Wakely
2017-09-12 20:41 ` François Dumont
2017-11-23 21:49 ` François Dumont
2018-01-08 14:07 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).