This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: exception safety validation


On 09/28/2012 10:14 PM, François Dumont wrote:
Hi

I enhance scope of the small exception safety test framework used to validate different levels of exception safety through the containers operations. I have added calls to C++ methods emplace/emplace_front/emplace_back/emplace_hint in C++11.

Doing so I start having propagation_consistency test for vector to fail. I finally realized that the problem was that the throw_value_limit used for this test had no move semantic. So the copy constructor and assignment operator was called sometimes throwing exceptions. In this case the Standard says that there is no strong exception safety guaranty explaining why the test was failing.

To fix it I have added for the moment move semantic to the throw_value_base type so that it never throws. I had to remove the dg-do run { xfail } in the equivalent deque test that was surely failing for the same reason.

Now I wonder if there is something in the Standard saying that move construct/assign shall never throw ? At least, on vector modifiers it is clearly said that if the move constructor throw it can have undefined behavior. If move construct/assign can throw then we might consider a way to control wether or not it will throw depending on the test. When testing vector and deque it shall not throw, when testing node based container it can throw. What do you think ?

I also wonder what are all the try { ... } catch (const __gnu_cxx::forced_error&) { throw; } in safety.h for ? Is it to influence the exception callstack ?


François




Any news ?


Here is an updated proposal using default implementations as much as possible.

François

Index: include/ext/throw_allocator.h
===================================================================
--- include/ext/throw_allocator.h	(revision 193118)
+++ include/ext/throw_allocator.h	(working copy)
@@ -467,6 +467,11 @@
       throw_value_base(const throw_value_base& __v) : _M_i(__v._M_i)
       { throw_conditionally(); }
 
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+      // Shall not throw.
+      throw_value_base(throw_value_base&&) = default;
+#endif
+
       explicit throw_value_base(const std::size_t __i) : _M_i(__i)
       { throw_conditionally(); }
 #endif
@@ -479,7 +484,13 @@
 	return *this;
       }
 
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+      // Shall not throw.
       throw_value_base&
+      operator=(throw_value_base&&) = default;
+#endif
+
+      throw_value_base&
       operator++()
       {
 	throw_conditionally();
@@ -568,8 +579,24 @@
     throw_value_limit(const throw_value_limit& __other)
     : base_type(__other._M_i) { }
 
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+    throw_value_limit(throw_value_limit&&) = default;
+#endif
+
     explicit throw_value_limit(const std::size_t __i) : base_type(__i) { }
 #endif
+
+    throw_value_limit&
+    operator=(const throw_value_limit& __other)
+    {
+      base_type::operator=(__other);
+      return *this;
+    }
+
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+    throw_value_limit&
+    operator=(throw_value_limit&&) = default;
+#endif
   };
 
   /// Type throwing via random condition.
@@ -583,9 +610,24 @@
     throw_value_random(const throw_value_random& __other)
     : base_type(__other._M_i) { }
 
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+    throw_value_random(throw_value_random&&) = default;
+#endif
 
     explicit throw_value_random(const std::size_t __i) : base_type(__i) { }
 #endif
+
+    throw_value_random&
+    operator=(const throw_value_random& __other)
+    {
+      base_type::operator=(__other);
+      return *this;
+    }
+
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+    throw_value_random&
+    operator=(throw_value_random&&) = default;
+#endif
   };
 
 
Index: testsuite/util/exception/safety.h
===================================================================
--- testsuite/util/exception/safety.h	(revision 193118)
+++ testsuite/util/exception/safety.h	(working copy)
@@ -226,17 +226,22 @@
 	// compared to the control container.
 	// NB: Should be equivalent to __test != __control, but
 	// computed without equivalence operators
-	const size_type szt = std::distance(__test.begin(), __test.end());
-	const size_type szc = std::distance(__control.begin(),
-					    __control.end());
-	bool __equal_size = szt == szc;
+	const size_type szt
+	  = std::distance(__test.begin(), __test.end());
+	const size_type szc
+	  = std::distance(__control.begin(), __control.end());
 
+	if (szt != szc)
+	  throw std::logic_error(
+		"setup_base::compare containers size not equal");
+
 	// Should test iterator validity before and after exception.
 	bool __equal_it = std::equal(__test.begin(), __test.end(),
 				     __control.begin());
 
-	if (!__equal_size || !__equal_it)
-	  throw std::logic_error("setup_base::compare containers not equal");
+	if (!__equal_it)
+	  throw std::logic_error(
+		"setup_base::compare containers iterators not equal");
 
 	return true;
       }
@@ -627,7 +632,97 @@
 	operator()(_Tp&, _Tp&) { }
       };
 
+    template<typename _Tp, bool = traits<_Tp>::has_push_pop::value
+				  && traits<_Tp>::has_emplace::value>
+      struct emplace_front
+      {
+	typedef _Tp 					container_type;
+	typedef typename container_type::value_type    	value_type;
 
+	void
+	operator()(_Tp& __test)
+	{
+	  try
+	    {
+	      const value_type cv = generate_unique<value_type>();
+	      __test.emplace_front(cv);
+	    }
+	  catch(const __gnu_cxx::forced_error&)
+	    { throw; }
+	}
+
+	// Assumes containers start out equivalent.
+	void
+	operator()(_Tp& __control, _Tp& __test)
+	{
+	  try
+	    {
+	      const value_type cv = generate_unique<value_type>();
+	      __test.emplace_front(cv);
+	    }
+	  catch(const __gnu_cxx::forced_error&)
+	    { throw; }
+	}
+    };
+
+    // Specialization, empty.
+    template<typename _Tp>
+      struct emplace_front<_Tp, false>
+      {
+	void
+	operator()(_Tp&) { }
+
+	void
+	operator()(_Tp&, _Tp&) { }
+      };
+
+
+    template<typename _Tp, bool = traits<_Tp>::has_push_pop::value
+				  && traits<_Tp>::has_emplace::value 
+				  && traits<_Tp>::is_reversible::value>
+      struct emplace_back
+      {
+	typedef _Tp 					container_type;
+	typedef typename container_type::value_type    	value_type;
+
+	void
+	operator()(_Tp& __test)
+	{
+	  try
+	    {
+	      const value_type cv = generate_unique<value_type>();
+	      __test.emplace_back(cv);
+	    }
+	  catch(const __gnu_cxx::forced_error&)
+	    { throw; }
+	}
+
+	// Assumes containers start out equivalent.
+	void
+	operator()(_Tp& __control, _Tp& __test)
+	{
+	  try
+	    {
+	      const value_type cv = generate_unique<value_type>();
+	      __test.push_back(cv);
+	    }
+	  catch(const __gnu_cxx::forced_error&)
+	    { throw; }
+	}
+    };
+
+    // Specialization, empty.
+    template<typename _Tp>
+      struct emplace_back<_Tp, false>
+      {
+	void
+	operator()(_Tp&) { }
+
+	void
+	operator()(_Tp&, _Tp&) { }
+      };
+
+
     // Abstract the insert function into two parts:
     // 1, insert_base_functions == holds function pointer
     // 2, insert_base == links function pointer to class insert method
@@ -726,9 +821,8 @@
 	insert_base() : _F_insert_point(&container_type::insert_after) { }
       };
 
-    template<typename _Tp,
-	     bool = traits<_Tp>::has_insert::value,
-	     bool = traits<_Tp>::has_insert_after::value>
+    template<typename _Tp, bool = traits<_Tp>::has_insert::value,
+			   bool = traits<_Tp>::has_insert_after::value>
       struct insert_point;
 
     // Specialization for most containers.
@@ -826,11 +920,12 @@
 	operator()(_Tp&, _Tp&) { }
       };
 
-    template<typename _Tp,
-	     bool = traits<_Tp>::has_emplace::value>
+    template<typename _Tp, bool = traits<_Tp>::has_emplace::value
+				  && (traits<_Tp>::is_associative::value
+				      || traits<_Tp>::is_unordered::value)>
       struct emplace;
 
-    // Specialization for most containers.
+    // Specialization for associative and unordered containers.
     template<typename _Tp>
       struct emplace<_Tp, true>
       {
@@ -875,13 +970,15 @@
 	operator()(_Tp&, _Tp&) { }
       };
 
-    template<typename _Tp,
-	     bool = traits<_Tp>::has_emplace::value>
-      struct emplace_hint;
+    template<typename _Tp, bool = traits<_Tp>::has_emplace::value,
+			   bool = traits<_Tp>::is_associative::value
+				  || traits<_Tp>::is_unordered::value,
+			   bool = traits<_Tp>::has_insert_after::value>
+      struct emplace_point;
 
     // Specialization for most containers.
     template<typename _Tp>
-      struct emplace_hint<_Tp, true>
+    struct emplace_point<_Tp, true, false, false>
       {
 	typedef _Tp 				       	container_type;
 	typedef typename container_type::value_type 	value_type;
@@ -896,6 +993,47 @@
 	      size_type s = generate(sz);
 	      auto i = __test.begin();
 	      std::advance(i, s);
+	      __test.emplace(i, cv);
+	    }
+	  catch(const __gnu_cxx::forced_error&)
+	    { throw; }
+	}
+
+	// Assumes containers start out equivalent.
+	void
+	operator()(_Tp& __control, _Tp& __test)
+	{
+	  try
+	    {
+	      const value_type cv = generate_unique<value_type>();
+	      const size_type sz = std::distance(__test.begin(), __test.end());
+	      size_type s = generate(sz);
+	      auto i = __test.begin();
+	      std::advance(i, s);
+	      __test.emplace(i, cv);
+	    }
+	  catch(const __gnu_cxx::forced_error&)
+	    { throw; }
+ 	}
+      };
+
+    // Specialization for associative and unordered containers.
+    template<typename _Tp>
+    struct emplace_point<_Tp, true, true, false>
+      {
+	typedef _Tp 				       	container_type;
+	typedef typename container_type::value_type 	value_type;
+
+	void
+	operator()(_Tp& __test)
+	{
+	  try
+	    {
+	      const value_type cv = generate_unique<value_type>();
+	      const size_type sz = std::distance(__test.begin(), __test.end());
+	      size_type s = generate(sz);
+	      auto i = __test.begin();
+	      std::advance(i, s);
 	      __test.emplace_hint(i, cv);
 	    }
 	  catch(const __gnu_cxx::forced_error&)
@@ -920,11 +1058,54 @@
  	}
       };
 
-    // Specialization, empty.
+    // Specialization for forward_list.
     template<typename _Tp>
-      struct emplace_hint<_Tp, false>
+    struct emplace_point<_Tp, true, false, true>
       {
+	typedef _Tp 				       	container_type;
+	typedef typename container_type::value_type 	value_type;
+
 	void
+	operator()(_Tp& __test)
+	{
+	  try
+	    {
+	      const value_type cv = generate_unique<value_type>();
+	      const size_type sz = std::distance(__test.begin(), __test.end());
+	      size_type s = generate(sz);
+	      auto i = __test.before_begin();
+	      std::advance(i, s);
+	      __test.emplace_after(i, cv);
+	    }
+	  catch(const __gnu_cxx::forced_error&)
+	    { throw; }
+	}
+
+	// Assumes containers start out equivalent.
+	void
+	operator()(_Tp& __control, _Tp& __test)
+	{
+	  try
+	    {
+	      const value_type cv = generate_unique<value_type>();
+	      const size_type sz = std::distance(__test.begin(), __test.end());
+	      size_type s = generate(sz);
+	      auto i = __test.before_begin();
+	      std::advance(i, s);
+	      __test.emplace_after(i, cv);
+	    }
+	  catch(const __gnu_cxx::forced_error&)
+	    { throw; }
+ 	}
+      };
+
+    // Specialization, empty.
+    template<typename _Tp, bool is_associative_or_unordered,
+			   bool has_insert_after>
+    struct emplace_point<_Tp, false, is_associative_or_unordered,
+			 has_insert_after>
+      {
+	void
 	operator()(_Tp&) { }
 
 	void
@@ -1128,7 +1309,9 @@
       typedef erase_range<container_type> 	       	erase_range;
       typedef insert_point<container_type> 	       	insert_point;
       typedef emplace<container_type>			emplace;
-      typedef emplace_hint<container_type>		emplace_hint;
+      typedef emplace_point<container_type>		emplace_point;
+      typedef emplace_front<container_type>		emplace_front;
+      typedef emplace_back<container_type>		emplace_back;
       typedef pop_front<container_type> 	       	pop_front;
       typedef pop_back<container_type> 			pop_back;
       typedef push_front<container_type> 	       	push_front;
@@ -1146,7 +1329,9 @@
       erase_range		_M_eraser;
       insert_point		_M_insertp;
       emplace			_M_emplace;
-      emplace_hint		_M_emplaceh;
+      emplace_point		_M_emplacep;
+      emplace_front		_M_emplacef;
+      emplace_back		_M_emplaceb;
       pop_front			_M_popf;
       pop_back			_M_popb;
       push_front	       	_M_pushf;
@@ -1207,7 +1392,9 @@
 	_M_functions.push_back(function_type(base_type::_M_eraser));
 	_M_functions.push_back(function_type(base_type::_M_insertp));
 	_M_functions.push_back(function_type(base_type::_M_emplace));
-	_M_functions.push_back(function_type(base_type::_M_emplaceh));
+	_M_functions.push_back(function_type(base_type::_M_emplacep));
+	_M_functions.push_back(function_type(base_type::_M_emplacef));
+	_M_functions.push_back(function_type(base_type::_M_emplaceb));
 	_M_functions.push_back(function_type(base_type::_M_popf));
 	_M_functions.push_back(function_type(base_type::_M_popb));
 	_M_functions.push_back(function_type(base_type::_M_pushf));
@@ -1328,7 +1515,8 @@
   // Test strong exception guarantee.
   // Run through all member functions with a roll-back, consistent
   // coherent requirement.
-  // all: member functions insert of a single element, push_back, push_front
+  // all: member functions insert and emplace of a single element, push_back,
+  // push_front
   // unordered: rehash
   template<typename _Tp>
     struct propagation_consistent : public test_base<_Tp>
@@ -1360,9 +1548,12 @@
 
 	// Construct containers.
 	populate p(_M_container_control);
-	sync();
 
 	// Construct list of member functions to exercise.
+	_M_functions.push_back(function_type(base_type::_M_emplace));
+	_M_functions.push_back(function_type(base_type::_M_emplacep));
+	_M_functions.push_back(function_type(base_type::_M_emplacef));
+	_M_functions.push_back(function_type(base_type::_M_emplaceb));
 	_M_functions.push_back(function_type(base_type::_M_pushf));
 	_M_functions.push_back(function_type(base_type::_M_pushb));
 	_M_functions.push_back(function_type(base_type::_M_insertp));
Index: testsuite/util/testsuite_container_traits.h
===================================================================
--- testsuite/util/testsuite_container_traits.h	(revision 193118)
+++ testsuite/util/testsuite_container_traits.h	(working copy)
@@ -73,6 +73,9 @@
       typedef std::true_type	has_insert;
       typedef std::true_type	has_push_pop;
       typedef std::true_type	has_size_type_constructor;
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+      typedef std::true_type	has_emplace;
+#endif
     };
 
   template<typename _Tp1, typename _Tp2>
@@ -85,6 +88,9 @@
       typedef std::true_type	has_insert_after;
       typedef std::true_type	has_push_pop;
       typedef std::true_type	has_size_type_constructor;
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+      typedef std::true_type	has_emplace;
+#endif
     };
 
   template<typename _Tp1, typename _Tp2>
@@ -98,6 +104,9 @@
       typedef std::true_type	has_insert;
       typedef std::true_type	has_push_pop;
       typedef std::true_type	has_size_type_constructor;
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+      typedef std::true_type	has_emplace;
+#endif
     };
 
   template<typename _Tp1, typename _Tp2>
@@ -111,6 +120,9 @@
       typedef std::true_type	has_throwing_erase;
       typedef std::true_type	has_insert;
       typedef std::true_type	has_size_type_constructor;
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+      typedef std::true_type	has_emplace;
+#endif
     };
 
   template<typename _Tp1, typename _Tp2, typename _Tp3>
Index: testsuite/23_containers/deque/requirements/exception/propagation_consistent.cc
===================================================================
--- testsuite/23_containers/deque/requirements/exception/propagation_consistent.cc	(revision 193118)
+++ testsuite/23_containers/deque/requirements/exception/propagation_consistent.cc	(working copy)
@@ -1,6 +1,5 @@
 // { dg-options "-std=gnu++0x" }
 // { dg-require-cstdint "" }
-// { dg-do run { xfail *-*-* } }
 
 // 2009-09-09  Benjamin Kosnik  <benjamin@redhat.com>
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]