[v3 PATCH] Don't revisit a variant we are already visiting.

[Jonathan Wakely]

On 28/03/19 15:21 +0200, Ville Voutilainen wrote:
>On Thu, 28 Mar 2019 at 15:07, Ville Voutilainen
><ville.voutilainen@gmail.com> wrote:
>>
>> 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.voutilainen@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.
>
>Minor adjustment, use direct-init in all cases:
>
>-+                  auto __tmp = std::move(__rhs_mem);
>++                  auto __tmp(std::move(__rhs_mem));

>diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
>index 3932a8a..fe96cc8 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)

Please indent this line to match the opening brace, not the
template-head.

>@@ -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)

Does __rhs_index need to be a template parameter in these function
templates? It seems to always be passed size_t, but the value will
always fit in unsigned short. Could it just use size_t? Or even
unsigned, since we don't really need to pass 64 bits?

I think typename _Base::__index_type is strictly correct, but that's a
lot to type and to read.

OK with the indentation tweak, and I'll leave changing the _Idx or not
to your judgment.