libstdc++/85184 remove __glibcxx_assert assertions from <variant>

Message ID 20180515165639.GJ7974@redhat.com
State New
Headers show
Series
  • libstdc++/85184 remove __glibcxx_assert assertions from <variant>
Related show

Commit Message

Jonathan Wakely May 15, 2018, 4:56 p.m.
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?

Comments

Fran├žois Dumont May 18, 2018, 8:52 a.m. | #1
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

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  <jwakely@redhat.com>
+
+	PR libstdc++/85184
+	* include/std/variant: Remove all uses of __glibcxx_assert.
+
 2018-05-15  Jonathan Wakely  <jwakely@redhat.com>
 
 	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<typename _Tp, typename... _Args,
 	       typename = enable_if_t<__exactly_once<_Tp>
@@ -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<typename _Tp, typename _Up, typename... _Args,
 	       typename = enable_if_t<__exactly_once<_Tp>
@@ -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<size_t _Np, typename... _Args,
 	       typename = enable_if_t<
@@ -1134,7 +1132,7 @@  namespace __variant
 	variant(in_place_index_t<_Np>, _Args&&... __args)
 	: _Base(in_place_index<_Np>, std::forward<_Args>(__args)...),
 	_Default_ctor_enabler(_Enable_default_constructor_tag{})
-	{ __glibcxx_assert(index() == _Np); }
+	{ }
 
       template<size_t _Np, typename _Up, typename... _Args,
 	       typename = enable_if_t<is_constructible_v<__to_type<_Np>,
@@ -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<typename _Tp>
 	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);
 	}