std::vector<bool> fix & enhancements

std::vector<bool> fix & enhancements

[François Dumont]

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

Following Marc Glisse change to ignore _M_start offset I wanted to go a 
little step further and just remove it in _GLIBCXX_INLINE_VERSION mode.

I also fix a regression we already fixed on mainstream std::vector 
regarding noexcept qualification of move constructor with allocator.

And I implemented the same optimizations than in std::vector for 
allocators always comparing equals and for the std::swap operation.

I also avoid re-implementing in vector::operator[] the same code already 
implemented in iterator::operator[] but this one should perhaps go in a 
different commit.


     * include/bits/stl_bvector.h
     [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
     _Bit_type*.
     (_Bvector_impl_data(const _Bvector_impl_data&)): New.
     (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
     (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): New.
(_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
     (_Bvector_impl_data::_M_reset()): Likewise.
     (_Bvector_impl_data::_M_begin()): New.
     (_Bvector_impl_data::_M_cbegin()): New.
     (_Bvector_impl_data::_M_start_p()): New.
     (_Bvector_impl_data::_M_set_start(_Bit_type*)): New.
     (_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.
     (_Bvector_base::_M_deallocate()): Adapt.
     (vector::vector(const vector&, const allocator_type&)): Adapt.
     (vector::vector(vector&&, const allocator_type&, true_type)): New.
     (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.
     (vector::begin()): Adapt.
     (vector::cbegin()): Adapt.
     (vector::operator[](size_type)): Use iterator operator[].
     (vector::swap(vector&)): Adapt.
     (vector::flip()): Adapt.
     (vector::_M_initialize(size_type)): Adapt.
     (vector::_M_initialize_value(bool)): Adapt.
     * include/bits/vector.tcc:
     (vector<bool>::_M_reallocate(size_type)): Adapt.
     (vector<bool>::_M_fill_insert(iterator, size_type, bool)): Adapt.
     (vector<bool>::_M_insert_range<_FwdIter>(iterator, _FwdIter, _FwdIter
     std::forward_iterator_tag)): Adapt.
     (vector<bool>::_M_insert_aux(iterator, bool)): Adapt.
     (std::hash<std::vector<bool>>::operator()): Adapt.
     * testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
     Add check.

Tested under Linux x86_64.

Ok to commit ?

François


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

diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index 8fbef7a1a3a..81b4a75236d 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -437,7 +437,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       struct _Bvector_impl_data
       {
+#if !_GLIBCXX_INLINE_VERSION
 	_Bit_iterator	_M_start;
+#else
+	_Bit_type*	_M_start;
+#endif
 	_Bit_iterator	_M_finish;
 	_Bit_pointer	_M_end_of_storage;
 
@@ -447,32 +451,74 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
 #if __cplusplus >= 201103L
 	_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(); }
 
+	_Bvector_impl_data(const _Bvector_impl_data&) = default;
+	_Bvector_impl_data&
+	operator=(const _Bvector_impl_data&) = default;
+
 	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();
 	}
+#else
+	_Bvector_impl_data(const _Bvector_impl_data& __x)
+	  : _M_start(__x._M_start), _M_finish(__x._M_finish)
+	  , _M_end_of_storage(__x._M_end_of_storage)
+	{ }
+
+	_Bvector_impl_data&
+	operator=(const _Bvector_impl_data& __x)
+	{
+	  _M_start = __x._M_start;
+	  _M_finish = __x._M_finish;
+	  _M_end_of_storage = __x._M_end_of_storage;
+	}
+#endif
+
+	_Bit_iterator
+	_M_begin() const _GLIBCXX_NOEXCEPT
+	{ return _Bit_iterator(_M_start_p(), 0); }
+
+	_Bit_const_iterator
+	_M_cbegin() const _GLIBCXX_NOEXCEPT
+	{ return _Bit_const_iterator(_M_start_p(), 0); }
+
+	_Bit_type*
+	_M_start_p() const _GLIBCXX_NOEXCEPT
+#if !_GLIBCXX_INLINE_VERSION
+	{ return _M_start._M_p; }
+#else
+	{ return _M_start; }
+#endif
+
+	void
+	_M_set_start(_Bit_type* __p) _GLIBCXX_NOEXCEPT
+#if !_GLIBCXX_INLINE_VERSION
+	{ _M_start._M_p = __p; }
+#else
+	{ _M_start = __p; }
 #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( noexcept(_Bit_alloc_type()) )
 	: _Bit_alloc_type()
@@ -483,7 +529,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*
@@ -521,6 +575,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()
@@ -536,9 +594,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       void
       _M_deallocate()
       {
-	if (_M_impl._M_start._M_p)
+	if (_M_impl._M_start_p())
 	  {
-	    const size_t __n = _M_impl._M_end_addr() - _M_impl._M_start._M_p;
+	    const size_t __n = _M_impl._M_end_addr() - _M_impl._M_start_p();
 	    _Bit_alloc_traits::deallocate(_M_impl,
 					  _M_impl._M_end_of_storage - __n,
 					  __n);
@@ -657,14 +715,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)
@@ -677,11 +739,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,
@@ -809,11 +878,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       iterator
       begin() _GLIBCXX_NOEXCEPT
-      { return iterator(this->_M_impl._M_start._M_p, 0); }
+      { return this->_M_impl._M_begin(); }
 
       const_iterator
       begin() const _GLIBCXX_NOEXCEPT
-      { return const_iterator(this->_M_impl._M_start._M_p, 0); }
+      { return this->_M_impl._M_cbegin(); }
 
       iterator
       end() _GLIBCXX_NOEXCEPT
@@ -842,7 +911,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 #if __cplusplus >= 201103L
       const_iterator
       cbegin() const noexcept
-      { return const_iterator(this->_M_impl._M_start._M_p, 0); }
+      { return this->_M_impl._M_cbegin(); }
 
       const_iterator
       cend() const noexcept
@@ -884,17 +953,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
@@ -961,10 +1024,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());
       }
@@ -1076,7 +1140,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       flip() _GLIBCXX_NOEXCEPT
       {
 	_Bit_type * const __end = this->_M_impl._M_end_addr();
-	for (_Bit_type * __p = this->_M_impl._M_start._M_p; __p != __end; ++__p)
+	for (_Bit_type * __p = this->_M_impl._M_start_p(); __p != __end; ++__p)
 	  *__p = ~*__p;
       }
 
@@ -1123,21 +1187,17 @@ _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);
+	    this->_M_impl._M_set_start(std::__addressof(*__q));
+	    this->_M_impl._M_finish = this->_M_impl._M_begin() + 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);
-
+	  this->_M_impl._M_reset();
       }
 
       void
       _M_initialize_value(bool __x)
       {
-	if (_Bit_type* __p = this->_M_impl._M_start._M_p)
+	if (_Bit_type* __p = this->_M_impl._M_start_p())
 	  __builtin_memset(__p, __x ? ~0 : 0,
 			   (this->_M_impl._M_end_addr() - __p)
 			   * sizeof(_Bit_type));
@@ -1186,7 +1246,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
diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
index 8df0f4180d4..46b42e05ecd 100644
--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -822,7 +822,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       iterator __start(std::__addressof(*__q), 0);
       iterator __finish(_M_copy_aligned(begin(), end(), __start));
       this->_M_deallocate();
-      this->_M_impl._M_start = __start;
+      this->_M_impl._M_set_start(__start._M_p);
       this->_M_impl._M_finish = __finish;
       this->_M_impl._M_end_of_storage = __q + _S_nword(__n);
     }
@@ -853,7 +853,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 					__i + difference_type(__n));
 	  this->_M_deallocate();
 	  this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
-	  this->_M_impl._M_start = __start;
+	  this->_M_impl._M_set_start(__start._M_p);
 	  this->_M_impl._M_finish = __finish;
 	}
     }
@@ -887,7 +887,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 		iterator __finish = std::copy(__position, end(), __i);
 		this->_M_deallocate();
 		this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
-		this->_M_impl._M_start = __start;
+		this->_M_impl._M_set_start(__start._M_p);
 		this->_M_impl._M_finish = __finish;
 	      }
 	  }
@@ -916,7 +916,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  iterator __finish = std::copy(__position, end(), __i);
 	  this->_M_deallocate();
 	  this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
-	  this->_M_impl._M_start = __start;
+	  this->_M_impl._M_set_start(__start._M_p);
 	  this->_M_impl._M_finish = __finish;
 	}
     }
@@ -983,7 +983,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       if (__words)
 	{
 	  const size_t __clength = __words * sizeof(_Bit_type);
-	  __hash = std::_Hash_impl::hash(__b._M_impl._M_start._M_p, __clength);
+	  __hash = std::_Hash_impl::hash(__b._M_impl._M_start_p(), __clength);
 	}
 
       const size_t __extrabits = __b.size() % _S_word_bit;
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 62429424a5f..fa2b053c66e 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> fix & enhancements

[François Dumont]

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

Running tests in C++98 mode show that I had forgotten a 'return *this;' 
in _Bvector_impl_data::operator=.

So here is the patch again.

On 10/30/18 7:28 AM, François Dumont wrote:
> Following Marc Glisse change to ignore _M_start offset I wanted to go 
> a little step further and just remove it in _GLIBCXX_INLINE_VERSION mode.
>
> I also fix a regression we already fixed on mainstream std::vector 
> regarding noexcept qualification of move constructor with allocator.
>
> And I implemented the same optimizations than in std::vector for 
> allocators always comparing equals and for the std::swap operation.
>
> I also avoid re-implementing in vector::operator[] the same code 
> already implemented in iterator::operator[] but this one should 
> perhaps go in a different commit.
>
>
>     * include/bits/stl_bvector.h
>     [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
>     _Bit_type*.
>     (_Bvector_impl_data(const _Bvector_impl_data&)): New.
>     (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>     (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): New.
> (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>     (_Bvector_impl_data::_M_reset()): Likewise.
>     (_Bvector_impl_data::_M_begin()): New.
>     (_Bvector_impl_data::_M_cbegin()): New.
>     (_Bvector_impl_data::_M_start_p()): New.
>     (_Bvector_impl_data::_M_set_start(_Bit_type*)): New.
>     (_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.
>     (_Bvector_base::_M_deallocate()): Adapt.
>     (vector::vector(const vector&, const allocator_type&)): Adapt.
>     (vector::vector(vector&&, const allocator_type&, true_type)): New.
>     (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.
>     (vector::begin()): Adapt.
>     (vector::cbegin()): Adapt.
>     (vector::operator[](size_type)): Use iterator operator[].
>     (vector::swap(vector&)): Adapt.
>     (vector::flip()): Adapt.
>     (vector::_M_initialize(size_type)): Adapt.
>     (vector::_M_initialize_value(bool)): Adapt.
>     * include/bits/vector.tcc:
>     (vector<bool>::_M_reallocate(size_type)): Adapt.
>     (vector<bool>::_M_fill_insert(iterator, size_type, bool)): Adapt.
>     (vector<bool>::_M_insert_range<_FwdIter>(iterator, _FwdIter, _FwdIter
>     std::forward_iterator_tag)): Adapt.
>     (vector<bool>::_M_insert_aux(iterator, bool)): Adapt.
>     (std::hash<std::vector<bool>>::operator()): Adapt.
>     * 
> testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>     Add check.
>
> Tested under Linux x86_64.
>
> Ok to commit ?
>
> François
>


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

diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index 0bcfd19fd3e..dd0f00fe07f 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -437,7 +437,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       struct _Bvector_impl_data
       {
+#if !_GLIBCXX_INLINE_VERSION
 	_Bit_iterator	_M_start;
+#else
+	_Bit_type*	_M_start;
+#endif
 	_Bit_iterator	_M_finish;
 	_Bit_pointer	_M_end_of_storage;
 
@@ -447,32 +451,75 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
 #if __cplusplus >= 201103L
 	_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(); }
 
+	_Bvector_impl_data(const _Bvector_impl_data&) = default;
+	_Bvector_impl_data&
+	operator=(const _Bvector_impl_data&) = default;
+
 	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();
 	}
+#else
+	_Bvector_impl_data(const _Bvector_impl_data& __x)
+	: _M_start(__x._M_start), _M_finish(__x._M_finish)
+	, _M_end_of_storage(__x._M_end_of_storage)
+	{ }
+
+	_Bvector_impl_data&
+	operator=(const _Bvector_impl_data& __x)
+	{
+	  _M_start = __x._M_start;
+	  _M_finish = __x._M_finish;
+	  _M_end_of_storage = __x._M_end_of_storage;
+	  return *this;
+	}
+#endif
+
+	_Bit_iterator
+	_M_begin() const _GLIBCXX_NOEXCEPT
+	{ return _Bit_iterator(_M_start_p(), 0); }
+
+	_Bit_const_iterator
+	_M_cbegin() const _GLIBCXX_NOEXCEPT
+	{ return _Bit_const_iterator(_M_start_p(), 0); }
+
+	_Bit_type*
+	_M_start_p() const _GLIBCXX_NOEXCEPT
+#if !_GLIBCXX_INLINE_VERSION
+	{ return _M_start._M_p; }
+#else
+	{ return _M_start; }
+#endif
+
+	void
+	_M_set_start(_Bit_type* __p) _GLIBCXX_NOEXCEPT
+#if !_GLIBCXX_INLINE_VERSION
+	{ _M_start._M_p = __p; }
+#else
+	{ _M_start = __p; }
 #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()
@@ -483,7 +530,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*
@@ -521,6 +576,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()
@@ -536,9 +595,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       void
       _M_deallocate()
       {
-	if (_M_impl._M_start._M_p)
+	if (_M_impl._M_start_p())
 	  {
-	    const size_t __n = _M_impl._M_end_addr() - _M_impl._M_start._M_p;
+	    const size_t __n = _M_impl._M_end_addr() - _M_impl._M_start_p();
 	    _Bit_alloc_traits::deallocate(_M_impl,
 					  _M_impl._M_end_of_storage - __n,
 					  __n);
@@ -657,14 +716,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)
@@ -677,11 +740,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,
@@ -809,11 +879,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       iterator
       begin() _GLIBCXX_NOEXCEPT
-      { return iterator(this->_M_impl._M_start._M_p, 0); }
+      { return this->_M_impl._M_begin(); }
 
       const_iterator
       begin() const _GLIBCXX_NOEXCEPT
-      { return const_iterator(this->_M_impl._M_start._M_p, 0); }
+      { return this->_M_impl._M_cbegin(); }
 
       iterator
       end() _GLIBCXX_NOEXCEPT
@@ -842,7 +912,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 #if __cplusplus >= 201103L
       const_iterator
       cbegin() const noexcept
-      { return const_iterator(this->_M_impl._M_start._M_p, 0); }
+      { return this->_M_impl._M_cbegin(); }
 
       const_iterator
       cend() const noexcept
@@ -884,17 +954,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
@@ -961,10 +1025,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());
       }
@@ -1076,7 +1141,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       flip() _GLIBCXX_NOEXCEPT
       {
 	_Bit_type * const __end = this->_M_impl._M_end_addr();
-	for (_Bit_type * __p = this->_M_impl._M_start._M_p; __p != __end; ++__p)
+	for (_Bit_type * __p = this->_M_impl._M_start_p(); __p != __end; ++__p)
 	  *__p = ~*__p;
       }
 
@@ -1123,21 +1188,17 @@ _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);
+	    this->_M_impl._M_set_start(std::__addressof(*__q));
+	    this->_M_impl._M_finish = this->_M_impl._M_begin() + 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);
-
+	  this->_M_impl._M_reset();
       }
 
       void
       _M_initialize_value(bool __x)
       {
-	if (_Bit_type* __p = this->_M_impl._M_start._M_p)
+	if (_Bit_type* __p = this->_M_impl._M_start_p())
 	  __builtin_memset(__p, __x ? ~0 : 0,
 			   (this->_M_impl._M_end_addr() - __p)
 			   * sizeof(_Bit_type));
@@ -1186,7 +1247,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
diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
index 8df0f4180d4..46b42e05ecd 100644
--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -822,7 +822,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       iterator __start(std::__addressof(*__q), 0);
       iterator __finish(_M_copy_aligned(begin(), end(), __start));
       this->_M_deallocate();
-      this->_M_impl._M_start = __start;
+      this->_M_impl._M_set_start(__start._M_p);
       this->_M_impl._M_finish = __finish;
       this->_M_impl._M_end_of_storage = __q + _S_nword(__n);
     }
@@ -853,7 +853,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 					__i + difference_type(__n));
 	  this->_M_deallocate();
 	  this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
-	  this->_M_impl._M_start = __start;
+	  this->_M_impl._M_set_start(__start._M_p);
 	  this->_M_impl._M_finish = __finish;
 	}
     }
@@ -887,7 +887,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 		iterator __finish = std::copy(__position, end(), __i);
 		this->_M_deallocate();
 		this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
-		this->_M_impl._M_start = __start;
+		this->_M_impl._M_set_start(__start._M_p);
 		this->_M_impl._M_finish = __finish;
 	      }
 	  }
@@ -916,7 +916,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  iterator __finish = std::copy(__position, end(), __i);
 	  this->_M_deallocate();
 	  this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
-	  this->_M_impl._M_start = __start;
+	  this->_M_impl._M_set_start(__start._M_p);
 	  this->_M_impl._M_finish = __finish;
 	}
     }
@@ -983,7 +983,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       if (__words)
 	{
 	  const size_t __clength = __words * sizeof(_Bit_type);
-	  __hash = std::_Hash_impl::hash(__b._M_impl._M_start._M_p, __clength);
+	  __hash = std::_Hash_impl::hash(__b._M_impl._M_start_p(), __clength);
 	}
 
       const size_t __extrabits = __b.size() % _S_word_bit;
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 62429424a5f..fa2b053c66e 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> fix & enhancements

[Jonathan Wakely]

On 30/10/18 07:28 +0100, François Dumont wrote:
>Following Marc Glisse change to ignore _M_start offset I wanted to go 
>a little step further and just remove it in _GLIBCXX_INLINE_VERSION 
>mode.
>
>I also fix a regression we already fixed on mainstream std::vector 
>regarding noexcept qualification of move constructor with allocator.
>
>And I implemented the same optimizations than in std::vector for 
>allocators always comparing equals and for the std::swap operation.
>
>I also avoid re-implementing in vector::operator[] the same code 
>already implemented in iterator::operator[] but this one should 
>perhaps go in a different commit.
>
>
>    * include/bits/stl_bvector.h
>    [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
>    _Bit_type*.
>    (_Bvector_impl_data(const _Bvector_impl_data&)): New.
>    (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>    (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): New.
>(_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>    (_Bvector_impl_data::_M_reset()): Likewise.
>    (_Bvector_impl_data::_M_begin()): New.
>    (_Bvector_impl_data::_M_cbegin()): New.
>    (_Bvector_impl_data::_M_start_p()): New.
>    (_Bvector_impl_data::_M_set_start(_Bit_type*)): New.
>    (_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.
>    (_Bvector_base::_M_deallocate()): Adapt.
>    (vector::vector(const vector&, const allocator_type&)): Adapt.
>    (vector::vector(vector&&, const allocator_type&, true_type)): New.
>    (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.
>    (vector::begin()): Adapt.
>    (vector::cbegin()): Adapt.
>    (vector::operator[](size_type)): Use iterator operator[].
>    (vector::swap(vector&)): Adapt.
>    (vector::flip()): Adapt.
>    (vector::_M_initialize(size_type)): Adapt.
>    (vector::_M_initialize_value(bool)): Adapt.
>    * include/bits/vector.tcc:
>    (vector<bool>::_M_reallocate(size_type)): Adapt.
>    (vector<bool>::_M_fill_insert(iterator, size_type, bool)): Adapt.
>    (vector<bool>::_M_insert_range<_FwdIter>(iterator, _FwdIter, _FwdIter
>    std::forward_iterator_tag)): Adapt.
>    (vector<bool>::_M_insert_aux(iterator, bool)): Adapt.
>    (std::hash<std::vector<bool>>::operator()): Adapt.
>    * testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>    Add check.
>
>Tested under Linux x86_64.
>
>Ok to commit ?
>
>François
>

>diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
>index 8fbef7a1a3a..81b4a75236d 100644
>--- a/libstdc++-v3/include/bits/stl_bvector.h
>+++ b/libstdc++-v3/include/bits/stl_bvector.h
>@@ -437,7 +437,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> 
>       struct _Bvector_impl_data
>       {
>+#if !_GLIBCXX_INLINE_VERSION
> 	_Bit_iterator	_M_start;
>+#else
>+	_Bit_type*	_M_start;
>+#endif

An alternative that would require fewer changes elsewhere in the file
would be:

#if !_GLIBCXX_INLINE_VERSION
 	_Bit_iterator	_M_start;
#else
        // We don't need the offset field for the start pointer,
        // 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

Now the rest of the file doesn't need any checks for
_GLIBCXX_INLINE_VERSION (which I'd prefer to avoid, because it
clutters the code up with extra conditionals for a mode that
**nobody** uses in practice).


> 	_Bit_iterator	_M_finish;
> 	_Bit_pointer	_M_end_of_storage;
> 
>@@ -447,32 +451,74 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> 
> #if __cplusplus >= 201103L
> 	_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(); }
> 
>+	_Bvector_impl_data(const _Bvector_impl_data&) = default;
>+	_Bvector_impl_data&
>+	operator=(const _Bvector_impl_data&) = default;
> 	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();
> 	}
>+#else
>+	_Bvector_impl_data(const _Bvector_impl_data& __x)
>+	  : _M_start(__x._M_start), _M_finish(__x._M_finish)
>+	  , _M_end_of_storage(__x._M_end_of_storage)
>+	{ }

Do we need this definition? Won't the compiler generate the same thing
for us anyway?

>+	_Bvector_impl_data&
>+	operator=(const _Bvector_impl_data& __x)
>+	{
>+	  _M_start = __x._M_start;
>+	  _M_finish = __x._M_finish;
>+	  _M_end_of_storage = __x._M_end_of_storage;
>+	}

Same here.

>+#endif
>+
>+	_Bit_iterator
>+	_M_begin() const _GLIBCXX_NOEXCEPT
>+	{ return _Bit_iterator(_M_start_p(), 0); }
>+
>+	_Bit_const_iterator
>+	_M_cbegin() const _GLIBCXX_NOEXCEPT
>+	{ return _Bit_const_iterator(_M_start_p(), 0); }
>+
>+	_Bit_type*
>+	_M_start_p() const _GLIBCXX_NOEXCEPT

We already have _M_end_addr() so would _M_begin_addr be a better name
for this new function?

>+#if !_GLIBCXX_INLINE_VERSION
>+	{ return _M_start._M_p; }
>+#else
>+	{ return _M_start; }
>+#endif

Using the suggestion above you wouldn't need #if because return
_M_start._M_p would always be right. But you wouldn't even need this
function at all, the code that uses _M_start._M_p would Just Workâ„¢.


>+	void
>+	_M_set_start(_Bit_type* __p) _GLIBCXX_NOEXCEPT
>+#if !_GLIBCXX_INLINE_VERSION
>+	{ _M_start._M_p = __p; }
>+#else
>+	{ _M_start = __p; }
> #endif

You wouldn't need this function.

etc. etc.


> 
> 	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( noexcept(_Bit_alloc_type()) )

You'll need to rebase the patch, because I just fixed a bug in this
exception specification (r265626).

> 	: _Bit_alloc_type()

Re: std::vector<bool> fix & enhancements

[François Dumont]

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

Here is the updated patch.

I forgot that pre-C+11 we already had the default copy constructor and 
assignment operators. And with the trick on definition of _M_start the 
patch is much smaller.

     * 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.
     (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)): Adapt.
     * testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
     Add check.

Tested under Linux x86_64 w/o versioned namespace, w/o C++98 mode.

Ok to commit ?

François


On 10/30/18 10:59 PM, Jonathan Wakely wrote:
> On 30/10/18 07:28 +0100, François Dumont wrote:
>> Following Marc Glisse change to ignore _M_start offset I wanted to go 
>> a little step further and just remove it in _GLIBCXX_INLINE_VERSION 
>> mode.
>>
>> I also fix a regression we already fixed on mainstream std::vector 
>> regarding noexcept qualification of move constructor with allocator.
>>
>> And I implemented the same optimizations than in std::vector for 
>> allocators always comparing equals and for the std::swap operation.
>>
>> I also avoid re-implementing in vector::operator[] the same code 
>> already implemented in iterator::operator[] but this one should 
>> perhaps go in a different commit.
>>
>>
>>     * include/bits/stl_bvector.h
>>     [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
>>     _Bit_type*.
>>     (_Bvector_impl_data(const _Bvector_impl_data&)): New.
>>     (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>>     (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): New.
>> (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>>     (_Bvector_impl_data::_M_reset()): Likewise.
>>     (_Bvector_impl_data::_M_begin()): New.
>>     (_Bvector_impl_data::_M_cbegin()): New.
>>     (_Bvector_impl_data::_M_start_p()): New.
>>     (_Bvector_impl_data::_M_set_start(_Bit_type*)): New.
>>     (_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.
>>     (_Bvector_base::_M_deallocate()): Adapt.
>>     (vector::vector(const vector&, const allocator_type&)): Adapt.
>>     (vector::vector(vector&&, const allocator_type&, true_type)): New.
>>     (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.
>>     (vector::begin()): Adapt.
>>     (vector::cbegin()): Adapt.
>>     (vector::operator[](size_type)): Use iterator operator[].
>>     (vector::swap(vector&)): Adapt.
>>     (vector::flip()): Adapt.
>>     (vector::_M_initialize(size_type)): Adapt.
>>     (vector::_M_initialize_value(bool)): Adapt.
>>     * include/bits/vector.tcc:
>>     (vector<bool>::_M_reallocate(size_type)): Adapt.
>>     (vector<bool>::_M_fill_insert(iterator, size_type, bool)): Adapt.
>> (vector<bool>::_M_insert_range<_FwdIter>(iterator, _FwdIter, _FwdIter
>>     std::forward_iterator_tag)): Adapt.
>>     (vector<bool>::_M_insert_aux(iterator, bool)): Adapt.
>>     (std::hash<std::vector<bool>>::operator()): Adapt.
>>     * 
>> testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>>     Add check.
>>
>> Tested under Linux x86_64.
>>
>> Ok to commit ?
>>
>> François
>>
>
>> diff --git a/libstdc++-v3/include/bits/stl_bvector.h 
>> b/libstdc++-v3/include/bits/stl_bvector.h
>> index 8fbef7a1a3a..81b4a75236d 100644
>> --- a/libstdc++-v3/include/bits/stl_bvector.h
>> +++ b/libstdc++-v3/include/bits/stl_bvector.h
>> @@ -437,7 +437,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>
>>       struct _Bvector_impl_data
>>       {
>> +#if !_GLIBCXX_INLINE_VERSION
>>     _Bit_iterator    _M_start;
>> +#else
>> +    _Bit_type*    _M_start;
>> +#endif
>
> An alternative that would require fewer changes elsewhere in the file
> would be:
>
> #if !_GLIBCXX_INLINE_VERSION
>     _Bit_iterator    _M_start;
> #else
>        // We don't need the offset field for the start pointer,
>        // 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
>
> Now the rest of the file doesn't need any checks for
> _GLIBCXX_INLINE_VERSION (which I'd prefer to avoid, because it
> clutters the code up with extra conditionals for a mode that
> **nobody** uses in practice).
>
>
>>     _Bit_iterator    _M_finish;
>>     _Bit_pointer    _M_end_of_storage;
>>
>> @@ -447,32 +451,74 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>
>> #if __cplusplus >= 201103L
>>     _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(); }
>>
>> +    _Bvector_impl_data(const _Bvector_impl_data&) = default;
>> +    _Bvector_impl_data&
>> +    operator=(const _Bvector_impl_data&) = default;
>>     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();
>>     }
>> +#else
>> +    _Bvector_impl_data(const _Bvector_impl_data& __x)
>> +      : _M_start(__x._M_start), _M_finish(__x._M_finish)
>> +      , _M_end_of_storage(__x._M_end_of_storage)
>> +    { }
>
> Do we need this definition? Won't the compiler generate the same thing
> for us anyway?
>
>> +    _Bvector_impl_data&
>> +    operator=(const _Bvector_impl_data& __x)
>> +    {
>> +      _M_start = __x._M_start;
>> +      _M_finish = __x._M_finish;
>> +      _M_end_of_storage = __x._M_end_of_storage;
>> +    }
>
> Same here.
>
>> +#endif
>> +
>> +    _Bit_iterator
>> +    _M_begin() const _GLIBCXX_NOEXCEPT
>> +    { return _Bit_iterator(_M_start_p(), 0); }
>> +
>> +    _Bit_const_iterator
>> +    _M_cbegin() const _GLIBCXX_NOEXCEPT
>> +    { return _Bit_const_iterator(_M_start_p(), 0); }
>> +
>> +    _Bit_type*
>> +    _M_start_p() const _GLIBCXX_NOEXCEPT
>
> We already have _M_end_addr() so would _M_begin_addr be a better name
> for this new function?
>
>> +#if !_GLIBCXX_INLINE_VERSION
>> +    { return _M_start._M_p; }
>> +#else
>> +    { return _M_start; }
>> +#endif
>
> Using the suggestion above you wouldn't need #if because return
> _M_start._M_p would always be right. But you wouldn't even need this
> function at all, the code that uses _M_start._M_p would Just Workâ„¢.
>
>
>> +    void
>> +    _M_set_start(_Bit_type* __p) _GLIBCXX_NOEXCEPT
>> +#if !_GLIBCXX_INLINE_VERSION
>> +    { _M_start._M_p = __p; }
>> +#else
>> +    { _M_start = __p; }
>> #endif
>
> You wouldn't need this function.
>
> etc. etc.
>
>
>>
>>     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( noexcept(_Bit_alloc_type()) )
>
> You'll need to rebase the patch, because I just fixed a bug in this
> exception specification (r265626).
>
>>     : _Bit_alloc_type()
>


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

diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index 0bcfd19fd3e..387f71b8b62 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -437,7 +437,17 @@ _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 pointer,
+	// 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;
 
@@ -446,33 +456,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()
@@ -483,7 +498,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*
@@ -521,6 +544,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()
@@ -657,14 +684,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)
@@ -677,11 +708,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,
@@ -884,17 +922,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
@@ -961,10 +993,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());
       }
@@ -1123,15 +1156,12 @@ _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);
-
+	  this->_M_impl._M_reset();
       }
 
       void
@@ -1186,7 +1216,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
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 62429424a5f..fa2b053c66e 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" );