On Thu, 28 Mar 2019 at 15:07, Ville Voutilainen <ville.voutilai...@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.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.
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 {};