This shaves off some unnecessary codegen. In the assignment
operators and swap, we are already visiting the rhs, so we shouldn't
visit it again to get to its guts, we already have them in-hand.

2019-03-28  Ville Voutilainen  <ville.voutilai...@gmail.com>

    Don't revisit a variant we are already visiting.
    * include/std/variant (__variant_construct_single): New.
    (__variant_construct): Use it.
    (_M_destructive_move): Likewise, turn into a template.
    (_M_destructive_copy): Likewise.
    (_Copy_assign_base::operator=): Adjust.
    (_Move_assign_base::operator=): Likewise.
    (swap): Likewise.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 3932a8a..003cc15 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -428,20 +428,25 @@ namespace __variant
     using _Variant_storage_alias =
 	_Variant_storage<_Traits<_Types...>::_S_trivial_dtor, _Types...>;
 
+  template<typename _Tp, typename _Up>
+  void __variant_construct_single(_Tp&& __lhs, _Up&& __rhs_mem)
+    {
+      void* __storage = std::addressof(__lhs._M_u);
+      using _Type = remove_reference_t<decltype(__rhs_mem)>;
+      if constexpr (!is_same_v<_Type, __variant_cookie>)
+        ::new (__storage)
+	  _Type(std::forward<decltype(__rhs_mem)>(__rhs_mem));
+    }
+
   template<typename... _Types, typename _Tp, typename _Up>
     void __variant_construct(_Tp&& __lhs, _Up&& __rhs)
     {
       __lhs._M_index = __rhs._M_index;
-      void* __storage = std::addressof(__lhs._M_u);
-      __do_visit([__storage](auto&& __rhs_mem) mutable
+      __do_visit([&__lhs](auto&& __rhs_mem) mutable
 		 -> __detail::__variant::__variant_cookie
         {
-	  using _Type = remove_reference_t<decltype(__rhs_mem)>;
-	  if constexpr (is_same_v<__remove_cvref_t<decltype(__rhs_mem)>,
-			          remove_cv_t<_Type>>
-			&& !is_same_v<_Type, __variant_cookie>)
-	    ::new (__storage)
-	      _Type(std::forward<decltype(__rhs_mem)>(__rhs_mem));
+	  __variant_construct_single(std::forward<_Tp>(__lhs),
+				     std::forward<decltype(__rhs_mem)>(__rhs_mem));
 	  return {};
 	}, __variant_cast<_Types...>(std::forward<decltype(__rhs)>(__rhs)));
     }
@@ -490,34 +495,38 @@ namespace __variant
 	  std::forward<_Move_ctor_base>(__rhs));
       }
 
-      void _M_destructive_move(_Move_ctor_base&& __rhs)
-      {
-	this->_M_reset();
-	__try
-	  {
-	    __variant_construct<_Types...>(*this,
-	      std::forward<_Move_ctor_base>(__rhs));
-	  }
-	__catch (...)
-	  {
-	    this->_M_index = variant_npos;
-	    __throw_exception_again;
-	  }
-      }
+      template<typename _Idx, typename _Up>
+        void _M_destructive_move(_Idx __rhs_index, _Up&& __rhs)
+        {
+	  this->_M_reset();
+	  this->_M_index = __rhs_index;
+	  __try
+	    {
+	      __variant_construct_single(*this,
+					 std::forward<_Up>(__rhs));
+	    }
+	  __catch (...)
+	    {
+	      this->_M_index = variant_npos;
+	      __throw_exception_again;
+	    }
+	}
 
-      void _M_destructive_copy(const _Move_ctor_base& __rhs)
-      {
-	this->_M_reset();
-	__try
-	  {
-	    __variant_construct<_Types...>(*this, __rhs);
-	  }
-	__catch (...)
-	  {
-	    this->_M_index = variant_npos;
-	    __throw_exception_again;
-	  }
-      }
+      template<typename _Idx, typename _Up>
+        void _M_destructive_copy(_Idx __rhs_index, const _Up& __rhs)
+        {
+	  this->_M_reset();
+	  this->_M_index = __rhs_index;
+	  __try
+	    {
+	      __variant_construct_single(*this, __rhs);
+	    }
+	  __catch (...)
+	    {
+	      this->_M_index = variant_npos;
+	      __throw_exception_again;
+	    }
+	}
 
       _Move_ctor_base(const _Move_ctor_base&) = default;
       _Move_ctor_base& operator=(const _Move_ctor_base&) = default;
@@ -530,17 +539,21 @@ namespace __variant
       using _Base = _Copy_ctor_alias<_Types...>;
       using _Base::_Base;
 
-      void _M_destructive_move(_Move_ctor_base&& __rhs)
-      {
-	this->_M_reset();
-	__variant_construct<_Types...>(*this,
-          std::forward<_Move_ctor_base>(__rhs));
-      }
-      void _M_destructive_copy(const _Move_ctor_base& __rhs)
-      {
-	this->_M_reset();
-	__variant_construct<_Types...>(*this, __rhs);
-      }
+      template<typename _Idx, typename _Up>
+        void _M_destructive_move(_Idx __rhs_index, _Up&& __rhs)
+        {
+	  this->_M_reset();
+	  this->_M_index = __rhs_index;
+	  __variant_construct_single(*this,
+				     std::forward<_Up>(__rhs));
+	}
+      template<typename _Idx, typename _Up>
+        void _M_destructive_copy(_Idx __rhs_index, const _Up& __rhs)
+        {
+	  this->_M_reset();
+	  this->_M_index = __rhs_index;
+	  __variant_construct_single(*this, __rhs);
+	}
     };
 
   template<typename... _Types>
@@ -580,12 +593,13 @@ namespace __variant
 		    if constexpr (is_nothrow_copy_constructible_v<__rhs_type>
 		      || !is_nothrow_move_constructible_v<__rhs_type>)
 		      {
-			this->_M_destructive_copy(__rhs);
+			this->_M_destructive_copy(__rhs._M_index, __rhs_mem);
 		      }
 		    else
 		      {
-			_Copy_assign_base __tmp(__rhs);
-			this->_M_destructive_move(std::move(__tmp));
+			auto __tmp(__rhs_mem);
+			this->_M_destructive_move(__rhs._M_index,
+						  std::move(__tmp));
 		      }
 		  }
 		else
@@ -641,8 +655,8 @@ namespace __variant
 	      }
 	    else
 	      {
-		_Move_assign_base __tmp(std::move(__rhs));
-		this->_M_destructive_move(std::move(__tmp));
+		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));
@@ -1409,21 +1423,26 @@ namespace __variant
 			        remove_reference_t<decltype(__this_mem)>,
 			        __detail::__variant::__variant_cookie>)
 		  {
-		    this->_M_destructive_move(std::move(__rhs));
+		    this->_M_destructive_move(__rhs.index(),
+					      std::move(__rhs_mem));
 		    __rhs._M_reset();
 		  }
 		else if constexpr (is_same_v<
 			             remove_reference_t<decltype(__rhs_mem)>,
 			             __detail::__variant::__variant_cookie>)
 		  {
-		    __rhs._M_destructive_move(std::move(*this));
+		    __rhs._M_destructive_move(this->index(),
+					      std::move(__this_mem));
 		    this->_M_reset();
 		  }
 		else
 		  {
-		    auto __tmp = std::move(__rhs);
-		    __rhs._M_destructive_move(std::move(*this));
-		    this->_M_destructive_move(std::move(__tmp));
+		    auto __tmp = std::move(__rhs_mem);
+		    auto __idx = __rhs.index();
+		    __rhs._M_destructive_move(this->index(),
+					      std::move(__this_mem));
+		    this->_M_destructive_move(__idx,
+					      std::move(__tmp));
 		  }
 	      }
 	  return {};

Reply via email to