Re: Default std::vector<bool> default and move constructor

[François Dumont <frs dot dumont at gmail dot com>]

On 01/06/2017 15:34, Jonathan Wakely wrote:
> I would expect the constructor to look like this:
>
>       _Bvector_impl()
>       _GLIBCXX_NOEXCEPT_IF( noexcept(_Bit_alloc_type()) )
>          : _Bit_alloc_type()
>          { }
>
> What happens when you do that?
>
>
> >       _Bvector_impl(const _Bit_alloc_type& __a)
> > -    : _Bit_alloc_type(__a), _M_start(), _M_finish(), 
> > _M_end_of_storage()
> > +         _GLIBCXX_NOEXCEPT_IF( noexcept(_Bit_alloc_type(__a)) )
>
> Copying the allocator is not allowed to throw. You can use simply
> _GLIBCXX_NOEXCEPT here.

Now that we find out what was the problem with default/value 
initialization of allocator I would like to re-submit this patch with 
the correct constructor.

Tested under Linux x86_64 normal mode.

Ok to commit ?

François

diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index 78195c1..6e58503 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -388,10 +388,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
   { return __x + __n; }
 
   inline void
-  __fill_bvector(_Bit_iterator __first, _Bit_iterator __last, bool __x)
+  __fill_bvector(_Bit_type * __v,
+		 unsigned int __first, unsigned int __last, bool __x)
   {
-    for (; __first != __last; ++__first)
-      *__first = __x;
+    const _Bit_type __fmask = ~0ul << __first;
+    const _Bit_type __lmask = ~0ul >> (_S_word_bit - __last);
+    const _Bit_type __mask = __fmask & __lmask;
+
+    if (__x)
+      *__v |= __mask;
+    else
+      *__v &= ~__mask;
   }
 
   inline void
@@ -399,12 +406,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
   {
     if (__first._M_p != __last._M_p)
       {
-	std::fill(__first._M_p + 1, __last._M_p, __x ? ~0 : 0);
-	__fill_bvector(__first, _Bit_iterator(__first._M_p + 1, 0), __x);
-	__fill_bvector(_Bit_iterator(__last._M_p, 0), __last, __x);
+	_Bit_type *__first_p = __first._M_p;
+	if (__first._M_offset != 0)
+	  __fill_bvector(__first_p++, __first._M_offset, _S_word_bit, __x);
+
+	__builtin_memset(__first_p, __x ? ~0 : 0,
+			 (__last._M_p - __first_p) * sizeof(_Bit_type));
+
+	if (__last._M_offset != 0)
+	  __fill_bvector(__last._M_p, 0, __last._M_offset, __x);
       }
     else
-      __fill_bvector(__first, __last, __x);
+      __fill_bvector(__first._M_p, __first._M_offset, __last._M_offset, __x);
   }
 
   template<typename _Alloc>
@@ -416,33 +429,62 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	_Bit_alloc_traits;
       typedef typename _Bit_alloc_traits::pointer _Bit_pointer;
 
-      struct _Bvector_impl
-      : public _Bit_alloc_type
+      struct _Bvector_impl_data
       {
 	_Bit_iterator 	_M_start;
 	_Bit_iterator 	_M_finish;
 	_Bit_pointer 	_M_end_of_storage;
 
+	_Bvector_impl_data() _GLIBCXX_NOEXCEPT
+	: _M_start(), _M_finish(), _M_end_of_storage()
+	{ }
+
+#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)
+	{ __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;
+	  __x._M_reset();
+	}
+#endif
+
+	void
+	_M_reset() _GLIBCXX_NOEXCEPT
+	{
+	  _M_start = _M_finish = _Bit_iterator();
+	  _M_end_of_storage = _Bit_pointer();
+	}
+      };
+
+      struct _Bvector_impl
+	: public _Bit_alloc_type, public _Bvector_impl_data
+	{
+	public:
 	  _Bvector_impl()
-	: _Bit_alloc_type(), _M_start(), _M_finish(), _M_end_of_storage()
+	    _GLIBCXX_NOEXCEPT_IF( noexcept(_Bit_alloc_type()) )
+	  : _Bit_alloc_type()
 	  { }
 
-	_Bvector_impl(const _Bit_alloc_type& __a)
-	: _Bit_alloc_type(__a), _M_start(), _M_finish(), _M_end_of_storage()
+	  _Bvector_impl(const _Bit_alloc_type& __a) _GLIBCXX_NOEXCEPT
+	  : _Bit_alloc_type(__a)
 	  { }
 
 #if __cplusplus >= 201103L
-	_Bvector_impl(_Bit_alloc_type&& __a)
-	: _Bit_alloc_type(std::move(__a)), _M_start(), _M_finish(),
-	  _M_end_of_storage()
-	{ }
+	_Bvector_impl(_Bvector_impl&&) = default;
 #endif
 
 	_Bit_type*
 	_M_end_addr() const _GLIBCXX_NOEXCEPT
 	{
-	  if (_M_end_of_storage)
-	    return std::__addressof(_M_end_of_storage[-1]) + 1;
+	  if (this->_M_end_of_storage)
+	    return std::__addressof(this->_M_end_of_storage[-1]) + 1;
 	  return 0;
 	}
       };
@@ -452,33 +494,27 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
       _Bit_alloc_type&
       _M_get_Bit_allocator() _GLIBCXX_NOEXCEPT
-      { return *static_cast<_Bit_alloc_type*>(&this->_M_impl); }
+      { return this->_M_impl; }
 
       const _Bit_alloc_type&
       _M_get_Bit_allocator() const _GLIBCXX_NOEXCEPT
-      { return *static_cast<const _Bit_alloc_type*>(&this->_M_impl); }
+      { return this->_M_impl; }
 
       allocator_type
       get_allocator() const _GLIBCXX_NOEXCEPT
       { return allocator_type(_M_get_Bit_allocator()); }
 
-      _Bvector_base()
-      : _M_impl() { }
+#if __cplusplus >= 201103L
+      _Bvector_base() = default;
+#else
+      _Bvector_base() { }
+#endif
       
       _Bvector_base(const allocator_type& __a)
       : _M_impl(__a) { }
 
 #if __cplusplus >= 201103L
-      _Bvector_base(_Bvector_base&& __x) noexcept
-      : _M_impl(std::move(__x._M_get_Bit_allocator()))
-      {
-	this->_M_impl._M_start = __x._M_impl._M_start;
-	this->_M_impl._M_finish = __x._M_impl._M_finish;
-	this->_M_impl._M_end_of_storage = __x._M_impl._M_end_of_storage;
-	__x._M_impl._M_start = _Bit_iterator();
-	__x._M_impl._M_finish = _Bit_iterator();
-	__x._M_impl._M_end_of_storage = nullptr;
-      }
+      _Bvector_base(_Bvector_base&&) = default;
 #endif
 
       ~_Bvector_base()
@@ -500,11 +536,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	    _Bit_alloc_traits::deallocate(_M_impl,
 					  _M_impl._M_end_of_storage - __n,
 					  __n);
-	    _M_impl._M_start = _M_impl._M_finish = _Bit_iterator();
-	    _M_impl._M_end_of_storage = _Bit_pointer();
+	    _M_impl._M_reset();
 	  }
       }
 
+#if __cplusplus >= 201103L
+      void
+      _M_move_data(_Bvector_base&& __x) noexcept
+      { _M_impl._M_move_data(std::move(__x._M_impl)); }
+#endif
+
       static size_t
       _S_nword(size_t __n)
       { return (__n + int(_S_word_bit) - 1) / int(_S_word_bit); }
@@ -564,7 +605,8 @@ template<typename _Alloc>
       typedef std::reverse_iterator<iterator>		reverse_iterator;
       typedef _Alloc					allocator_type;
 
-    allocator_type get_allocator() const
+      allocator_type
+      get_allocator() const
       { return _Base::get_allocator(); }
 
     protected:
@@ -574,11 +616,11 @@ template<typename _Alloc>
       using _Base::_M_get_Bit_allocator;
 
     public:
-    vector()
 #if __cplusplus >= 201103L
-      noexcept(is_nothrow_default_constructible<allocator_type>::value)
+      vector() = default;
+#else
+      vector() { }
 #endif
-    : _Base() { }
 
       explicit
       vector(const allocator_type& __a)
@@ -592,23 +634,16 @@ template<typename _Alloc>
 
       vector(size_type __n, const bool& __value, 
 	     const allocator_type& __a = allocator_type())
-    : _Base(__a)
-    {
-      _M_initialize(__n);
-      std::fill(this->_M_impl._M_start._M_p, this->_M_impl._M_end_addr(),
-		__value ? ~0 : 0);
-    }
 #else
       explicit
       vector(size_type __n, const bool& __value = bool(), 
 	     const allocator_type& __a = allocator_type())
+#endif
       : _Base(__a)
       {
 	_M_initialize(__n);
-      std::fill(this->_M_impl._M_start._M_p, this->_M_impl._M_end_addr(),
-		__value ? ~0 : 0);
+	_M_initialize_value(__value);
       }
-#endif
 
       vector(const vector& __x)
       : _Base(_Bit_alloc_traits::_S_select_on_copy(__x._M_get_Bit_allocator()))
@@ -618,22 +653,14 @@ template<typename _Alloc>
       }
 
 #if __cplusplus >= 201103L
-    vector(vector&& __x) noexcept
-    : _Base(std::move(__x)) { }
+      vector(vector&&) = default;
 
       vector(vector&& __x, const allocator_type& __a)
       noexcept(_Bit_alloc_traits::_S_always_equal())
       : _Base(__a)
       {
 	if (__x.get_allocator() == __a)
-	{
-	  this->_M_impl._M_start = __x._M_impl._M_start;
-	  this->_M_impl._M_finish = __x._M_impl._M_finish;
-	  this->_M_impl._M_end_of_storage = __x._M_impl._M_end_of_storage;
-	  __x._M_impl._M_start = _Bit_iterator();
-	  __x._M_impl._M_finish = _Bit_iterator();
-	  __x._M_impl._M_end_of_storage = nullptr;
-	}
+	  this->_M_move_data(std::move(__x));
 	else
 	  {
 	    _M_initialize(__x.size());
@@ -716,12 +743,7 @@ template<typename _Alloc>
 	    || this->_M_get_Bit_allocator() == __x._M_get_Bit_allocator())
 	  {
 	    this->_M_deallocate();
-	  this->_M_impl._M_start = __x._M_impl._M_start;
-	  this->_M_impl._M_finish = __x._M_impl._M_finish;
-	  this->_M_impl._M_end_of_storage = __x._M_impl._M_end_of_storage;
-	  __x._M_impl._M_start = _Bit_iterator();
-	  __x._M_impl._M_finish = _Bit_iterator();
-	  __x._M_impl._M_end_of_storage = nullptr;
+	    this->_M_move_data(std::move(__x));
 	    std::__alloc_on_move(_M_get_Bit_allocator(),
 				 __x._M_get_Bit_allocator());
 	  }
@@ -760,7 +782,7 @@ template<typename _Alloc>
 	       typename = std::_RequireInputIter<_InputIterator>>
 	void
 	assign(_InputIterator __first, _InputIterator __last)
-      { _M_assign_dispatch(__first, __last, __false_type()); }
+	{ _M_assign_aux(__first, __last, std::__iterator_category(__first)); }
 #else
       template<typename _InputIterator>
 	void
@@ -774,7 +796,7 @@ template<typename _Alloc>
 #if __cplusplus >= 201103L
       void
       assign(initializer_list<bool> __l)
-    { this->assign(__l.begin(), __l.end()); }
+      { _M_assign_aux(__l.begin(), __l.end(), random_access_iterator_tag()); }
 #endif
 
       iterator
@@ -1101,6 +1123,15 @@ template<typename _Alloc>
 	    this->_M_impl._M_start = iterator(0, 0);
 	  }
 	this->_M_impl._M_finish = this->_M_impl._M_start + difference_type(__n);
+
+      }
+
+      void
+      _M_initialize_value(bool __x)
+      {
+	__builtin_memset(this->_M_impl._M_start._M_p, __x ? ~0 : 0,
+		(this->_M_impl._M_end_addr() - this->_M_impl._M_start._M_p)
+		* sizeof(_Bit_type));
       }
 
       void
@@ -1120,8 +1151,7 @@ template<typename _Alloc>
 	_M_initialize_dispatch(_Integer __n, _Integer __x, __true_type)
 	{
 	  _M_initialize(static_cast<size_type>(__n));
-	std::fill(this->_M_impl._M_start._M_p, 
-		  this->_M_impl._M_end_addr(), __x ? ~0 : 0);
+	  _M_initialize_value(__x);
 	}
 
       template<typename _InputIterator>
@@ -1168,15 +1198,13 @@ template<typename _Alloc>
       {
 	if (__n > size())
 	  {
-	  std::fill(this->_M_impl._M_start._M_p, 
-		    this->_M_impl._M_end_addr(), __x ? ~0 : 0);
+	    _M_initialize_value(__x);
 	    insert(end(), __n - size(), __x);
 	  }
 	else
 	  {
 	    _M_erase_at_end(begin() + __n);
-	  std::fill(this->_M_impl._M_start._M_p, 
-		    this->_M_impl._M_end_addr(), __x ? ~0 : 0);
+	    _M_initialize_value(__x);
 	  }
       }
 
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/default_init.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/default_init.cc
new file mode 100644
index 0000000..9ee4697
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/default_init.cc
@@ -0,0 +1,67 @@
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++11 } }
+// { dg-options "-O0" }
+// { dg-xfail-run-if "PR c++/65816" { *-*-* } }
+
+#include <vector>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+#include <ext/aligned_buffer.h>
+
+using T = bool;
+
+using __gnu_test::default_init_allocator;
+
+void test01()
+{
+  typedef default_init_allocator<T> alloc_type;
+  typedef std::vector<T, alloc_type> test_type;
+
+  __gnu_cxx::__aligned_buffer<test_type> buf;
+  __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+  test_type *tmp = ::new(buf._M_addr()) test_type;
+
+  VERIFY( tmp->get_allocator().state == 0 );
+
+  tmp->~test_type();
+}
+
+void test02()
+{
+  typedef default_init_allocator<T> alloc_type;
+  typedef std::vector<T, alloc_type> test_type;
+
+  __gnu_cxx::__aligned_buffer<test_type> buf;
+  __builtin_memset(buf._M_addr(), ~0, sizeof(test_type));
+
+  test_type *tmp = ::new(buf._M_addr()) test_type();
+
+  VERIFY( tmp->get_allocator().state == 0 );
+
+  tmp->~test_type();
+}
+
+int main()
+{
+  test01();
+  test02();
+  return 0;
+}