[PATCH][Hashtable 3/6] Fix noexcept qualifications
Jonathan Wakely
jwakely@redhat.com
Fri Jul 17 09:55:22 GMT 2020
On 17/11/19 21:59 +0100, François Dumont wrote:
>This patch adds noexcept qualification on allocator aware constructors
>and fix the one on the default constructor.
>
>Â Â Â * include/bits/hashtable.h
>Â Â Â (_Hashtable(_Hashtable&& __ht, __node_alloc_type&& __a, true_type)):
>Â Â Â Add noexcept qualification.
>Â Â Â (_Hashtable(_Hashtable&&)): Fix noexcept qualification.
>Â Â Â (_Hashtable(_Hashtable&&, const allocator_type&)): Add noexcept
>Â Â Â qualification.
>Â Â Â * include/bits/unordered_map.h
>Â Â Â (unordered_map(unordered_map&&, const allocator_type&)): Add noexcept
>Â Â Â qualification.
>Â Â Â (unordered_multimap(unordered_multimap&&, const allocator_type&)): Add
>Â Â Â noexcept qualification.
>Â Â Â * include/bits/unordered_set.h
>Â Â Â (unordered_set(unordered_set&&, const allocator_type&)): Add noexcept
>Â Â Â qualification.
>Â Â Â (unordered_multiset(unordered_multiset&&, const allocator_type&)): Add
>Â Â Â noexcept qualification.
>Â Â Â * testsuite/23_containers/unordered_map/allocator/default_init.cc:
>Â Â Â New.
>Â Â Â * testsuite/23_containers/unordered_map/cons/
>Â Â Â noexcept_default_construct.cc: New.
>Â Â Â * testsuite/23_containers/unordered_map/cons/
>Â Â Â noexcept_move_construct.cc: New.
>Â Â Â * testsuite/23_containers/unordered_map/modifiers/move_assign.cc:
>Â Â Â New.
>Â Â Â * testsuite/23_containers/unordered_multimap/cons/
>Â Â Â noexcept_default_construct.cc: New.
>Â Â Â * testsuite/23_containers/unordered_multimap/cons/
>Â Â Â noexcept_move_construct.cc: New.
>Â Â Â * testsuite/23_containers/unordered_multiset/cons/
>Â Â Â noexcept_default_construct.cc: New.
>Â Â Â * testsuite/23_containers/unordered_multiset/cons/
>Â Â Â noexcept_move_construct.cc: New.
>Â Â Â * testsuite/23_containers/unordered_set/allocator/default_init.cc: New.
>Â Â Â * testsuite/23_containers/unordered_set/cons/
>Â Â Â noexcept_default_construct.cc: New.
>Â Â Â *
>testsuite/23_containers/unordered_set/cons/noexcept_move_construct.cc:
>Â Â Â New.
>
>Tested under Linux x86_64.
>
>François
>
>diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
>index 5f785d4904d..ad07a36eb83 100644
>--- a/libstdc++-v3/include/bits/hashtable.h
>+++ b/libstdc++-v3/include/bits/hashtable.h
>@@ -463,6 +463,35 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> __hashtable_alloc(__node_alloc_type(__a))
> { }
>
>+ _Hashtable(_Hashtable&& __ht, __node_alloc_type&& __a, true_type)
>+ noexcept(std::is_nothrow_copy_constructible<_H1>::value &&
>+ std::is_nothrow_copy_constructible<_Equal>::value)
>+ : __hashtable_base(__ht),
>+ __map_base(__ht),
>+ __rehash_base(__ht),
>+ __hashtable_alloc(std::move(__a)),
>+ _M_buckets(__ht._M_buckets),
>+ _M_bucket_count(__ht._M_bucket_count),
>+ _M_before_begin(__ht._M_before_begin._M_nxt),
>+ _M_element_count(__ht._M_element_count),
>+ _M_rehash_policy(__ht._M_rehash_policy)
>+ {
>+ // Update, if necessary, buckets if __ht is using its single bucket.
I know the comment was already present, but I can't parse this.
There are two 'if' conditions, but don't they refer to the same thing?
It's necessary if __ht is using a single bucket, so shouldn't it be
either "Update buckets if __ht is using its single bucket" or "Update
buckets if necessary"?
"Update buckets if __ht is using its single bucket" seems preferable.
>+ if (__ht._M_uses_single_bucket())
>+ {
>+ _M_buckets = &_M_single_bucket;
>+ _M_single_bucket = __ht._M_single_bucket;
>+ }
>+
>+ // Fix bucket containing the _M_before_begin pointer that can't be
>+ // moved.
>+ _M_update_bbegin();
>+
>+ __ht._M_reset();
>+ }
>+
>+ _Hashtable(_Hashtable&&, __node_alloc_type&&, false_type);
>+
> template<typename _InputIterator>
> _Hashtable(_InputIterator __first, _InputIterator __last,
> size_type __bkt_count_hint,
>@@ -489,11 +518,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> _Hashtable(const _Hashtable&);
>
>- _Hashtable(_Hashtable&&) noexcept;
>+ _Hashtable(_Hashtable&& __ht)
>+ noexcept( noexcept(
>+ _Hashtable(std::declval<_Hashtable&&>(),
The && here is redundant, declval adds && to the type anyway.
>+ std::declval<__node_alloc_type&&>(), std::declval<true_type>())) )
Same for __node_alloc_type&& here.
And true_type{} is shorter and simpler than std::declval<true_type>().
We know is_nothrow_default_constructible<true_type> is true, so we can
just use that.
>+ : _Hashtable(std::move(__ht), std::move(__ht._M_node_allocator()),
>+ true_type{})
>+ { }
>
> _Hashtable(const _Hashtable&, const allocator_type&);
>
>- _Hashtable(_Hashtable&&, const allocator_type&);
>+ _Hashtable(_Hashtable&& __ht, const allocator_type& __a)
>+ noexcept( noexcept(
>+ _Hashtable(std::declval<_Hashtable&&>(),
>+ std::declval<__node_alloc_type&&>(),
Redundant &&s.
>+ std::declval<typename __node_alloc_traits::is_always_equal>())) )
Redundant declval: typename __node_alloc_traits::is_always_equal{}
>+ : _Hashtable(std::move(__ht), __node_alloc_type(__a),
>+ typename __node_alloc_traits::is_always_equal{})
>+ { }
>
> // Use delegating constructors.
> template<typename _InputIterator>
>@@ -1368,36 +1410,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> _M_assign(__ht, __alloc_node_gen);
> }
>
>- template<typename _Key, typename _Value,
>- typename _Alloc, typename _ExtractKey, typename _Equal,
>- typename _H1, typename _H2, typename _Hash, typename _RehashPolicy,
>- typename _Traits>
>- _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal,
>- _H1, _H2, _Hash, _RehashPolicy, _Traits>::
>- _Hashtable(_Hashtable&& __ht) noexcept
>- : __hashtable_base(__ht),
>- __map_base(__ht),
>- __rehash_base(__ht),
>- __hashtable_alloc(std::move(__ht._M_base_alloc())),
>- _M_buckets(__ht._M_buckets),
>- _M_bucket_count(__ht._M_bucket_count),
>- _M_before_begin(__ht._M_before_begin._M_nxt),
>- _M_element_count(__ht._M_element_count),
>- _M_rehash_policy(__ht._M_rehash_policy)
>- {
>- // Update, if necessary, buckets if __ht is using its single bucket.
>- if (__ht._M_uses_single_bucket())
>- {
>- _M_buckets = &_M_single_bucket;
>- _M_single_bucket = __ht._M_single_bucket;
>- }
>-
>- // Fix bucket containing the _M_before_begin pointer that can't be moved.
>- _M_update_bbegin();
>-
>- __ht._M_reset();
>- }
>-
> template<typename _Key, typename _Value,
> typename _Alloc, typename _ExtractKey, typename _Equal,
> typename _H1, typename _H2, typename _Hash, typename _RehashPolicy,
>@@ -1424,11 +1436,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> typename _Traits>
> _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal,
> _H1, _H2, _Hash, _RehashPolicy, _Traits>::
>- _Hashtable(_Hashtable&& __ht, const allocator_type& __a)
>+ _Hashtable(_Hashtable&& __ht, __node_alloc_type&& __a, false_type)
> : __hashtable_base(__ht),
> __map_base(__ht),
> __rehash_base(__ht),
>- __hashtable_alloc(__node_alloc_type(__a)),
>+ __hashtable_alloc(std::move(__a)),
> _M_buckets(nullptr),
> _M_bucket_count(__ht._M_bucket_count),
> _M_element_count(__ht._M_element_count),
>diff --git a/libstdc++-v3/include/bits/unordered_map.h b/libstdc++-v3/include/bits/unordered_map.h
>index 49c175a90be..5131e02e8aa 100644
>--- a/libstdc++-v3/include/bits/unordered_map.h
>+++ b/libstdc++-v3/include/bits/unordered_map.h
>@@ -209,6 +209,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> */
> unordered_map(unordered_map&& __umap,
> const allocator_type& __a)
>+ noexcept( noexcept(_Hashtable(declval<_Hashtable&&>(), __a)) )
declval should be qualified. It might be simpler to just say:
noexcept( noexcept(_Hashtable(std::move(__umap._M_h), __a)) )
> : _M_h(std::move(__umap._M_h), __a)
> { }
>
>@@ -1356,6 +1357,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> */
> unordered_multimap(unordered_multimap&& __ummap,
> const allocator_type& __a)
>+ noexcept( noexcept(_Hashtable(declval<_Hashtable&&>(), __a)) )
Same here.
> : _M_h(std::move(__ummap._M_h), __a)
> { }
>
>diff --git a/libstdc++-v3/include/bits/unordered_set.h b/libstdc++-v3/include/bits/unordered_set.h
>index 855579ee623..7154ec843db 100644
>--- a/libstdc++-v3/include/bits/unordered_set.h
>+++ b/libstdc++-v3/include/bits/unordered_set.h
>@@ -203,6 +203,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> */
> unordered_set(unordered_set&& __uset,
> const allocator_type& __a)
>+ noexcept( noexcept(_Hashtable(declval<_Hashtable&&>(), __a)) )
Same here.
> : _M_h(std::move(__uset._M_h), __a)
> { }
>
>@@ -1044,6 +1045,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> */
> unordered_multiset(unordered_multiset&& __umset,
> const allocator_type& __a)
>+ noexcept( noexcept(_Hashtable(declval<_Hashtable&&>(), __a)) )
Same here.
> : _M_h(std::move(__umset._M_h), __a)
> { }
>
>diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/default_init.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/default_init.cc
>new file mode 100644
>index 00000000000..8c3103c2614
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/default_init.cc
>@@ -0,0 +1,69 @@
>+// Copyright (C) 2019 Free Software Foundation, Inc.
Please remember to update the dates in the new tests.
OK for master with those adjustments. Thanks.
More information about the Gcc-patches
mailing list