[v3 PATCH] Use single-visitation in variant assignment and swap.

[Jonathan Wakely]

On 01/04/19 11:45 +0300, Ville Voutilainen wrote:
>@@ -570,45 +574,44 @@ namespace __variant
>       operator=(const _Copy_assign_base& __rhs)
> 	  noexcept(_Traits<_Types...>::_S_nothrow_copy_assign)
>       {
>-	__do_visit([this, &__rhs](auto&& __this_mem, auto&& __rhs_mem) mutable
>-		   -> __detail::__variant::__variant_cookie
>+	__do_visit<__visit_with_index>([this](auto&& __rhs_mem,
>+					      auto __rhs_index) mutable
>+	    -> __detail::__variant::__variant_idx_cookie
> 	  {
>-	    if constexpr (is_same_v<
>-			    remove_reference_t<decltype(__this_mem)>,
>-			    remove_reference_t<decltype(__rhs_mem)>>)
>+	    if constexpr (__rhs_index != variant_npos)
> 	      {
>-		if constexpr (!is_same_v<
>-			        remove_reference_t<decltype(__rhs_mem)>,
>-			        __variant_cookie>)
>-		  __this_mem = __rhs_mem;
>-	      }
>-	    else
>-	      {
>-		if constexpr (!is_same_v<
>-			        remove_reference_t<decltype(__rhs_mem)>,
>-			        __variant_cookie>)
>+		if (this->_M_index == __rhs_index)
> 		  {
>-		    using __rhs_type =
>-		      remove_reference_t<decltype(__rhs_mem)>;
>-		    if constexpr (is_nothrow_copy_constructible_v<__rhs_type>
>-		      || !is_nothrow_move_constructible_v<__rhs_type>)
>+		    if constexpr (__rhs_index != variant_npos)
> 		      {
>-			this->_M_destructive_copy(__rhs._M_index, __rhs_mem);
>+			auto& __this_mem =
>+			  __get<__rhs_index>(*this);

Qualify this as __variant::__get please.

>+			if constexpr (is_same_v<
>+				      remove_reference_t<decltype(__this_mem)>,
>+				      remove_reference_t<decltype(__rhs_mem)>>)
>+			  __this_mem = __rhs_mem;
> 		      }
>+		  }
>+		else
>+		  {
>+		    using __rhs_type =
>+		      remove_reference_t<decltype(__rhs_mem)>;
>+		    if constexpr
>+		      (is_nothrow_copy_constructible_v<__rhs_type>
>+		       || !is_nothrow_move_constructible_v<__rhs_type>)
>+			this->_M_destructive_copy(__rhs_index, __rhs_mem);
> 		    else
> 		      {
> 			auto __tmp(__rhs_mem);
>-			this->_M_destructive_move(__rhs._M_index,
>+			this->_M_destructive_move(__rhs_index,
> 						  std::move(__tmp));
> 		      }
> 		  }
>-		else
>-		  {
>-		    this->_M_reset();
>-		  }
> 	      }
>-	  return {};
>-	}, __variant_cast<_Types...>(*this), __variant_cast<_Types...>(__rhs));
>+	    else
>+	      this->_M_reset();
>+	    return {};
>+	  }, __variant_cast<_Types...>(__rhs));
> 	__glibcxx_assert(this->_M_index == __rhs._M_index);
> 	return *this;
>       }
>@@ -641,25 +644,32 @@ namespace __variant
>       operator=(_Move_assign_base&& __rhs)
> 	  noexcept(_Traits<_Types...>::_S_nothrow_move_assign)
>       {
>-	__do_visit([this, &__rhs](auto&& __this_mem, auto&& __rhs_mem) mutable
>-		   -> __detail::__variant::__variant_cookie
>+	__do_visit<__visit_with_index>([this](auto&& __rhs_mem,
>+					      auto __rhs_index) mutable
>+	    -> __detail::__variant::__variant_idx_cookie
> 	  {
>-	    if constexpr (is_same_v<
>-			    remove_reference_t<decltype(__this_mem)>,
>-			    remove_reference_t<decltype(__rhs_mem)>>)
>+	    if constexpr (__rhs_index != variant_npos)
> 	      {
>-		if constexpr (!is_same_v<
>-			        remove_reference_t<decltype(__rhs_mem)>,
>-			        __variant_cookie>)
>-		  __this_mem = std::move(__rhs_mem);
>+		if (this->_M_index == __rhs_index)
>+		  {
>+		    if constexpr (__rhs_index != variant_npos)
>+		      {
>+			auto& __this_mem =
>+			  __get<__rhs_index>(*this);

And here.

>+			if constexpr (is_same_v<
>+				      remove_reference_t<decltype(__this_mem)>,
>+				      remove_reference_t<decltype(__rhs_mem)>>)
>+			  __this_mem = std::move(__rhs_mem);
>+		      }
>+		  }
>+		else
>+		  this->_M_destructive_move(__rhs_index,
>+					    std::move(__rhs_mem));
> 	      }
> 	    else
>-	      {
>-		auto __tmp(std::move(__rhs_mem));
>-		this->_M_destructive_move(__rhs._M_index, std::move(__tmp));
>-	      }
>-	  return {};
>-	}, __variant_cast<_Types...>(*this), __variant_cast<_Types...>(__rhs));
>+	      this->_M_reset();
>+	    return {};
>+	  }, __variant_cast<_Types...>(__rhs));
> 	__glibcxx_assert(this->_M_index == __rhs._M_index);
> 	return *this;
>       }
>@@ -1082,25 +1099,25 @@ namespace __variant
> 				 const variant<_Types...>& __rhs) \
>     { \
>       bool __ret = true; \
>-      __do_visit([&__ret, &__lhs, __rhs] \
>-		 (auto&& __this_mem, auto&& __rhs_mem) mutable	\
>-		   -> __detail::__variant::__variant_cookie \
>+      __do_visit<__detail::__variant::__visit_with_index>( \
>+        [&__ret, &__lhs, __rhs] \
>+		 (auto&& __rhs_mem, auto __rhs_index) mutable \
>+		   -> __detail::__variant::__variant_idx_cookie \
>         { \
>-	  if constexpr (!is_same_v< \
>-			  remove_reference_t<decltype(__this_mem)>, \
>-			  remove_reference_t<decltype(__rhs_mem)>> \
>-			|| is_same_v<decltype(__this_mem), \
>-			             __detail::__variant::__variant_cookie>) \
>-	    __ret = (__lhs.index() + 1) __OP (__rhs.index() + 1); \
>-	  else if constexpr (is_same_v< \
>-			       remove_reference_t<decltype(__this_mem)>, \
>-			       remove_reference_t<decltype(__rhs_mem)>> \
>-                             && !is_same_v< \
>-	                          remove_reference_t<decltype(__this_mem)>, \
>-				  __detail::__variant::__variant_cookie>) \
>-	    __ret = __this_mem __OP __rhs_mem; \
>+	  if constexpr (__rhs_index != variant_npos) \
>+	    { \
>+	      if (__lhs.index() == __rhs_index) \
>+	        { \
>+		  auto& __this_mem = get<__rhs_index>(__lhs); \

And std::get here.

>+                  __ret = __this_mem __OP __rhs_mem; \
>+                } \
>+	      else \
>+		__ret = (__lhs.index() + 1) __OP (__rhs_index + 1); \
>+            } \
>+          else \
>+            __ret = (__lhs.index() + 1) __OP (__rhs_index + 1); \
> 	  return {}; \
>-	}, __lhs, __rhs); \
>+	}, __rhs); \
>       return __ret; \
>     } \
> \
>@@ -1402,51 +1419,47 @@ namespace __variant
>       noexcept((__is_nothrow_swappable<_Types>::value && ...)
> 	       && is_nothrow_move_constructible_v<variant>)
>       {
>-	__do_visit([this, &__rhs](auto&& __this_mem, auto&& __rhs_mem) mutable
>-		   -> __detail::__variant::__variant_cookie
>+	__do_visit<__detail::__variant::__visit_with_index>(
>+	  [this, &__rhs](auto&& __rhs_mem,
>+			 auto __rhs_index) mutable
>+	    -> __detail::__variant::__variant_idx_cookie
> 	  {
>-	    if constexpr (is_same_v<
>-			    remove_reference_t<decltype(__this_mem)>,
>-			    remove_reference_t<decltype(__rhs_mem)>>)
>+	    if constexpr (__rhs_index != variant_npos)
> 	      {
>-		if constexpr (!is_same_v<
>-			        remove_reference_t<decltype(__rhs_mem)>,
>-			        __detail::__variant::__variant_cookie>)
>+		if (this->index() == __rhs_index)
> 		  {
>+		    auto& __this_mem =
>+		      get<__rhs_index>(*this);

And here.

OK with those four qualifications, thanks very much.