This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [v3 PATCH] Don't revisit a variant we are already visiting.
- From: Ville Voutilainen <ville dot voutilainen at gmail dot com>
- To: "libstdc++" <libstdc++ at gcc dot gnu dot org>, gcc-patches List <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 28 Mar 2019 15:21:24 +0200
- Subject: Re: [v3 PATCH] Don't revisit a variant we are already visiting.
- References: <CAFk2RUYK0nF-jDA_wASDSkudhMJ6p1HwcvqT4Uo9yf_zXv4paQ@mail.gmail.com>
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)
+ {
+ 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 {};