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 <jwak...@redhat.com>
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 <bits/allocated_ptr.h>
+
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<typename _NodeAlloc>
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<typename _NodeAlloc>
