diff mbox

exception safety validation

Message ID 50952DAC.7070909@gmail.com
State New
Headers show

Commit Message

François Dumont Nov. 3, 2012, 2:43 p.m. UTC 
On 11/03/2012 03:12 PM, Jonathan Wakely wrote:
> 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
>
Ok, it looks like this file is only used in C++11 mode so here is the 
simplified patch.

Note that I run exception tests to validate it and the recently 
introduced 23_containers/forward_list/allocator/noexcept.cc test doesn't 
work in debug mode. I think it needs a dg-require-normal-node attribute.

François

Comments

Paolo Carlini Nov. 3, 2012, 7:05 p.m. UTC | #1 
Hi,

On 11/03/2012 03:43 PM, François Dumont wrote:
> On 11/03/2012 03:12 PM, Jonathan Wakely wrote:
>> 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
>>
> Ok, it looks like this file is only used in C++11 mode so here is the 
> simplified patch.
Finally I found the time (sorry!) to have a look to the patch and the 
latest version looks good to me...
> Note that I run exception tests to validate it and the recently 
> introduced 23_containers/forward_list/allocator/noexcept.cc test 
> doesn't work in debug mode. I think it needs a dg-require-normal-node 
> attribute.
... if you reach an agreement about the right thing to do concerning 
this failure, I think the patch could otherwise go in.

Thanks!
Paolo.
Jonathan Wakely Nov. 4, 2012, 4:01 p.m. UTC | #2 
On 3 November 2012 19:05, Paolo Carlini wrote:
>
> On 11/03/2012 03:43 PM, François Dumont wrote:
>>
>> Note that I run exception tests to validate it and the recently introduced
>> 23_containers/forward_list/allocator/noexcept.cc test doesn't work in debug
>> mode. I think it needs a dg-require-normal-node attribute.
>
> ... if you reach an agreement about the right thing to do concerning this
> failure, I think the patch could otherwise go in.

Yes, the patch looks good, please commit it and  I'll look into the
forward_list failure, since my changes caused it.

Thanks.
Jonathan Wakely Nov. 4, 2012, 4:04 p.m. UTC | #3 
Ooh, I almost forgot, please remember to update the copyright years in
the changed files.  For trunk you can change them to ranges like
2005-2012

On 4 November 2012 16:01, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> On 3 November 2012 19:05, Paolo Carlini wrote:
>>
>> On 11/03/2012 03:43 PM, François Dumont wrote:
>>>
>>> Note that I run exception tests to validate it and the recently introduced
>>> 23_containers/forward_list/allocator/noexcept.cc test doesn't work in debug
>>> mode. I think it needs a dg-require-normal-node attribute.
>>
>> ... if you reach an agreement about the right thing to do concerning this
>> failure, I think the patch could otherwise go in.
>
> Yes, the patch looks good, please commit it and  I'll look into the
> forward_list failure, since my changes caused it.
>
> Thanks.
Jonathan Wakely Nov. 5, 2012, 10:31 a.m. UTC | #4 
On 4 November 2012 16:01, Jonathan Wakely wrote:
> On 3 November 2012 19:05, Paolo Carlini wrote:
>>
>> On 11/03/2012 03:43 PM, François Dumont wrote:
>>>
>>> Note that I run exception tests to validate it and the recently introduced
>>> 23_containers/forward_list/allocator/noexcept.cc test doesn't work in debug
>>> mode. I think it needs a dg-require-normal-node attribute.
>>
>> ... if you reach an agreement about the right thing to do concerning this
>> failure, I think the patch could otherwise go in.
>
> Yes, the patch looks good, please commit it and  I'll look into the
> forward_list failure, since my changes caused it.

I'll be committing a fix for the forward_list debug mode failure (and
some unrelated profile mode failures) tonight.
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/testsuite_container_traits.h
===================================================================
--- testsuite/util/testsuite_container_traits.h	(revision 193118)
+++ testsuite/util/testsuite_container_traits.h	(working copy)
@@ -73,6 +73,7 @@ 
       typedef std::true_type	has_insert;
       typedef std::true_type	has_push_pop;
       typedef std::true_type	has_size_type_constructor;
+      typedef std::true_type	has_emplace;
     };
 
   template<typename _Tp1, typename _Tp2>
@@ -85,6 +86,7 @@ 
       typedef std::true_type	has_insert_after;
       typedef std::true_type	has_push_pop;
       typedef std::true_type	has_size_type_constructor;
+      typedef std::true_type	has_emplace;
     };
 
   template<typename _Tp1, typename _Tp2>
@@ -98,6 +100,7 @@ 
       typedef std::true_type	has_insert;
       typedef std::true_type	has_push_pop;
       typedef std::true_type	has_size_type_constructor;
+      typedef std::true_type	has_emplace;
     };
 
   template<typename _Tp1, typename _Tp2>
@@ -111,6 +114,7 @@ 
       typedef std::true_type	has_throwing_erase;
       typedef std::true_type	has_insert;
       typedef std::true_type	has_size_type_constructor;
+      typedef std::true_type	has_emplace;
     };
 
   template<typename _Tp1, typename _Tp2, typename _Tp3>
@@ -148,9 +152,7 @@ 
 
       typedef std::true_type	has_erase;
       typedef std::true_type	has_insert;
-#ifdef __GXX_EXPERIMENTAL_CXX0X__
       typedef std::true_type	has_emplace;
-#endif
     };
 
   template<typename _Tp1, typename _Tp2, typename _Tp3, typename _Tp4>
@@ -164,9 +166,7 @@ 
 
       typedef std::true_type	has_erase;
       typedef std::true_type	has_insert;
-#ifdef __GXX_EXPERIMENTAL_CXX0X__
       typedef std::true_type	has_emplace;
-#endif
     };
 
   template<typename _Tp1, typename _Tp2, typename _Tp3>
@@ -179,9 +179,7 @@ 
 
       typedef std::true_type	has_erase;
       typedef std::true_type	has_insert;
-#ifdef __GXX_EXPERIMENTAL_CXX0X__
       typedef std::true_type	has_emplace;
-#endif
     };
 
   template<typename _Tp1, typename _Tp2, typename _Tp3>
@@ -194,9 +192,7 @@ 
 
       typedef std::true_type	has_erase;
       typedef std::true_type	has_insert;
-#ifdef __GXX_EXPERIMENTAL_CXX0X__
       typedef std::true_type	has_emplace;
-#endif
     };
 
   template<typename _Tp1, typename _Tp2>
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/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>