diff mbox

[v3] Use move_if_noexcept in std::vector

Message ID 4DEE4FA6.7040109@oracle.com
State New
Headers show

Commit Message

Paolo Carlini June 7, 2011, 4:19 p.m. UTC
Hi,

the below starts using it, for correctness wrt move constructors which 
can throw. For sure we are missing the optimization which uses the 
default-constructor + swap when the type isn't nothrow move 
constructible, per Note 5 in n3050, for example.

Tested x86_64-linux, committed.

Paolo.

/////////////////////
2011-06-07  Paolo Carlini  <paolo.carlini@oracle.com>

	* include/bits/move.h (struct __move_if_noexcept_cond): Add.
	(move_if_noexcept): Use the latter.
	* include/bits/stl_iterator.h (__make_move_if_noexcept_iterator,
	_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR): Add.
	* include/bits/stl_uninitialized.h
	(__uninitialized_move_if_noexcept_a): Add.
	* include/bits/vector.tcc (vector<>::reserve): Use
	_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR.
	(vector<>::_M_insert_aux, _M_fill_insert, _M_default_append,
	_M_range_insert): Use __uninitialized_move_if_noexcept_a.
	* testsuite/util/testsuite_rvalref.h (throwing_move_constructor):
	Add.
	(copycounter::copycounter(copycounter&&)): Use noexcept.
	* testsuite/23_containers/vector/modifiers/moveable2.cc: New.
	* testsuite/23_containers/vector/capacity/resize/moveable2.cc:
	Likewise.
	* testsuite/23_containers/vector/capacity/reserve/moveable2.cc:
	Likewise.
diff mbox

Patch

Index: include/bits/move.h
===================================================================
--- include/bits/move.h	(revision 174739)
+++ include/bits/move.h	(working copy)
@@ -82,6 +82,12 @@ 
     move(_Tp&& __t) noexcept
     { return static_cast<typename std::remove_reference<_Tp>::type&&>(__t); }
 
+
+  template<typename _Tp>
+    struct __move_if_noexcept_cond
+    : public __and_<__not_<is_nothrow_move_constructible<_Tp>>,
+                    is_copy_constructible<_Tp>>::type { };
+
   /**
    *  @brief Move unless it could throw and the type is copyable.
    *  @ingroup utilities
@@ -90,9 +96,7 @@ 
    */
   template<typename _Tp>
     inline typename
-    conditional<__and_<__not_<is_nothrow_move_constructible<_Tp>>,
-                       is_copy_constructible<_Tp>>::value,
-                const _Tp&, _Tp&&>::type
+    conditional<__move_if_noexcept_cond<_Tp>::value, const _Tp&, _Tp&&>::type
     move_if_noexcept(_Tp& __x) noexcept
     { return std::move(__x); }
 
Index: include/bits/stl_uninitialized.h
===================================================================
--- include/bits/stl_uninitialized.h	(revision 174739)
+++ include/bits/stl_uninitialized.h	(working copy)
@@ -269,6 +269,19 @@ 
 					 __result, __alloc);
     }
 
+  template<typename _InputIterator, typename _ForwardIterator,
+	   typename _Allocator>
+    inline _ForwardIterator
+    __uninitialized_move_if_noexcept_a(_InputIterator __first,
+				       _InputIterator __last,
+				       _ForwardIterator __result,
+				       _Allocator& __alloc)
+    {
+      return std::__uninitialized_copy_a
+	(_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__first),
+	 _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__last), __result, __alloc);
+    }
+
   template<typename _ForwardIterator, typename _Tp, typename _Allocator>
     void
     __uninitialized_fill_a(_ForwardIterator __first, _ForwardIterator __last,
Index: include/bits/stl_iterator.h
===================================================================
--- include/bits/stl_iterator.h	(revision 174739)
+++ include/bits/stl_iterator.h	(working copy)
@@ -1118,14 +1118,25 @@ 
     make_move_iterator(const _Iterator& __i)
     { return move_iterator<_Iterator>(__i); }
 
+  template<typename _Iterator, typename _ReturnType
+    = typename conditional<__move_if_noexcept_cond
+      <typename iterator_traits<_Iterator>::value_type>::value,
+                _Iterator, move_iterator<_Iterator>>::type>
+    inline _ReturnType
+    __make_move_if_noexcept_iterator(_Iterator __i)
+    { return _ReturnType(__i); }
+
   // @} group iterators
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 
 #define _GLIBCXX_MAKE_MOVE_ITERATOR(_Iter) std::make_move_iterator(_Iter)
+#define _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(_Iter) \
+  std::__make_move_if_noexcept_iterator(_Iter)
 #else
 #define _GLIBCXX_MAKE_MOVE_ITERATOR(_Iter) (_Iter)
+#define _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(_Iter) (_Iter)
 #endif // __GXX_EXPERIMENTAL_CXX0X__
 
 #endif
Index: include/bits/vector.tcc
===================================================================
--- include/bits/vector.tcc	(revision 174739)
+++ include/bits/vector.tcc	(working copy)
@@ -72,8 +72,8 @@ 
 	{
 	  const size_type __old_size = size();
 	  pointer __tmp = _M_allocate_and_copy(__n,
-		 _GLIBCXX_MAKE_MOVE_ITERATOR(this->_M_impl._M_start),
-		 _GLIBCXX_MAKE_MOVE_ITERATOR(this->_M_impl._M_finish));
+	    _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(this->_M_impl._M_start),
+	    _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(this->_M_impl._M_finish));
 	  std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish,
 			_M_get_Tp_allocator());
 	  _M_deallocate(this->_M_impl._M_start,
@@ -337,17 +337,17 @@ 
 #endif
 	      __new_finish = 0;
 
-	      __new_finish =
-		std::__uninitialized_move_a(this->_M_impl._M_start,
-					    __position.base(), __new_start,
-					    _M_get_Tp_allocator());
+	      __new_finish
+		= std::__uninitialized_move_if_noexcept_a
+		(this->_M_impl._M_start, __position.base(),
+		 __new_start, _M_get_Tp_allocator());
+
 	      ++__new_finish;
 
-	      __new_finish =
-		std::__uninitialized_move_a(__position.base(),
-					    this->_M_impl._M_finish,
-					    __new_finish,
-					    _M_get_Tp_allocator());
+	      __new_finish
+		= std::__uninitialized_move_if_noexcept_a
+		(__position.base(), this->_M_impl._M_finish,
+		 __new_finish, _M_get_Tp_allocator());
 	    }
           __catch(...)
 	    {
@@ -423,18 +423,17 @@ 
 						_M_get_Tp_allocator());
 		  __new_finish = 0;
 
-		  __new_finish =
-		    std::__uninitialized_move_a(this->_M_impl._M_start,
-						__position.base(),
-						__new_start,
-						_M_get_Tp_allocator());
+		  __new_finish
+		    = std::__uninitialized_move_if_noexcept_a
+		    (this->_M_impl._M_start, __position.base(),
+		     __new_start, _M_get_Tp_allocator());
+
 		  __new_finish += __n;
 
-		  __new_finish =
-		    std::__uninitialized_move_a(__position.base(),
-						this->_M_impl._M_finish,
-						__new_finish,
-						_M_get_Tp_allocator());
+		  __new_finish
+		    = std::__uninitialized_move_if_noexcept_a
+		    (__position.base(), this->_M_impl._M_finish,
+		     __new_finish, _M_get_Tp_allocator());
 		}
 	      __catch(...)
 		{
@@ -484,11 +483,10 @@ 
 	      pointer __new_finish(__new_start);
 	      __try
 		{
-		  __new_finish =
-		    std::__uninitialized_move_a(this->_M_impl._M_start,
-						this->_M_impl._M_finish,
-						__new_start,
-						_M_get_Tp_allocator());
+		  __new_finish
+		    = std::__uninitialized_move_if_noexcept_a
+		    (this->_M_impl._M_start, this->_M_impl._M_finish,
+		     __new_start, _M_get_Tp_allocator());
 		  std::__uninitialized_default_n_a(__new_finish, __n,
 						   _M_get_Tp_allocator());
 		  __new_finish += __n;
@@ -577,20 +575,18 @@ 
 		pointer __new_finish(__new_start);
 		__try
 		  {
-		    __new_finish =
-		      std::__uninitialized_move_a(this->_M_impl._M_start,
-						  __position.base(),
-						  __new_start,
-						  _M_get_Tp_allocator());
-		    __new_finish =
-		      std::__uninitialized_copy_a(__first, __last,
-						  __new_finish,
-						  _M_get_Tp_allocator());
-		    __new_finish =
-		      std::__uninitialized_move_a(__position.base(),
-						  this->_M_impl._M_finish,
-						  __new_finish,
-						  _M_get_Tp_allocator());
+		    __new_finish
+		      = std::__uninitialized_move_if_noexcept_a
+		      (this->_M_impl._M_start, __position.base(),
+		       __new_start, _M_get_Tp_allocator());
+		    __new_finish
+		      = std::__uninitialized_copy_a(__first, __last,
+						    __new_finish,
+						    _M_get_Tp_allocator());
+		    __new_finish
+		      = std::__uninitialized_move_if_noexcept_a
+		      (__position.base(), this->_M_impl._M_finish,
+		       __new_finish, _M_get_Tp_allocator());
 		  }
 		__catch(...)
 		  {
Index: testsuite/23_containers/vector/modifiers/moveable2.cc
===================================================================
--- testsuite/23_containers/vector/modifiers/moveable2.cc	(revision 0)
+++ testsuite/23_containers/vector/modifiers/moveable2.cc	(revision 0)
@@ -0,0 +1,77 @@ 
+// { dg-options "-std=gnu++0x" }
+
+// 2011-06-07  Paolo Carlini  <paolo.carlini@oracle.com>
+
+// Copyright (C) 2011 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/>.
+
+#include <vector>
+#include <testsuite_hooks.h>
+#include <testsuite_rvalref.h>
+
+void
+test01()
+{
+  bool test __attribute__((unused)) = true;
+  using namespace __gnu_test;
+
+  std::vector<throwing_move_constructor> v1;
+
+  throwing_move_constructor tmc;
+
+  v1.push_back(tmc);
+  VERIFY( v1.size() == 1 );
+
+  v1.push_back(tmc);
+  VERIFY( v1.size() == 2 );
+
+  v1.insert(v1.end(), tmc);
+  VERIFY( v1.size() == 3 );
+
+  v1.insert(v1.end(), 100, tmc);
+  VERIFY( v1.size() == 103 );
+
+  v1.insert(v1.end(), 10, tmc);
+  VERIFY( v1.size() == 113 );
+
+  v1.insert(v1.end(), 1, tmc);
+  VERIFY( v1.size() == 114 );
+
+  std::vector<throwing_move_constructor> v2;
+
+  throwing_move_constructor tmca[]
+    = { throwing_move_constructor(), throwing_move_constructor(),
+	throwing_move_constructor(), throwing_move_constructor() };
+
+  v2.insert(v2.end(), tmca, tmca + 1);
+  VERIFY( v2.size() == 1 );
+
+  v2.insert(v2.end(), tmca, tmca + 4);
+  VERIFY( v2.size() == 5 );
+
+  v2.insert(v2.end(), tmca, tmca + 2);
+  VERIFY( v2.size() == 7 );
+
+  v2.insert(v2.end(), tmca, tmca + 1);
+  VERIFY( v2.size() == 8 );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/vector/capacity/resize/moveable2.cc
===================================================================
--- testsuite/23_containers/vector/capacity/resize/moveable2.cc	(revision 0)
+++ testsuite/23_containers/vector/capacity/resize/moveable2.cc	(revision 0)
@@ -0,0 +1,57 @@ 
+// { dg-options "-std=gnu++0x" }
+
+// 2011-06-07  Paolo Carlini  <paolo.carlini@oracle.com>
+
+// Copyright (C) 2011 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/>.
+
+#include <vector>
+#include <testsuite_hooks.h>
+#include <testsuite_rvalref.h>
+
+void
+test01()
+{
+  bool test __attribute__((unused)) = true;
+  using namespace __gnu_test;
+
+  std::vector<throwing_move_constructor> v(5);
+
+  v.resize(50);
+  VERIFY( v.size() == 50 );
+
+  v.reserve(200);
+  VERIFY( v.capacity() >= 200 );
+
+  v.resize(100);
+  VERIFY( v.size() == 100 );
+
+  v.resize(500, throwing_move_constructor());
+  VERIFY( v.size() == 500 );
+
+  v.reserve(2000);
+  VERIFY( v.capacity() >= 2000 );
+
+  v.resize(1000, throwing_move_constructor());
+  VERIFY( v.size() == 1000 );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/23_containers/vector/capacity/reserve/moveable2.cc
===================================================================
--- testsuite/23_containers/vector/capacity/reserve/moveable2.cc	(revision 0)
+++ testsuite/23_containers/vector/capacity/reserve/moveable2.cc	(revision 0)
@@ -0,0 +1,45 @@ 
+// { dg-options "-std=gnu++0x" }
+
+// 2011-06-07  Paolo Carlini  <paolo.carlini@oracle.com>
+
+// Copyright (C) 2011 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/>.
+
+#include <vector>
+#include <testsuite_hooks.h>
+#include <testsuite_rvalref.h>
+
+void
+test01()
+{
+  bool test __attribute__((unused)) = true;
+  using namespace __gnu_test;
+
+  std::vector<throwing_move_constructor> v(5);
+
+  v.reserve(50);
+  VERIFY( v.capacity() >= 50 );
+
+  v.reserve(500);
+  VERIFY( v.capacity() >= 500 );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/util/testsuite_rvalref.h
===================================================================
--- testsuite/util/testsuite_rvalref.h	(revision 174735)
+++ testsuite/util/testsuite_rvalref.h	(working copy)
@@ -117,7 +117,7 @@ 
       ++copycount;
     }
 
-    copycounter(copycounter&& in)
+    copycounter(copycounter&& in) noexcept
     {
       bool test __attribute__((unused)) = true;
       VERIFY( in.valid == true );
@@ -156,7 +156,7 @@ 
       return *this;
     }
     
-    ~copycounter()
+    ~copycounter() noexcept
     { valid = false; }
   };
 
@@ -246,6 +246,16 @@ 
     return lh.val < rh.val;
   }
 
+  struct throwing_move_constructor
+  {
+    throwing_move_constructor() = default;
+
+    throwing_move_constructor(throwing_move_constructor&&)
+    { throw 1; }
+
+    throwing_move_constructor(const throwing_move_constructor&) = default;
+  };
+
 } // namespace __gnu_test
 
 namespace std