Re: [PATCH] libstdc++/85184 remove __glibcxx_assert assertions from

[François Dumont]

On 15/05/2018 18:56, Jonathan Wakely wrote:

As I said in the bugzilla PR, these assertions are all to catch our
own mistakes, not user error.

If we're comfortable the code is correct then we should remove them.

Should we wait until near the end of stage 1, to get more time with
these checks in place?



I am in favor of doing this cleanup now.

We will surely forget it later and we have proper tests to track our dev 
mistakes rather than assertions.


François

[PATCH] libstdc++/85184 remove __glibcxx_assert assertions from

[Jonathan Wakely]

As I said in the bugzilla PR, these assertions are all to catch our
own mistakes, not user error.

If we're comfortable the code is correct then we should remove them.

Should we wait until near the end of stage 1, to get more time with
these checks in place?


diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 8359f4f5335..a3d74966435 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,8 @@
+2018-05-15  Jonathan Wakely  
+
+	PR libstdc++/85184
+	* include/std/variant: Remove all uses of __glibcxx_assert.
+
 2018-05-15  Jonathan Wakely  
 
 	PR libstdc++/85749
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index c0212404bb2..efc1d3bf1e0 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -555,7 +555,6 @@ namespace __variant
 		__throw_exception_again;
 	  }
 	  }
-	__glibcxx_assert(this->_M_index == __rhs._M_index);
 	return *this;
   }
 
@@ -624,7 +623,6 @@ namespace __variant
 		__throw_exception_again;
 	  }
 	  }
-	__glibcxx_assert(this->_M_index == __rhs._M_index);
 	return *this;
   }
 
@@ -1105,7 +1103,7 @@ namespace __variant
 	noexcept(is_nothrow_constructible_v<__accepted_type<_Tp&&>, _Tp&&>)
 	: variant(in_place_index<__accepted_index<_Tp&&>>,
 		  std::forward<_Tp>(__t))
-	{ __glibcxx_assert(holds_alternative<__accepted_type<_Tp&&>>(*this)); }
+	{ }
 
   template
@@ -1114,7 +1112,7 @@ namespace __variant
 	variant(in_place_type_t<_Tp>, _Args&&... __args)
 	: variant(in_place_index<__index_of<_Tp>>,
 		  std::forward<_Args>(__args)...)
-	{ __glibcxx_assert(holds_alternative<_Tp>(*this)); }
+	{ }
 
   template
@@ -1125,7 +1123,7 @@ namespace __variant
 		_Args&&... __args)
 	: variant(in_place_index<__index_of<_Tp>>, __il,
 		  std::forward<_Args>(__args)...)
-	{ __glibcxx_assert(holds_alternative<_Tp>(*this)); }
+	{ }
 
   template, _Args&&... __args)
 	: _Base(in_place_index<_Np>, std::forward<_Args>(__args)...),
 	_Default_ctor_enabler(_Enable_default_constructor_tag{})
-	{ __glibcxx_assert(index() == _Np); }
+	{ }
 
   template,
@@ -1144,7 +1142,7 @@ namespace __variant
 		_Args&&... __args)
 	: _Base(in_place_index<_Np>, __il, std::forward<_Args>(__args)...),
 	_Default_ctor_enabler(_Enable_default_constructor_tag{})
-	{ __glibcxx_assert(index() == _Np); }
+	{ }
 
   template
 	enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
@@ -1160,7 +1158,6 @@ namespace __variant
 	std::get<__index>(*this) = std::forward<_Tp>(__rhs);
 	  else
 	this->emplace<__index>(std::forward<_Tp>(__rhs));
-	  __glibcxx_assert(holds_alternative<__accepted_type<_Tp&&>>(*this));
 	  return *this;
 	}
 
@@ -1171,7 +1168,6 @@ namespace __variant
 	{
 	  auto& ret =
 	this->emplace<__index_of<_Tp>>(std::forward<_Args>(__args)...);
-	  __glibcxx_assert(holds_alternative<_Tp>(*this));
 	  return ret;
 	}
 
@@ -1184,7 +1180,6 @@ namespace __variant
 	  auto& ret =
 	this->emplace<__index_of<_Tp>>(__il,
 	   std::forward<_Args>(__args)...);
-	  __glibcxx_assert(holds_alternative<_Tp>(*this));
 	  return ret;
 	}
 
@@ -1207,7 +1202,6 @@ namespace __variant
 	  this->_M_index = variant_npos;
 	  __throw_exception_again;
 	}
-	  __glibcxx_assert(index() == _Np);
 	  return std::get<_Np>(*this);
 	}
 
@@ -1230,7 +1224,6 @@ namespace __variant
 	  this->_M_index = variant_npos;
 	  __throw_exception_again;
 	}
-	  __glibcxx_assert(index() == _Np);
 	  return std::get<_Np>(*this);
 	}