std::vector<bool> code cleanup fixes optimizations

std::vector<bool> code cleanup fixes optimizations

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 2888 bytes --]

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 ?

François


[-- Attachment #2: bvector.patch --]
[-- Type: text/x-patch, Size: 12077 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index 280d40f60c5..d6fcc2d848d 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -441,7 +441,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;
 
@@ -450,33 +459,38 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	{ }
 
 #if __cplusplus >= 201103L
+	_Bvector_impl_data(const _Bvector_impl_data&) = default;
+	_Bvector_impl_data&
+	operator=(const _Bvector_impl_data&) = default;
+
 	_Bvector_impl_data(_Bvector_impl_data&& __x) noexcept
-	: _M_start(__x._M_start), _M_finish(__x._M_finish)
-	, _M_end_of_storage(__x._M_end_of_storage)
+	: _Bvector_impl_data(__x)
 	{ __x._M_reset(); }
 
 	void
 	_M_move_data(_Bvector_impl_data&& __x) noexcept
 	{
-	  this->_M_start = __x._M_start;
-	  this->_M_finish = __x._M_finish;
-	  this->_M_end_of_storage = __x._M_end_of_storage;
+	  *this = __x;
 	  __x._M_reset();
 	}
 #endif
 
 	void
 	_M_reset() _GLIBCXX_NOEXCEPT
+	{ *this = _Bvector_impl_data(); }
+
+	void
+	_M_swap_data(_Bvector_impl_data& __x) _GLIBCXX_NOEXCEPT
 	{
-	  _M_start = _M_finish = _Bit_iterator();
-	  _M_end_of_storage = _Bit_pointer();
+	  // Do not use std::swap(_M_start, __x._M_start), etc as it loses
+	  // information used by TBAA.
+	  std::swap(*this, __x);
 	}
       };
 
       struct _Bvector_impl
 	: public _Bit_alloc_type, public _Bvector_impl_data
       {
-	public:
 	_Bvector_impl() _GLIBCXX_NOEXCEPT_IF(
 	  is_nothrow_default_constructible<_Bit_alloc_type>::value)
 	: _Bit_alloc_type()
@@ -487,7 +501,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	{ }
 
 #if __cplusplus >= 201103L
-	_Bvector_impl(_Bvector_impl&&) = default;
+	// Not defaulted, to enforce noexcept(true) even when
+	// !is_nothrow_move_constructible<_Bit_alloc_type>.
+	_Bvector_impl(_Bvector_impl&& __x) noexcept
+	: _Bit_alloc_type(std::move(__x)), _Bvector_impl_data(std::move(__x))
+	{ }
+
+	_Bvector_impl(_Bit_alloc_type&& __a, _Bvector_impl&& __x) noexcept
+	: _Bit_alloc_type(std::move(__a)), _Bvector_impl_data(std::move(__x))
+	{ }
 #endif
 
 	_Bit_type*
@@ -525,6 +547,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
 #if __cplusplus >= 201103L
       _Bvector_base(_Bvector_base&&) = default;
+
+      _Bvector_base(_Bvector_base&& __x, const allocator_type& __a)
+      : _M_impl(_Bit_alloc_type(__a), std::move(__x._M_impl))
+      { }
 #endif
 
       ~_Bvector_base()
@@ -661,14 +687,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       : _Base(_Bit_alloc_traits::_S_select_on_copy(__x._M_get_Bit_allocator()))
       {
 	_M_initialize(__x.size());
-	_M_copy_aligned(__x.begin(), __x.end(), this->_M_impl._M_start);
+	_M_copy_aligned(__x.begin(), __x.end(), begin());
       }
 
 #if __cplusplus >= 201103L
       vector(vector&&) = default;
 
-      vector(vector&& __x, const allocator_type& __a)
-      noexcept(_Bit_alloc_traits::_S_always_equal())
+    private:
+      vector(vector&& __x, const allocator_type& __a, true_type) noexcept
+      : _Base(std::move(__x), __a)
+      { }
+
+      vector(vector&& __x, const allocator_type& __a, false_type)
       : _Base(__a)
       {
 	if (__x.get_allocator() == __a)
@@ -681,11 +711,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  }
       }
 
+    public:
+      vector(vector&& __x, const allocator_type& __a)
+      noexcept(_Bit_alloc_traits::_S_always_equal())
+      : vector(std::move(__x), __a,
+	       typename _Bit_alloc_traits::is_always_equal{})
+      { }
+
       vector(const vector& __x, const allocator_type& __a)
       : _Base(__a)
       {
 	_M_initialize(__x.size());
-	_M_copy_aligned(__x.begin(), __x.end(), this->_M_impl._M_start);
+	_M_copy_aligned(__x.begin(), __x.end(), begin());
       }
 
       vector(initializer_list<bool> __l,
@@ -703,13 +740,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	vector(_InputIterator __first, _InputIterator __last,
 	       const allocator_type& __a = allocator_type())
 	: _Base(__a)
-	{ _M_initialize_dispatch(__first, __last, __false_type()); }
+	{
+	  _M_initialize_range(__first, __last,
+			      std::__iterator_category(__first));
+	}
 #else
       template<typename _InputIterator>
 	vector(_InputIterator __first, _InputIterator __last,
 	       const allocator_type& __a = allocator_type())
 	: _Base(__a)
 	{
+	  // Check whether it's an integral type. If so, it's not an iterator.
 	  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
 	  _M_initialize_dispatch(__first, __last, _Integral());
 	}
@@ -800,6 +841,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	void
 	assign(_InputIterator __first, _InputIterator __last)
 	{
+	  // Check whether it's an integral type. If so, it's not an iterator.
 	  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
 	  _M_assign_dispatch(__first, __last, _Integral());
 	}
@@ -888,17 +930,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       reference
       operator[](size_type __n)
-      {
-	return *iterator(this->_M_impl._M_start._M_p
-			 + __n / int(_S_word_bit), __n % int(_S_word_bit));
-      }
+      { return begin()[__n]; }
 
       const_reference
       operator[](size_type __n) const
-      {
-	return *const_iterator(this->_M_impl._M_start._M_p
-			     + __n / int(_S_word_bit), __n % int(_S_word_bit));
-      }
+      { return begin()[__n]; }
 
     protected:
       void
@@ -965,10 +1001,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       void
       swap(vector& __x) _GLIBCXX_NOEXCEPT
       {
-	std::swap(this->_M_impl._M_start, __x._M_impl._M_start);
-	std::swap(this->_M_impl._M_finish, __x._M_impl._M_finish);
-	std::swap(this->_M_impl._M_end_of_storage,
-		  __x._M_impl._M_end_of_storage);
+#if __cplusplus >= 201103L
+	__glibcxx_assert(_Bit_alloc_traits::propagate_on_container_swap::value
+			 || _M_get_Bit_allocator() == __x._M_get_Bit_allocator());
+#endif
+	this->_M_impl._M_swap_data(__x._M_impl);
 	_Bit_alloc_traits::_S_on_swap(_M_get_Bit_allocator(),
 				      __x._M_get_Bit_allocator());
       }
@@ -1006,8 +1043,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	       _InputIterator __first, _InputIterator __last)
 	{
 	  difference_type __offset = __position - cbegin();
-	  _M_insert_dispatch(__position._M_const_cast(),
-			     __first, __last, __false_type());
+	  _M_insert_range(__position._M_const_cast(),
+			  __first, __last,
+			  std::__iterator_category(__first));
 	  return begin() + __offset;
 	}
 #else
@@ -1016,6 +1054,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	insert(iterator __position,
 	       _InputIterator __first, _InputIterator __last)
 	{
+	  // Check whether it's an integral type. If so, it's not an iterator.
 	  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
 	  _M_insert_dispatch(__position, __first, __last, _Integral());
 	}
@@ -1127,15 +1166,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  {
 	    _Bit_pointer __q = this->_M_allocate(__n);
 	    this->_M_impl._M_end_of_storage = __q + _S_nword(__n);
-	    this->_M_impl._M_start = iterator(std::__addressof(*__q), 0);
+	    iterator __start = iterator(std::__addressof(*__q), 0);
+	    this->_M_impl._M_start = __start;
+	    this->_M_impl._M_finish = __start + difference_type(__n);
 	  }
-	else
-	  {
-	    this->_M_impl._M_end_of_storage = _Bit_pointer();
-	    this->_M_impl._M_start = iterator(0, 0);
-	  }
-	this->_M_impl._M_finish = this->_M_impl._M_start + difference_type(__n);
-
       }
 
       void
@@ -1155,8 +1189,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       _M_shrink_to_fit();
 #endif
 
-      // Check whether it's an integral type.  If so, it's not an iterator.
-
+#if __cplusplus < 201103L
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // 438. Ambiguity in the "do the right thing" clause
       template<typename _Integer>
@@ -1173,6 +1206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 			       __false_type)
 	{ _M_initialize_range(__first, __last,
 			      std::__iterator_category(__first)); }
+#endif
 
       template<typename _InputIterator>
 	void
@@ -1190,7 +1224,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	{
 	  const size_type __n = std::distance(__first, __last);
 	  _M_initialize(__n);
-	  std::copy(__first, __last, this->_M_impl._M_start);
+	  std::copy(__first, __last, begin());
 	}
 
 #if __cplusplus < 201103L
@@ -1254,8 +1288,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	    }
 	}
 
-      // Check whether it's an integral type.  If so, it's not an iterator.
-
+#if __cplusplus < 201103L
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // 438. Ambiguity in the "do the right thing" clause
       template<typename _Integer>
@@ -1271,6 +1304,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 			   __false_type)
 	{ _M_insert_range(__pos, __first, __last,
 			  std::__iterator_category(__first)); }
+#endif
 
       void
       _M_fill_insert(iterator __position, size_type __n, bool __x);
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; }
 }
 
 using __gnu_test::propagating_allocator;
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
index abc4114e31d..90c863d8a35 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
@@ -23,4 +23,34 @@
 
 typedef std::vector<bool> vbtype;
 
-static_assert(std::is_nothrow_move_constructible<vbtype>::value, "Error");
+static_assert( std::is_nothrow_move_constructible<vbtype>::value,
+	       "noexcept move constructor" );
+static_assert( std::is_nothrow_constructible<vbtype,
+	       vbtype&&, const typename vbtype::allocator_type&>::value,
+	       "noexcept move constructor with allocator" );
+
+template<typename Type>
+  class not_noexcept_move_constructor_alloc : public std::allocator<Type>
+  {
+  public:
+    not_noexcept_move_constructor_alloc() noexcept { }
+
+    not_noexcept_move_constructor_alloc(
+	const not_noexcept_move_constructor_alloc& x) noexcept
+    : std::allocator<Type>(x)
+    { }
+
+    not_noexcept_move_constructor_alloc(
+	not_noexcept_move_constructor_alloc&& x) noexcept(false)
+    : std::allocator<Type>(std::move(x))
+    { }
+
+    template<typename _Tp1>
+      struct rebind
+      { typedef not_noexcept_move_constructor_alloc<_Tp1> other; };
+  };
+
+typedef std::vector<bool, not_noexcept_move_constructor_alloc<bool>> vbtype2;
+
+static_assert( std::is_nothrow_move_constructible<vbtype2>::value,
+	       "noexcept move constructor with not noexcept alloc" );

Re: std::vector<bool> code cleanup fixes optimizations

[François Dumont]

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 ?
>
> François
>

Re: std::vector<bool> code cleanup fixes optimizations

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 5746 bytes --]

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 ?
>>
>> François
>>
>


[-- Attachment #2: bvector.patch --]
[-- Type: text/x-patch, Size: 13461 bytes --]

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;
 
@@ -458,33 +467,38 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	{ }
 
 #if __cplusplus >= 201103L
+	_Bvector_impl_data(const _Bvector_impl_data&) = default;
+	_Bvector_impl_data&
+	operator=(const _Bvector_impl_data&) = default;
+
 	_Bvector_impl_data(_Bvector_impl_data&& __x) noexcept
-	: _M_start(__x._M_start), _M_finish(__x._M_finish)
-	, _M_end_of_storage(__x._M_end_of_storage)
+	: _Bvector_impl_data(__x)
 	{ __x._M_reset(); }
 
 	void
 	_M_move_data(_Bvector_impl_data&& __x) noexcept
 	{
-	  this->_M_start = __x._M_start;
-	  this->_M_finish = __x._M_finish;
-	  this->_M_end_of_storage = __x._M_end_of_storage;
+	  *this = __x;
 	  __x._M_reset();
 	}
 #endif
 
 	void
 	_M_reset() _GLIBCXX_NOEXCEPT
+	{ *this = _Bvector_impl_data(); }
+
+	void
+	_M_swap_data(_Bvector_impl_data& __x) _GLIBCXX_NOEXCEPT
 	{
-	  _M_start = _M_finish = _Bit_iterator();
-	  _M_end_of_storage = _Bit_pointer();
+	  // Do not use std::swap(_M_start, __x._M_start), etc as it loses
+	  // information used by TBAA.
+	  std::swap(*this, __x);
 	}
       };
 
       struct _Bvector_impl
 	: public _Bit_alloc_type, public _Bvector_impl_data
       {
-	public:
 	_Bvector_impl() _GLIBCXX_NOEXCEPT_IF(
 	  is_nothrow_default_constructible<_Bit_alloc_type>::value)
 	: _Bit_alloc_type()
@@ -495,7 +509,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	{ }
 
 #if __cplusplus >= 201103L
-	_Bvector_impl(_Bvector_impl&&) = default;
+	// Not defaulted, to enforce noexcept(true) even when
+	// !is_nothrow_move_constructible<_Bit_alloc_type>.
+	_Bvector_impl(_Bvector_impl&& __x) noexcept
+	: _Bit_alloc_type(std::move(__x)), _Bvector_impl_data(std::move(__x))
+	{ }
+
+	_Bvector_impl(_Bit_alloc_type&& __a, _Bvector_impl&& __x) noexcept
+	: _Bit_alloc_type(std::move(__a)), _Bvector_impl_data(std::move(__x))
+	{ }
 #endif
 
 	_Bit_type*
@@ -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)
+      : _M_impl(_Bit_alloc_type(__a), std::move(__x._M_impl))
+      { }
 #endif
 
       ~_Bvector_base()
@@ -669,14 +695,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       : _Base(_Bit_alloc_traits::_S_select_on_copy(__x._M_get_Bit_allocator()))
       {
 	_M_initialize(__x.size());
-	_M_copy_aligned(__x.begin(), __x.end(), this->_M_impl._M_start);
+	_M_copy_aligned(__x.begin(), __x.end(), begin());
       }
 
 #if __cplusplus >= 201103L
       vector(vector&&) = default;
 
-      vector(vector&& __x, const allocator_type& __a)
-      noexcept(_Bit_alloc_traits::_S_always_equal())
+    private:
+      vector(vector&& __x, const allocator_type& __a, true_type) noexcept
+      : _Base(std::move(__x), __a)
+      { }
+
+      vector(vector&& __x, const allocator_type& __a, false_type)
       : _Base(__a)
       {
 	if (__x.get_allocator() == __a)
@@ -689,11 +719,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  }
       }
 
+    public:
+      vector(vector&& __x, const allocator_type& __a)
+      noexcept(_Bit_alloc_traits::_S_always_equal())
+      : vector(std::move(__x), __a,
+	       typename _Bit_alloc_traits::is_always_equal{})
+      { }
+
       vector(const vector& __x, const allocator_type& __a)
       : _Base(__a)
       {
 	_M_initialize(__x.size());
-	_M_copy_aligned(__x.begin(), __x.end(), this->_M_impl._M_start);
+	_M_copy_aligned(__x.begin(), __x.end(), begin());
       }
 
       vector(initializer_list<bool> __l,
@@ -711,13 +748,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	vector(_InputIterator __first, _InputIterator __last,
 	       const allocator_type& __a = allocator_type())
 	: _Base(__a)
-	{ _M_initialize_dispatch(__first, __last, __false_type()); }
+	{
+	  _M_initialize_range(__first, __last,
+			      std::__iterator_category(__first));
+	}
 #else
       template<typename _InputIterator>
 	vector(_InputIterator __first, _InputIterator __last,
 	       const allocator_type& __a = allocator_type())
 	: _Base(__a)
 	{
+	  // Check whether it's an integral type. If so, it's not an iterator.
 	  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
 	  _M_initialize_dispatch(__first, __last, _Integral());
 	}
@@ -808,6 +849,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	void
 	assign(_InputIterator __first, _InputIterator __last)
 	{
+	  // Check whether it's an integral type. If so, it's not an iterator.
 	  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
 	  _M_assign_dispatch(__first, __last, _Integral());
 	}
@@ -896,17 +938,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       reference
       operator[](size_type __n)
-      {
-	return *iterator(this->_M_impl._M_start._M_p
-			 + __n / int(_S_word_bit), __n % int(_S_word_bit));
-      }
+      { return begin()[__n]; }
 
       const_reference
       operator[](size_type __n) const
-      {
-	return *const_iterator(this->_M_impl._M_start._M_p
-			     + __n / int(_S_word_bit), __n % int(_S_word_bit));
-      }
+      { return begin()[__n]; }
 
     protected:
       void
@@ -973,10 +1009,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       void
       swap(vector& __x) _GLIBCXX_NOEXCEPT
       {
-	std::swap(this->_M_impl._M_start, __x._M_impl._M_start);
-	std::swap(this->_M_impl._M_finish, __x._M_impl._M_finish);
-	std::swap(this->_M_impl._M_end_of_storage,
-		  __x._M_impl._M_end_of_storage);
+#if __cplusplus >= 201103L
+	__glibcxx_assert(_Bit_alloc_traits::propagate_on_container_swap::value
+			 || _M_get_Bit_allocator() == __x._M_get_Bit_allocator());
+#endif
+	this->_M_impl._M_swap_data(__x._M_impl);
 	_Bit_alloc_traits::_S_on_swap(_M_get_Bit_allocator(),
 				      __x._M_get_Bit_allocator());
       }
@@ -1014,8 +1051,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	       _InputIterator __first, _InputIterator __last)
 	{
 	  difference_type __offset = __position - cbegin();
-	  _M_insert_dispatch(__position._M_const_cast(),
-			     __first, __last, __false_type());
+	  _M_insert_range(__position._M_const_cast(),
+			  __first, __last,
+			  std::__iterator_category(__first));
 	  return begin() + __offset;
 	}
 #else
@@ -1024,6 +1062,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	insert(iterator __position,
 	       _InputIterator __first, _InputIterator __last)
 	{
+	  // Check whether it's an integral type. If so, it's not an iterator.
 	  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
 	  _M_insert_dispatch(__position, __first, __last, _Integral());
 	}
@@ -1135,15 +1174,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  {
 	    _Bit_pointer __q = this->_M_allocate(__n);
 	    this->_M_impl._M_end_of_storage = __q + _S_nword(__n);
-	    this->_M_impl._M_start = iterator(std::__addressof(*__q), 0);
+	    iterator __start = iterator(std::__addressof(*__q), 0);
+	    this->_M_impl._M_start = __start;
+	    this->_M_impl._M_finish = __start + difference_type(__n);
 	  }
-	else
-	  {
-	    this->_M_impl._M_end_of_storage = _Bit_pointer();
-	    this->_M_impl._M_start = iterator(0, 0);
-	  }
-	this->_M_impl._M_finish = this->_M_impl._M_start + difference_type(__n);
-
       }
 
       void
@@ -1163,8 +1197,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       _M_shrink_to_fit();
 #endif
 
-      // Check whether it's an integral type.  If so, it's not an iterator.
-
+#if __cplusplus < 201103L
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // 438. Ambiguity in the "do the right thing" clause
       template<typename _Integer>
@@ -1181,6 +1214,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 			       __false_type)
 	{ _M_initialize_range(__first, __last,
 			      std::__iterator_category(__first)); }
+#endif
 
       template<typename _InputIterator>
 	void
@@ -1198,7 +1232,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	{
 	  const size_type __n = std::distance(__first, __last);
 	  _M_initialize(__n);
-	  std::copy(__first, __last, this->_M_impl._M_start);
+	  std::copy(__first, __last, begin());
 	}
 
 #if __cplusplus < 201103L
@@ -1262,8 +1296,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	    }
 	}
 
-      // Check whether it's an integral type.  If so, it's not an iterator.
-
+#if __cplusplus < 201103L
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // 438. Ambiguity in the "do the right thing" clause
       template<typename _Integer>
@@ -1279,6 +1312,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 			   __false_type)
 	{ _M_insert_range(__pos, __first, __last,
 			  std::__iterator_category(__first)); }
+#endif
 
       void
       _M_fill_insert(iterator __position, size_type __n, bool __x);
diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 08327516b28..251e3ca098d 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -406,7 +406,7 @@ class StdVectorPrinter:
             self.bitvec = bitvec
             if bitvec:
                 self.item   = start['_M_p']
-                self.so     = start['_M_offset']
+                self.so     = 0
                 self.finish = finish['_M_p']
                 self.fo     = finish['_M_offset']
                 itype = self.item.dereference().type
@@ -454,12 +454,11 @@ class StdVectorPrinter:
         end = self.val['_M_impl']['_M_end_of_storage']
         if self.is_bool:
             start = self.val['_M_impl']['_M_start']['_M_p']
-            so    = self.val['_M_impl']['_M_start']['_M_offset']
             finish = self.val['_M_impl']['_M_finish']['_M_p']
             fo     = self.val['_M_impl']['_M_finish']['_M_offset']
             itype = start.dereference().type
             bl = 8 * itype.sizeof
-            length   = (bl - so) + bl * ((finish - start) - 1) + fo
+            length   = bl * (finish - start) + fo
             capacity = bl * (end - start)
             return ('%s<bool> of length %d, capacity %d'
                     % (self.typename, int (length), int (capacity)))
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; }
 }
 
 using __gnu_test::propagating_allocator;
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
index abc4114e31d..90c863d8a35 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
@@ -23,4 +23,34 @@
 
 typedef std::vector<bool> vbtype;
 
-static_assert(std::is_nothrow_move_constructible<vbtype>::value, "Error");
+static_assert( std::is_nothrow_move_constructible<vbtype>::value,
+	       "noexcept move constructor" );
+static_assert( std::is_nothrow_constructible<vbtype,
+	       vbtype&&, const typename vbtype::allocator_type&>::value,
+	       "noexcept move constructor with allocator" );
+
+template<typename Type>
+  class not_noexcept_move_constructor_alloc : public std::allocator<Type>
+  {
+  public:
+    not_noexcept_move_constructor_alloc() noexcept { }
+
+    not_noexcept_move_constructor_alloc(
+	const not_noexcept_move_constructor_alloc& x) noexcept
+    : std::allocator<Type>(x)
+    { }
+
+    not_noexcept_move_constructor_alloc(
+	not_noexcept_move_constructor_alloc&& x) noexcept(false)
+    : std::allocator<Type>(std::move(x))
+    { }
+
+    template<typename _Tp1>
+      struct rebind
+      { typedef not_noexcept_move_constructor_alloc<_Tp1> other; };
+  };
+
+typedef std::vector<bool, not_noexcept_move_constructor_alloc<bool>> vbtype2;
+
+static_assert( std::is_nothrow_move_constructible<vbtype2>::value,
+	       "noexcept move constructor with not noexcept alloc" );

Re: std::vector<bool> code cleanup fixes optimizations

[Jonathan Wakely]

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.

>+      : _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?

Re: std::vector<bool> code cleanup fixes optimizations

[François Dumont]

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

Re: std::vector<bool> code cleanup fixes optimizations

[Jonathan Wakely]

On 31/07/20 23:03 +0200, François Dumont via Libstdc++ wrote:
>On 17/07/20 12:36 pm, Jonathan Wakely wrote:
>>On 16/12/19 08:18 +0100, François Dumont wrote:
>>>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>.

Ah yes, of course. Thanks.


>Building with __glibcxx_assert was showing this.
>
>François
>

Re: std::vector<bool> code cleanup fixes optimizations

[François Dumont]

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
>

Re: std::vector<bool> code cleanup fixes optimizations

[Jonathan Wakely]

On 06/08/20 21:30 +0200, François Dumont via Libstdc++ wrote:
>I wonder if following the application of this patch we shouldn't bump 
>versioned namespace through _GLIBCXX_BEGIN_NAMESPACE_VERSION ?

Definitely not.

>Unless it is still considered as experimental.

Yes, it is. And it doesn't claim to provide any stability or backwards
compatibility.

A small change like this doesn't justify changing it.

We changed it last time because I was hoping to change the std::string
ABI used by the versioned namespace, but that never happened.