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
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" );
