[patch] Simplify allocator use
A couple of related patches that came out of some work on std::list
and std::string which is not ready yet.
One adds an alias template to simplify rebinding allocators.
The other adds an RAII type to help manage pointers obtained from
allocators. The new type means I can remove several ugly try-catch
blocks that are all very similar in structure and have been bothering
me for some time. The new type also makes it trivial to support
allocators with fancy pointers, fixing long-standing (but not very
important) bugs in std::promise and std::shared_ptr.
Tested x86_64-linux, committed to trunk.
commit 3309b9fc32a266f5bf296aec8e8fcbbe6f41d81b
Author: Jonathan Wakely
Date: Wed Jun 25 20:16:16 2014 +0100
* include/bits/alloc_traits.h (__alloc_rebind): Define alias template.
* include/bits/forward_list.h (_Fwd_list_base): Use __alloc_rebind.
* include/bits/hashtable_policy.h (_Insert_base, _Hashtable_alloc):
Likewise.
* include/ext/alloc_traits.h: Fix comment.
diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h
index 23fe8de..3afcc6f 100644
--- a/libstdc++-v3/include/bits/alloc_traits.h
+++ b/libstdc++-v3/include/bits/alloc_traits.h
@@ -72,6 +72,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typedef _Alloc<_Tp, _Args...> __type;
};
+ template
+using __alloc_rebind = typename __alloctr_rebind<_Ptr, _Tp>::__type;
+
/**
* @brief Uniform interface to all allocator types.
* @ingroup allocators
diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h
index 524fad1..7cf699e 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -274,13 +274,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
struct _Fwd_list_base
{
protected:
- typedef typename __gnu_cxx::__alloc_traits<_Alloc> _Alloc_traits;
- typedef typename _Alloc_traits::template rebind<_Tp>::other
-_Tp_alloc_type;
-
- typedef typename _Alloc_traits::template
-rebind<_Fwd_list_node<_Tp>>::other _Node_alloc_type;
-
+ typedef __alloc_rebind<_Alloc, _Tp> _Tp_alloc_type;
+ typedef __alloc_rebind<_Alloc, _Fwd_list_node<_Tp>> _Node_alloc_type;
typedef __gnu_cxx::__alloc_traits<_Node_alloc_type> _Node_alloc_traits;
struct _Fwd_list_impl
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index ef15b0e..606fbab 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -701,8 +701,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
using __unique_keys = typename __hashtable_base::__unique_keys;
using __ireturn_type = typename __hashtable_base::__ireturn_type;
using __node_type = _Hash_node<_Value, _Traits::__hash_cached::value>;
- using __node_alloc_type =
- typename __alloctr_rebind<_Alloc, __node_type>::__type;
+ using __node_alloc_type = __alloc_rebind<_Alloc, __node_type>;
using __node_gen_type = _AllocNode<__node_alloc_type>;
__hashtable&
@@ -1898,13 +1897,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
using __value_type = typename __node_type::value_type;
using __value_alloc_type =
- typename __alloctr_rebind<__node_alloc_type, __value_type>::__type;
+ __alloc_rebind<__node_alloc_type, __value_type>;
using __value_alloc_traits = std::allocator_traits<__value_alloc_type>;
using __node_base = __detail::_Hash_node_base;
using __bucket_type = __node_base*;
using __bucket_alloc_type =
- typename __alloctr_rebind<__node_alloc_type, __bucket_type>::__type;
+ __alloc_rebind<__node_alloc_type, __bucket_type>;
using __bucket_alloc_traits = std::allocator_traits<__bucket_alloc_type>;
_Hashtable_alloc(const _Hashtable_alloc&) = default;
diff --git a/libstdc++-v3/include/ext/alloc_traits.h b/libstdc++-v3/include/ext/alloc_traits.h
index 14fbc43..abdb611 100644
--- a/libstdc++-v3/include/ext/alloc_traits.h
+++ b/libstdc++-v3/include/ext/alloc_traits.h
@@ -210,6 +210,6 @@ template
};
_GLIBCXX_END_NAMESPACE_VERSION
-} // namespace std
+} // namespace __gnu_cxx
#endif
commit 6339d3559931228c0c3e4ce6e0cebcb0b0b0265d
Author: Jonathan Wakely
Date: Wed Jun 25 16:03:52 2014 +0100
* include/Makefile.am: Add new header.
* include/Makefile.in: Regenerate.
* include/bits/allocated_ptr.h (__allocated_ptr, __allocate_guarded):
New RAII utilities for working with allocators.
* include/bits/shared_ptr_base.h (_Sp_counted_deleter): Define
__allocator_type typedef and use new __allocated_ptr type.
(_Sp_counted_ptr_inplace): Likewise.
(__shared_count::__shared_count, __shared_ptr::__shared_ptr): Use
__allocate_guarded to to simplify exception handling.
* include/experimental/any (any::_Manager_alloc::_S_alloc): Likewise.
* include/std/future (_Result_alloc::_M_destroy): Likewise.
(_Re
Re: [patch] Simplify allocator use
This simplifies some of the test changes in my last patch, I was
misusing the CustomPointerAlloc due to confusion with some uncommitted
changes.
Tested x86_64-linux, committed to trunk.
commit d1a05535e99bfecb427829d3e03ef82e0977e60c
Author: Jonathan Wakely
Date: Wed Jun 25 23:39:20 2014 +0100
* testsuite/20_util/shared_ptr/creation/alloc.cc: Fix use of test
allocator.
* testsuite/20_util/shared_ptr/creation/no_rtti.cc: Likewise.
* testsuite/30_threads/promise/cons/alloc.cc: Likewise.
diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/creation/alloc.cc b/libstdc++-v3/testsuite/20_util/shared_ptr/creation/alloc.cc
index 402c612..0628807 100644
--- a/libstdc++-v3/testsuite/20_util/shared_ptr/creation/alloc.cc
+++ b/libstdc++-v3/testsuite/20_util/shared_ptr/creation/alloc.cc
@@ -101,16 +101,10 @@ test02()
== tracker_allocator_counter::get_deallocation_count() );
}
-template
- struct Pointer : __gnu_test::PointerBase, T>
- {
-using __gnu_test::PointerBase, T>::PointerBase;
- };
-
void
test03()
{
- __gnu_test::CustomPointerAlloc> alloc;
+ __gnu_test::CustomPointerAlloc alloc;
auto p = std::allocate_shared(alloc, 1);
VERIFY( *p == 1 );
}
diff --git a/libstdc++-v3/testsuite/20_util/shared_ptr/creation/no_rtti.cc b/libstdc++-v3/testsuite/20_util/shared_ptr/creation/no_rtti.cc
index 127bafb..d1fab6c 100644
--- a/libstdc++-v3/testsuite/20_util/shared_ptr/creation/no_rtti.cc
+++ b/libstdc++-v3/testsuite/20_util/shared_ptr/creation/no_rtti.cc
@@ -29,13 +29,7 @@ struct X { };
// test allocate_shared with no RTTI
-template
- struct Pointer : __gnu_test::PointerBase, T>
- {
-using __gnu_test::PointerBase, T>::PointerBase;
- };
-
-__gnu_test::CustomPointerAlloc> alloc;
+__gnu_test::CustomPointerAlloc alloc;
auto p = std::allocate_shared(alloc);
diff --git a/libstdc++-v3/testsuite/30_threads/promise/cons/alloc.cc b/libstdc++-v3/testsuite/30_threads/promise/cons/alloc.cc
index c45e646..ea9bb1a 100644
--- a/libstdc++-v3/testsuite/30_threads/promise/cons/alloc.cc
+++ b/libstdc++-v3/testsuite/30_threads/promise/cons/alloc.cc
@@ -38,16 +38,9 @@ void test01()
VERIFY( p1.get_future().get() == 5 );
}
-template
- struct Pointer : __gnu_test::PointerBase, T>
- {
-using __gnu_test::PointerBase, T>::PointerBase;
- };
-
-void
-test02()
+void test02()
{
- __gnu_test::CustomPointerAlloc> alloc;
+ __gnu_test::CustomPointerAlloc alloc;
promise p1(allocator_arg, alloc);
p1.set_value(5);
VERIFY( p1.get_future().get() == 5 );
Re: [patch] Simplify allocator use
On 25/06/14 21:56 +0100, Jonathan Wakely wrote:
The other adds an RAII type to help manage pointers obtained from
allocators. The new type means I can remove several ugly try-catch
blocks that are all very similar in structure and have been bothering
me for some time. The new type also makes it trivial to support
allocators with fancy pointers, fixing long-standing (but not very
important) bugs in std::promise and std::shared_ptr.
This patch applies the __allocated_ptr type to hashtable_policy.h to
remove most explicit deallocation (yay!) The buckets are still
allocated and deallocated manually, because __allocated_ptr only works
for allocations of single objects, not arrays.
As well as __allocated_ptr this change relies on two things:
1) the node type has a trivial destructor, so we don't actually need
to call it, we can just reuse or release its storage.
(See 3.8 [basic.life] p1)
2) allocator_traits::construct and allocator_traits::destroy can be
used with an allocator that has a different value_type, so we don't
need to create a rebound copy to destroy every element, we can just
use the node-allocator.
(See http://cplusplus.github.io/LWG/lwg-active.html#2218 which is
Open, but I've discussed the issue with Howard, Pablo and others,
and I think libc++ already relies on this assumption).
François, could you check it, and let me know if you see anything
wrong or have any comments?
commit d2fd02daab715c79c766bc0a476d1d01da1fc305
Author: Jonathan Wakely
Date: Thu Jun 26 12:28:56 2014 +0100
* include/bits/hashtable_policy.h (_ReuseOrAllocNode::operator()): Use
__allocated_ptr.
(_Hashtable_alloc::_M_allocate_node): Likewise.
(_Hashtable_alloc::_M_deallocate_node): Likewise.
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index 606fbab..ed6b2d7 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -31,6 +31,8 @@
#ifndef _HASHTABLE_POLICY_H
#define _HASHTABLE_POLICY_H 1
+#include
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -137,20 +139,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__node_type* __node = _M_nodes;
_M_nodes = _M_nodes->_M_next();
__node->_M_nxt = nullptr;
- __value_alloc_type __a(_M_h._M_node_allocator());
- __value_alloc_traits::destroy(__a, __node->_M_valptr());
- __try
- {
- __value_alloc_traits::construct(__a, __node->_M_valptr(),
- std::forward<_Arg>(__arg));
- }
- __catch(...)
- {
- __node->~__node_type();
- __node_alloc_traits::deallocate(_M_h._M_node_allocator(),
- __node, 1);
- __throw_exception_again;
- }
+ auto& __a = _M_h._M_node_allocator();
+ __node_alloc_traits::destroy(__a, __node->_M_valptr());
+ __allocated_ptr<_NodeAlloc> __guard{__a, __node};
+ __node_alloc_traits::construct(__a, __node->_M_valptr(),
+ std::forward<_Arg>(__arg));
+ __guard = nullptr;
return __node;
}
return _M_h._M_allocate_node(std::forward<_Arg>(__arg));
@@ -1947,33 +1941,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typename _Hashtable_alloc<_NodeAlloc>::__node_type*
_Hashtable_alloc<_NodeAlloc>::_M_allocate_node(_Args&&... __args)
{
- auto __nptr = __node_alloc_traits::allocate(_M_node_allocator(), 1);
- __node_type* __n = std::__addressof(*__nptr);
- __try
- {
- __value_alloc_type __a(_M_node_allocator());
- ::new ((void*)__n) __node_type;
- __value_alloc_traits::construct(__a, __n->_M_valptr(),
- std::forward<_Args>(__args)...);
- return __n;
- }
- __catch(...)
- {
- __node_alloc_traits::deallocate(_M_node_allocator(), __nptr, 1);
- __throw_exception_again;
- }
+ auto& __a = _M_node_allocator();
+ auto __guard = std::__allocate_guarded(__a);
+ __node_type* __n = __guard.get();
+ ::new ((void*)__n) __node_type;
+ __node_alloc_traits::construct(__a, __n->_M_valptr(),
+ std::forward<_Args>(__args)...);
+ __guard = nullptr;
+ return __n;
}
template
void
_Hashtable_alloc<_NodeAlloc>::_M_deallocate_node(__node_type* __n)
{
- typedef typename __node_alloc_traits::pointer _Ptr;
- auto __ptr = std::pointer_traits<_Ptr>::pointer_to(*__n);
- __value_alloc_type __a(_M_node_allocator());
- __value_alloc_traits::destroy(__a, __n->_M_valptr());
- __n->~__node_type();
- __node_alloc_traits::deallocate(_M_node_allocator(), __ptr, 1);
+ static_assert(std::is_trivially_destructible<__node_type>::value,
+ "Nodes must not require non-trivial destruction");
+ auto& __alloc = _M_node_allocator();
+ __allocated_ptr<__node_alloc_type> __guard{__alloc, __n};
+ __node_alloc_traits::destroy(__alloc, __n->_M_valptr());
}
template
Re: [patch] Simplify allocator use
On 26/06/14 12:31 +0100, Jonathan Wakely wrote:
@@ -137,20 +139,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__node_type* __node = _M_nodes;
_M_nodes = _M_nodes->_M_next();
__node->_M_nxt = nullptr;
- __value_alloc_type __a(_M_h._M_node_allocator());
- __value_alloc_traits::destroy(__a, __node->_M_valptr());
- __try
- {
- __value_alloc_traits::construct(__a, __node->_M_valptr(),
- std::forward<_Arg>(__arg));
- }
- __catch(...)
- {
- __node->~__node_type();
- __node_alloc_traits::deallocate(_M_h._M_node_allocator(),
- __node, 1);
I forgot to mention the change also fixes a bug in the line above:
__node should be converted to the allocator's pointer type before
passing it to deallocate. Using __allocated_ptr takes care of that.
Re: [patch] Simplify allocator use
On 26/06/14 00:06 +0100, Jonathan Wakely wrote:
This simplifies some of the test changes in my last patch, I was
misusing the CustomPointerAlloc due to confusion with some uncommitted
changes.
And this fixes the -fno-rtti version of make_shared, I shouldn't have
changed the deleter's parameter to the allocator's pointer. That
worked with the current test, but only because our CustomPointerAlloc
uses a custom pointer that is implicitly-convertible from value_type*.
I have a completely rewritten custom pointer for the testsuite which
doesn't support implicit conversions (only the minimum requirements)
and that caught this bug. The new custom pointer type is proving very
useful while I'm making some std::list changes but isn't ready for
prime-time yet.
Tested x86_64-linux, committed to trunk.
commit e69d8134edde691db7ea2567032229b210dd263d
Author: Jonathan Wakely
Date: Thu Jun 26 13:27:30 2014 +0100
* include/bits/shared_ptr_base.h (__shared_ptr::_Deleter): Fix
parameter type.
diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h
index 590a8d3..6f85ffa 100644
--- a/libstdc++-v3/include/bits/shared_ptr_base.h
+++ b/libstdc++-v3/include/bits/shared_ptr_base.h
@@ -1085,7 +1085,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template
struct _Deleter
{
- void operator()(typename _Alloc::pointer __ptr)
+ void operator()(_Tp* __ptr)
{
__allocated_ptr<_Alloc> __guard{ _M_alloc, __ptr };
allocator_traits<_Alloc>::destroy(_M_alloc, __guard.get());
Re: [patch] Simplify allocator use
On 26/06/2014 13:31, Jonathan Wakely wrote: On 25/06/14 21:56 +0100, Jonathan Wakely wrote: The other adds an RAII type to help manage pointers obtained from allocators. The new type means I can remove several ugly try-catch blocks that are all very similar in structure and have been bothering me for some time. The new type also makes it trivial to support allocators with fancy pointers, fixing long-standing (but not very important) bugs in std::promise and std::shared_ptr. This patch applies the __allocated_ptr type to hashtable_policy.h to remove most explicit deallocation (yay!) The buckets are still allocated and deallocated manually, because __allocated_ptr only works for allocations of single objects, not arrays. As well as __allocated_ptr this change relies on two things: 1) the node type has a trivial destructor, so we don't actually need to call it, we can just reuse or release its storage. (See 3.8 [basic.life] p1) 2) allocator_traits::construct and allocator_traits::destroy can be used with an allocator that has a different value_type, so we don't need to create a rebound copy to destroy every element, we can just use the node-allocator. (See http://cplusplus.github.io/LWG/lwg-active.html#2218 which is Open, but I've discussed the issue with Howard, Pablo and others, and I think libc++ already relies on this assumption). François, could you check it, and let me know if you see anything wrong or have any comments? That looks fine to me, nice simplification. François
