diff mbox

[v3] More noexcept for vectors

Message ID alpine.DEB.2.02.1309151042420.30089@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse Sept. 15, 2013, 9:12 a.m. UTC
I had to separate the constructor that takes an allocator from the default 
constructor in debug/profile, since in release the noexcept only applies 
to one of them (and the testsuite asserts that release and debug agree on 
this). An alternative would be to make the release vector default 
constructor conditionally noexcept (depending on the allocator). Or to use 
explicit vector(const Allocator& = Allocator()); also in normal mode 
instead of splitting it in two.

I also removed a noexcept in profile mode where the release mode doesn't 
have noexcept.

Tested, no regression.

2013-09-15  Marc Glisse  <marc.glisse@inria.fr>

 	PR libstdc++/58338
 	* include/bits/stl_vector.h
 	(_Vector_impl::_Vector_impl(_Tp_alloc_type const&),
 	_Vector_impl::_Vector_impl(_Tp_alloc_type&&),
 	_Vector_impl::_M_swap_data,
 	_Vector_base::_Vector_base(const allocator_type&),
 	_Vector_base::_Vector_base(allocator_type&&),
 	_Vector_base::_Vector_base(_Vector_base&&),
 	vector::vector(const allocator_type&), vector::operator[],
 	vector::operator[] const, vector::front, vector::front const,
 	vector::back, vector::back const, vector::pop_back,
 	vector::_M_erase_at_end): Mark as noexcept.
 	(vector::~vector): Remove useless noexcept.
 	* include/debug/vector (vector::vector()): Split.
 	(vector::vector(const _Allocator&), vector::operator[],
 	vector::operator[] const, vector::front, vector::front const,
 	vector::back, vector::back const, vector::pop_back,
 	_M_requires_reallocation, _M_update_guaranteed_capacity,
 	_M_invalidate_after_nth): Mark as noexcept.
 	(vector::~vector): Remove useless noexcept.
 	* include/profile/vector (vector::vector()): Split.
 	(vector::vector(const _Allocator&), vector::operator[],
 	vector::operator[] const, vector::front, vector::front const,
 	vector::back, vector::back const): Mark as noexcept.
 	(vector::vector(vector&&, const _Allocator&)): Remove wrong noexcept.
 	(vector::~vector): Remove useless noexcept.

Comments

Paolo Carlini Sept. 16, 2013, 9:27 a.m. UTC | #1
Hi Marc,

On 09/15/2013 11:12 AM, Marc Glisse wrote:
> I had to separate the constructor that takes an allocator from the 
> default constructor in debug/profile, since in release the noexcept 
> only applies to one of them (and the testsuite asserts that release 
> and debug agree on this). An alternative would be to make the release 
> vector default constructor conditionally noexcept (depending on the 
> allocator). Or to use explicit vector(const Allocator& = Allocator()); 
> also in normal mode instead of splitting it in two.
Thanks a lot. Now I'm wondering if we shouldn't really do the latter: 
the issue is, if I remember correctly, in C++11, at variance with C++98, 
allocators aren't necessarily default constructible, thus by explicit 
instantiatiation the user can easily tell whether that constructor is 
split or not. What do you think?

Paolo.
Marc Glisse Sept. 16, 2013, 11:13 a.m. UTC | #2
On Mon, 16 Sep 2013, Paolo Carlini wrote:

> On 09/15/2013 11:12 AM, Marc Glisse wrote:
>> I had to separate the constructor that takes an allocator from the default 
>> constructor in debug/profile, since in release the noexcept only applies to 
>> one of them (and the testsuite asserts that release and debug agree on 
>> this). An alternative would be to make the release vector default 
>> constructor conditionally noexcept (depending on the allocator). Or to use 
>> explicit vector(const Allocator& = Allocator()); also in normal mode 
>> instead of splitting it in two.
> Thanks a lot. Now I'm wondering if we shouldn't really do the latter: the 
> issue is, if I remember correctly, in C++11, at variance with C++98, 
> allocators aren't necessarily default constructible, thus by explicit 
> instantiatiation the user can easily tell whether that constructor is split 
> or not. What do you think?

Shouldn't it just be illegal to explicitly instantiate a full class like 
std::vector?

But ok, I'll post that version as soon as I can test it.
Jonathan Wakely Sept. 16, 2013, 12:20 p.m. UTC | #3
On 15 September 2013 10:12, Marc Glisse wrote:
>
>         PR libstdc++/58338
>         * include/bits/stl_vector.h
>         (_Vector_impl::_Vector_impl(_Tp_alloc_type const&),
>         _Vector_impl::_Vector_impl(_Tp_alloc_type&&),
>         _Vector_impl::_M_swap_data,
>         _Vector_base::_Vector_base(const allocator_type&),
>         _Vector_base::_Vector_base(allocator_type&&),
>         _Vector_base::_Vector_base(_Vector_base&&),
>         vector::vector(const allocator_type&), vector::operator[],
>         vector::operator[] const, vector::front, vector::front const,
>         vector::back, vector::back const, vector::pop_back,
>         vector::_M_erase_at_end): Mark as noexcept.
>         (vector::~vector): Remove useless noexcept.

Are you sure the noexcept on the destructors is useless?

A user-defined allocator type could have
is_nothrow_descructible<allocator_type>::value==false or
is_nothrow_destructible<pointer>::value==false, which would alter the
type of is_nothrow_destructible<vector<T, A>>.

Although the user-defined allocator must not actually throw from its
destructor, it is not required to have a noexcept destructor.
However, since it must not actually throw (irrespective of its
exception spec) we can unconditionally give std::vector a noexcept
destructor.
Marc Glisse Sept. 16, 2013, 12:28 p.m. UTC | #4
On Mon, 16 Sep 2013, Jonathan Wakely wrote:

> Are you sure the noexcept on the destructors is useless?

Ok, I am putting it back in.
diff mbox

Patch

Index: include/bits/stl_vector.h
===================================================================
--- include/bits/stl_vector.h	(revision 202588)
+++ include/bits/stl_vector.h	(working copy)
@@ -80,32 +80,32 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       : public _Tp_alloc_type
       {
 	pointer _M_start;
 	pointer _M_finish;
 	pointer _M_end_of_storage;
 
 	_Vector_impl()
 	: _Tp_alloc_type(), _M_start(0), _M_finish(0), _M_end_of_storage(0)
 	{ }
 
-	_Vector_impl(_Tp_alloc_type const& __a)
+	_Vector_impl(_Tp_alloc_type const& __a) _GLIBCXX_NOEXCEPT
 	: _Tp_alloc_type(__a), _M_start(0), _M_finish(0), _M_end_of_storage(0)
 	{ }
 
 #if __cplusplus >= 201103L
-	_Vector_impl(_Tp_alloc_type&& __a)
+	_Vector_impl(_Tp_alloc_type&& __a) noexcept
 	: _Tp_alloc_type(std::move(__a)),
 	  _M_start(0), _M_finish(0), _M_end_of_storage(0)
 	{ }
 #endif
 
-	void _M_swap_data(_Vector_impl& __x)
+	void _M_swap_data(_Vector_impl& __x) _GLIBCXX_NOEXCEPT
 	{
 	  std::swap(_M_start, __x._M_start);
 	  std::swap(_M_finish, __x._M_finish);
 	  std::swap(_M_end_of_storage, __x._M_end_of_storage);
 	}
       };
       
     public:
       typedef _Alloc allocator_type;
 
@@ -117,36 +117,36 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       _M_get_Tp_allocator() const _GLIBCXX_NOEXCEPT
       { return *static_cast<const _Tp_alloc_type*>(&this->_M_impl); }
 
       allocator_type
       get_allocator() const _GLIBCXX_NOEXCEPT
       { return allocator_type(_M_get_Tp_allocator()); }
 
       _Vector_base()
       : _M_impl() { }
 
-      _Vector_base(const allocator_type& __a)
+      _Vector_base(const allocator_type& __a) _GLIBCXX_NOEXCEPT
       : _M_impl(__a) { }
 
       _Vector_base(size_t __n)
       : _M_impl()
       { _M_create_storage(__n); }
 
       _Vector_base(size_t __n, const allocator_type& __a)
       : _M_impl(__a)
       { _M_create_storage(__n); }
 
 #if __cplusplus >= 201103L
-      _Vector_base(_Tp_alloc_type&& __a)
+      _Vector_base(_Tp_alloc_type&& __a) noexcept
       : _M_impl(std::move(__a)) { }
 
-      _Vector_base(_Vector_base&& __x)
+      _Vector_base(_Vector_base&& __x) noexcept
       : _M_impl(std::move(__x._M_get_Tp_allocator()))
       { this->_M_impl._M_swap_data(__x._M_impl); }
 
       _Vector_base(_Vector_base&& __x, const allocator_type& __a)
       : _M_impl(__a)
       {
 	if (__x.get_allocator() == __a)
 	  this->_M_impl._M_swap_data(__x._M_impl);
 	else
 	  {
@@ -246,21 +246,21 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        *  @brief  Default constructor creates no elements.
        */
       vector()
       : _Base() { }
 
       /**
        *  @brief  Creates a %vector with no elements.
        *  @param  __a  An allocator object.
        */
       explicit
-      vector(const allocator_type& __a)
+      vector(const allocator_type& __a) _GLIBCXX_NOEXCEPT
       : _Base(__a) { }
 
 #if __cplusplus >= 201103L
       /**
        *  @brief  Creates a %vector with default constructed elements.
        *  @param  __n  The number of elements to initially create.
        *  @param  __a  An allocator.
        *
        *  This constructor fills the %vector with @a __n default
        *  constructed elements.
@@ -404,21 +404,21 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  _M_initialize_dispatch(__first, __last, _Integral());
 	}
 #endif
 
       /**
        *  The dtor only erases the elements, and note that if the
        *  elements themselves are pointers, the pointed-to memory is
        *  not touched in any way.  Managing the pointer is the user's
        *  responsibility.
        */
-      ~vector() _GLIBCXX_NOEXCEPT
+      ~vector()
       { std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish,
 		      _M_get_Tp_allocator()); }
 
       /**
        *  @brief  %Vector assignment operator.
        *  @param  __x  A %vector of identical element and allocator types.
        *
        *  All the elements of @a __x are copied, but any extra memory in
        *  @a __x (for fast expansion) will not be copied.  Unlike the
        *  copy constructor, the allocator object is not copied.
@@ -760,36 +760,36 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
        *  @param __n The index of the element for which data should be
        *  accessed.
        *  @return  Read/write reference to data.
        *
        *  This operator allows for easy, array-style, data access.
        *  Note that data access with this operator is unchecked and
        *  out_of_range lookups are not defined. (For checked lookups
        *  see at().)
        */
       reference
-      operator[](size_type __n)
+      operator[](size_type __n) _GLIBCXX_NOEXCEPT
       { return *(this->_M_impl._M_start + __n); }
 
       /**
        *  @brief  Subscript access to the data contained in the %vector.
        *  @param __n The index of the element for which data should be
        *  accessed.
        *  @return  Read-only (constant) reference to data.
        *
        *  This operator allows for easy, array-style, data access.
        *  Note that data access with this operator is unchecked and
        *  out_of_range lookups are not defined. (For checked lookups
        *  see at().)
        */
       const_reference
-      operator[](size_type __n) const
+      operator[](size_type __n) const _GLIBCXX_NOEXCEPT
       { return *(this->_M_impl._M_start + __n); }
 
     protected:
       /// Safety check used only from at().
       void
       _M_range_check(size_type __n) const
       {
 	if (__n >= this->size())
 	  __throw_out_of_range(__N("vector::_M_range_check"));
       }
@@ -829,45 +829,45 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       {
 	_M_range_check(__n);
 	return (*this)[__n];
       }
 
       /**
        *  Returns a read/write reference to the data at the first
        *  element of the %vector.
        */
       reference
-      front()
+      front() _GLIBCXX_NOEXCEPT
       { return *begin(); }
 
       /**
        *  Returns a read-only (constant) reference to the data at the first
        *  element of the %vector.
        */
       const_reference
-      front() const
+      front() const _GLIBCXX_NOEXCEPT
       { return *begin(); }
 
       /**
        *  Returns a read/write reference to the data at the last
        *  element of the %vector.
        */
       reference
-      back()
+      back() _GLIBCXX_NOEXCEPT
       { return *(end() - 1); }
       
       /**
        *  Returns a read-only (constant) reference to the data at the
        *  last element of the %vector.
        */
       const_reference
-      back() const
+      back() const _GLIBCXX_NOEXCEPT
       { return *(end() - 1); }
 
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // DR 464. Suggestion for new member functions in standard containers.
       // data access
       /**
        *   Returns a pointer such that [data(), data() + size()) is a valid
        *   range.  For a non-empty %vector, data() == &front().
        */
 #if __cplusplus >= 201103L
@@ -927,21 +927,21 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       /**
        *  @brief  Removes last element.
        *
        *  This is a typical stack operation. It shrinks the %vector by one.
        *
        *  Note that no data is returned, and if the last element's
        *  data is needed, it should be retrieved before pop_back() is
        *  called.
        */
       void
-      pop_back()
+      pop_back() _GLIBCXX_NOEXCEPT
       {
 	--this->_M_impl._M_finish;
 	_Alloc_traits::destroy(this->_M_impl, this->_M_impl._M_finish);
       }
 
 #if __cplusplus >= 201103L
       /**
        *  @brief  Inserts an object in %vector before specified iterator.
        *  @param  __position  A const_iterator into the %vector.
        *  @param  __args  Arguments.
@@ -1408,21 +1408,21 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
 	const size_type __len = size() + std::max(size(), __n);
 	return (__len < size() || __len > max_size()) ? max_size() : __len;
       }
 
       // Internal erase functions follow.
 
       // Called by erase(q1,q2), clear(), resize(), _M_fill_assign,
       // _M_assign_aux.
       void
-      _M_erase_at_end(pointer __pos)
+      _M_erase_at_end(pointer __pos) _GLIBCXX_NOEXCEPT
       {
 	std::_Destroy(__pos, this->_M_impl._M_finish, _M_get_Tp_allocator());
 	this->_M_impl._M_finish = __pos;
       }
 
       iterator
       _M_erase(iterator __position);
 
       iterator
       _M_erase(iterator __first, iterator __last);
Index: include/debug/vector
===================================================================
--- include/debug/vector	(revision 202588)
+++ include/debug/vector	(working copy)
@@ -68,22 +68,25 @@  namespace __debug
       typedef typename _Base::difference_type       difference_type;
 
       typedef _Tp				    value_type;
       typedef _Allocator			    allocator_type;
       typedef typename _Base::pointer               pointer;
       typedef typename _Base::const_pointer         const_pointer;
       typedef std::reverse_iterator<iterator>       reverse_iterator;
       typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
 
       // 23.2.4.1 construct/copy/destroy:
+      vector()
+      : _Base(), _M_guaranteed_capacity(0) { }
+
       explicit
-      vector(const _Allocator& __a = _Allocator())
+      vector(const _Allocator& __a) _GLIBCXX_NOEXCEPT
       : _Base(__a), _M_guaranteed_capacity(0) { }
 
 #if __cplusplus >= 201103L
       explicit
       vector(size_type __n, const _Allocator& __a = _Allocator())
       : _Base(__n, __a), _M_guaranteed_capacity(__n) { }
 
       vector(size_type __n, const _Tp& __value,
 	     const _Allocator& __a = _Allocator())
       : _Base(__n, __value, __a), _M_guaranteed_capacity(__n) { }
@@ -134,21 +137,21 @@  namespace __debug
 	__x._M_invalidate_all();
 	__x._M_guaranteed_capacity = 0;
       }
 
       vector(initializer_list<value_type> __l,
 	     const allocator_type& __a = allocator_type())
       : _Base(__l, __a),
 	_M_guaranteed_capacity(__l.size()) { }
 #endif
 
-      ~vector() _GLIBCXX_NOEXCEPT { }
+      ~vector() { }
 
       vector&
       operator=(const vector& __x)
       {
 	static_cast<_Base&>(*this) = __x;
 	this->_M_invalidate_all();
 	_M_update_guaranteed_capacity();
 	return *this;
       }
 
@@ -334,58 +337,58 @@  namespace __debug
 	bool __realloc = _M_requires_reallocation(__n);
 	_Base::reserve(__n);
 	if (__n > _M_guaranteed_capacity)
 	  _M_guaranteed_capacity = __n;
 	if (__realloc)
 	  this->_M_invalidate_all();
       }
 
       // element access:
       reference
-      operator[](size_type __n)
+      operator[](size_type __n) _GLIBCXX_NOEXCEPT
       {
 	__glibcxx_check_subscript(__n);
 	return _M_base()[__n];
       }
 
       const_reference
-      operator[](size_type __n) const
+      operator[](size_type __n) const _GLIBCXX_NOEXCEPT
       {
 	__glibcxx_check_subscript(__n);
 	return _M_base()[__n];
       }
 
       using _Base::at;
 
       reference
-      front()
+      front() _GLIBCXX_NOEXCEPT
       {
 	__glibcxx_check_nonempty();
 	return _Base::front();
       }
 
       const_reference
-      front() const
+      front() const _GLIBCXX_NOEXCEPT
       {
 	__glibcxx_check_nonempty();
 	return _Base::front();
       }
 
       reference
-      back()
+      back() _GLIBCXX_NOEXCEPT
       {
 	__glibcxx_check_nonempty();
 	return _Base::back();
       }
 
       const_reference
-      back() const
+      back() const _GLIBCXX_NOEXCEPT
       {
 	__glibcxx_check_nonempty();
 	return _Base::back();
       }
 
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // DR 464. Suggestion for new member functions in standard containers.
       using _Base::data;
 
       // 23.2.4.3 modifiers:
@@ -412,21 +415,21 @@  namespace __debug
 	{
 	  bool __realloc = _M_requires_reallocation(this->size() + 1);
 	  _Base::emplace_back(std::forward<_Args>(__args)...);
 	  if (__realloc)
 	    this->_M_invalidate_all();
 	  _M_update_guaranteed_capacity();
 	}
 #endif
 
       void
-      pop_back()
+      pop_back() _GLIBCXX_NOEXCEPT
       {
 	__glibcxx_check_nonempty();
 	this->_M_invalidate_if(_Equal(--_Base::end()));
 	_Base::pop_back();
       }
 
 #if __cplusplus >= 201103L
       template<typename... _Args>
         iterator
         emplace(const_iterator __position, _Args&&... __args)
@@ -623,32 +626,32 @@  namespace __debug
       _Base&
       _M_base() _GLIBCXX_NOEXCEPT { return *this; }
 
       const _Base&
       _M_base() const _GLIBCXX_NOEXCEPT { return *this; }
 
     private:
       size_type _M_guaranteed_capacity;
 
       bool
-      _M_requires_reallocation(size_type __elements)
+      _M_requires_reallocation(size_type __elements) _GLIBCXX_NOEXCEPT
       { return __elements > this->capacity(); }
 
       void
-      _M_update_guaranteed_capacity()
+      _M_update_guaranteed_capacity() _GLIBCXX_NOEXCEPT
       {
 	if (this->size() > _M_guaranteed_capacity)
 	  _M_guaranteed_capacity = this->size();
       }
 
       void
-      _M_invalidate_after_nth(difference_type __n)
+      _M_invalidate_after_nth(difference_type __n) _GLIBCXX_NOEXCEPT
       {
 	typedef __gnu_debug::_After_nth_from<_Base_const_iterator> _After_nth;
 	this->_M_invalidate_if(_After_nth(__n, _Base::begin()));
       }
     };
 
   template<typename _Tp, typename _Alloc>
     inline bool
     operator==(const vector<_Tp, _Alloc>& __lhs,
 	       const vector<_Tp, _Alloc>& __rhs)
Index: include/profile/vector
===================================================================
--- include/profile/vector	(revision 202588)
+++ include/profile/vector	(working copy)
@@ -70,22 +70,29 @@  namespace __profile
       typedef std::reverse_iterator<iterator>       reverse_iterator;
       typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
       
       _Base&
       _M_base() _GLIBCXX_NOEXCEPT { return *this; }
 
       const _Base&
       _M_base() const _GLIBCXX_NOEXCEPT { return *this; }
 
       // 23.2.4.1 construct/copy/destroy:
+      vector()
+      : _Base()
+      {
+        __profcxx_vector_construct(this, this->capacity());
+        __profcxx_vector_construct2(this);
+      }
+
       explicit
-      vector(const _Allocator& __a = _Allocator())
+      vector(const _Allocator& __a) _GLIBCXX_NOEXCEPT
       : _Base(__a)
       {
         __profcxx_vector_construct(this, this->capacity());
         __profcxx_vector_construct2(this);
       }
 
 #if __cplusplus >= 201103L
       explicit
       vector(size_type __n, const _Allocator& __a = _Allocator())
       : _Base(__n, __a)
@@ -149,33 +156,33 @@  namespace __profile
         __profcxx_vector_construct2(this);
       }
 
       vector(const _Base& __x, const _Allocator& __a)
       : _Base(__x, __a)
       { 
         __profcxx_vector_construct(this, this->capacity());
         __profcxx_vector_construct2(this);
       }
 
-      vector(vector&& __x, const _Allocator& __a) noexcept
+      vector(vector&& __x, const _Allocator& __a)
       : _Base(std::move(__x), __a)
       {
         __profcxx_vector_construct(this, this->capacity());
         __profcxx_vector_construct2(this);
       }
 
       vector(initializer_list<value_type> __l,
 	     const allocator_type& __a = allocator_type())
       : _Base(__l, __a) { }
 #endif
 
-      ~vector() _GLIBCXX_NOEXCEPT
+      ~vector()
       {
         __profcxx_vector_destruct(this, this->capacity(), this->size());
         __profcxx_vector_destruct2(this);
       }
 
       vector&
       operator=(const vector& __x)
       {
         static_cast<_Base&>(*this) = __x;
         return *this;
@@ -285,54 +292,54 @@  namespace __profile
 #endif
 
 #if __cplusplus >= 201103L
       using _Base::shrink_to_fit;
 #endif
 
       using _Base::empty;
 
       // element access:
       reference
-      operator[](size_type __n)
+      operator[](size_type __n) _GLIBCXX_NOEXCEPT
       {
         __profcxx_vector_invalid_operator(this);
         return _M_base()[__n];
       }
       const_reference
-      operator[](size_type __n) const
+      operator[](size_type __n) const _GLIBCXX_NOEXCEPT
       {
         __profcxx_vector_invalid_operator(this);
         return _M_base()[__n];
       }
 
       using _Base::at;
 
       reference
-      front()
+      front() _GLIBCXX_NOEXCEPT
       { 
         return _Base::front();
       }
 
       const_reference
-      front() const
+      front() const _GLIBCXX_NOEXCEPT
       {
 	return _Base::front();
       }
 
       reference
-      back()
+      back() _GLIBCXX_NOEXCEPT
       {
 	return _Base::back();
       }
 
       const_reference
-      back() const
+      back() const _GLIBCXX_NOEXCEPT
       {
 	return _Base::back();
       }
 
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // DR 464. Suggestion for new member functions in standard containers.
       using _Base::data;
 
       // 23.2.4.3 modifiers:
       void