diff mbox

exception safety validation

Message ID 509521C5.1030809@gmail.com
State New
Headers show

Commit Message

François Dumont Nov. 3, 2012, 1:53 p.m. UTC 
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

Comments

Jonathan Wakely Nov. 3, 2012, 2:11 p.m. UTC | #1 
On 3 November 2012 13:53, François Dumont wrote:
> Here is an updated proposal using default implementations as much as
> possible.

Please check the indentation of the "struct emplace_point" specializations.

In testsuite/util/testsuite_container_traits.h

+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+      typedef std::true_type    has_emplace;
+#endif

true_type isn't even defined unless it's C++11
Jonathan Wakely Nov. 3, 2012, 2:12 p.m. UTC | #2 
On 3 November 2012 14:11, Jonathan Wakely wrote:
> On 3 November 2012 13:53, François Dumont wrote:
>> Here is an updated proposal using default implementations as much as
>> possible.
>
> Please check the indentation of the "struct emplace_point" specializations.
>
> In testsuite/util/testsuite_container_traits.h
>
> +#ifdef __GXX_EXPERIMENTAL_CXX0X__
> +      typedef std::true_type    has_emplace;
> +#endif
>
> true_type isn't even defined unless it's C++11

My point being that the rest of the file already uses C++11-only features
diff mbox

Patch

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>