I wonder if following the application of this patch we shouldn't bump
versioned namespace through _GLIBCXX_BEGIN_NAMESPACE_VERSION ?
Unless it is still considered as experimental.
François
On 31/07/20 11:03 pm, François Dumont wrote:
On 17/07/20 12:36 pm, Jonathan Wakely wrote:
On 16/12/19 08:18 +0100, François Dumont wrote:
A small refresh on this patch now tested also for versioned
namespace which require printers.py to be updated.
Note that this simplification works also for normal mode so I can
apply it independently from the stl_bvector.h part.
   * include/bits/stl_bvector.h
   [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start):
Define as
   _Bit_type*.
   (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
   (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
   (_Bvector_impl_data::operator=(const _Bvector_impl_data&)):
Default.
(_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
   (_Bvector_impl_data::_M_reset()): Likewise.
   (_Bvector_impl_data::_M_swap_data): New.
   (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
explicitely.
   (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
_Bvector_impl&&)): New.
   (_Bvector_base::_Bvector_base(_Bvector_base&&, const
allocator_type&)):
   New, use latter.
   (vector::vector(vector&&, const allocator_type&, true_type)):
New, use
   latter.
   (vector::vector(vector&&, const allocator_type&,
false_type)): New.
   (vector::vector(vector&&, const allocator_type&)): Use latters.
   (vector::vector(const vector&, const allocator_type&)): Adapt.
   [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
   const allocator_type&)): Use _M_initialize_range.
   (vector::operator[](size_type)): Use iterator operator[].
   (vector::operator[](size_type) const): Use const_iterator
operator[].
   (vector::swap(vector&)): Add assertions on allocators. Use
_M_swap_data.
   [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
   _InputIt)): Use _M_insert_range.
   (vector::_M_initialize(size_type)): Adapt.
   [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
   [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
   * python/libstdcxx/v6/printers.py
(StdVectorPrinter._iterator): Stop
   using start _M_offset.
   (StdVectorPrinter.to_string): Likewise.
   * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
   *
testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
   Add check.
François
On 6/24/19 9:31 PM, François Dumont wrote:
Hi
   Any feedback regarding this patch ?
Thanks
On 5/14/19 7:46 AM, François Dumont wrote:
Hi
   This is the patch on vector<bool> to:
- Optimize sizeof in Versioned namespace mode. We could go one
step further by removing _M_p from _M_finish and just transform it
into an offset but it is a little bit more impacting for the code.
- Implement the swap optimization already done on main std::vector
template class.
- Fix move constructor so that it is noexcept no matter allocator
move constructor noexcept qualification
- Optimize move constructor with allocator when allocator type is
always equal.
- Use shortcuts in C++11 by skipping the _M_XXX_dispatch methods.
Those are now defined only in pre-C++11 mode, I can't see any abi
issue in doing so.
   * include/bits/stl_bvector.h
   [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start):
Define as
   _Bit_type*.
   (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
   (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to
latter.
   (_Bvector_impl_data::operator=(const _Bvector_impl_data&)):
Default.
(_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
   (_Bvector_impl_data::_M_reset()): Likewise.
   (_Bvector_impl_data::_M_swap_data): New.
   (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
explicitely.
   (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
_Bvector_impl&&)): New.
   (_Bvector_base::_Bvector_base(_Bvector_base&&, const
allocator_type&)):
   New, use latter.
   (vector::vector(vector&&, const allocator_type&,
true_type)): New, use
   latter.
   (vector::vector(vector&&, const allocator_type&,
false_type)): New.
   (vector::vector(vector&&, const allocator_type&)): Use
latters.
   (vector::vector(const vector&, const allocator_type&)): Adapt.
   [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
   const allocator_type&)): Use _M_initialize_range.
   (vector::operator[](size_type)): Use iterator operator[].
   (vector::operator[](size_type) const): Use const_iterator
operator[].
   (vector::swap(vector&)): Adapt.
   (vector::_M_initialize(size_type)): Add assertions on
allocators.
   Use _M_swap_data.
   [__cplusplus >= 201103](vector::insert(const_iterator,
_InputIt,
   _InputIt)): Use _M_insert_range.
   [__cplusplus >= 201103](vector::_M_initialize_dispatch):
Remove.
   [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
   * testsuite/23_containers/vector/bool/allocator/swap.cc:
Adapt.
   *
testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
   Add check.
Tested under Linux x86_64, normal and debug modes.
Ok to commit ?
Yes, OK for master, but I have one question below (and one minor
comment).
diff --git a/libstdc++-v3/include/bits/stl_bvector.h
b/libstdc++-v3/include/bits/stl_bvector.h
index f2eea7799dc..15ecce62683 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -449,7 +449,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
struct _Bvector_impl_data
{
+#if !_GLIBCXX_INLINE_VERSION
_Bit_iterator _M_start;
+#else
+ // We don't need the offset field for the start, it's always zero.
+ struct {
+ _Bit_type* _M_p;
+ // Allow assignment from iterators (assume offset is zero):
+ void operator=(_Bit_iterator __it) { _M_p = __it._M_p; }
+ } _M_start;
+#endif
_Bit_iterator _M_finish;
_Bit_pointer _M_end_of_storage;
@@ -533,6 +555,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
#if __cplusplus >= 201103L
_Bvector_base(_Bvector_base&&) = default;
+
+ _Bvector_base(_Bvector_base&& __x, const allocator_type& __a)
I think you can add 'noexcept' here.
Constructing the _Bit_alloc_type can't throw, and the base constructor
is noexcept.
Ok, added.
+ : _M_impl(_Bit_alloc_type(__a), std::move(__x._M_impl))
+ { }
#endif
~_Bvector_base()
diff --git
a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
index de441426532..745fdc85cf6 100644
---
a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
+++
b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
@@ -28,19 +28,17 @@ namespace __gnu_test
// It is undefined behaviour to swap() containers with unequal
allocators
// if the allocator doesn't propagate, so ensure the allocators
compare
// equal, while still being able to test propagation via
get_personality().
+ template<typename Type>
bool
- operator==(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
- {
- return true;
- }
+ operator==(const propagating_allocator<Type, false>&,
+ const propagating_allocator<Type, false>&)
+ { return true; }
+ template<typename Type>
bool
- operator!=(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
- {
- return false;
- }
+ operator!=(const propagating_allocator<Type, false>&,
+ const propagating_allocator<Type, false>&)
+ { return false; }
Why does this test need to be adapted?
Because I replicate and adapt the static assertion from the main
vector implementation which is:
#if __cplusplus >= 201103L
__glibcxx_assert(_Bit_alloc_traits::propagate_on_container_swap::value
|| _M_get_Bit_allocator() == __x._M_get_Bit_allocator());
#endif
in the case of vector<bool> the compared allocators are not
propagating_allocator<bool, false> but propagating_allocator<unsigned
long, false>.
Building with __glibcxx_assert was showing this.
François
