On 06/04/19 03:07 +0300, Ville Voutilainen wrote:
On Sat, 6 Apr 2019 at 02:55, Ville Voutilainen
<ville.voutilai...@gmail.com> wrote:
Just in case that cast looks scary: the implicit conversion is also
deep down in __visit_invoke, so
we do actually require implicit convertibility as we are supposed to.
If that's still too scary,
we can just do
- return (_Res)
+ if constexpr (!is_void_v<_Res>)
+ return
+ __do_visit<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
+ else
like in the attached patch.
Okay, I merged that with the original. I also renamed the neg-test, so
here goes the hopefully final variant change for GCC 9:
2019-04-06 Ville Voutilainen <ville.voutilai...@gmail.com>
Fix visit<R> for variant.
* include/std/variant (__do_visit): Add a template parameter
for enforcing same return types for visit.
(__gen_vtable_impl): Likewise.
(_S_apply_single_alt): Adjust.
(__visit_invoke_impl): New. Handle casting to void.
(__do_visit_invoke): New. Enforces same return types.
(__do_visit_invoke_r): New. Converts return types.
(__visit_invoke): Adjust.
(__gen_vtable): Add a template parameter for enforcing
same return types for visit.
* testsuite/20_util/variant/visit_r.cc: Add a test for a visitor with
different return types.
* testsuite/20_util/variant/visit_neg.cc: New. Ensures that
visitors with different return types don't accidentally
compile with regular visitation.
I'm concerned about how dense and impenetrable this code is (the
absence of comments certainly doesn't help).
The attached patch implements the same thing with totally separate
__gen_vtable_r and __gen_vtable_r_impl class templates, instead of
adding all the visit<R> functionality into the existing code (and then
needing to tease it apart again with if-constexpr).
The visit<R> implementation doesn't need to care about the
__variant_cookie or __variant_idx_cookie cases, which simplifies
things.
This also adjusts some whitespace, for correct indentation and for
readability. And removes a redundant && from a type.
What do you think?
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf624a..8370b6c742d 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,12 +138,14 @@ namespace __variant
constexpr variant_alternative_t<_Np, variant<_Types...>> const&&
get(const variant<_Types...>&&);
- template<bool __use_index=false, typename _Visitor, typename... _Variants>
+ template<bool __use_index = false,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
template <typename... _Types, typename _Tp>
- decltype(auto) __variant_cast(_Tp&& __rhs)
+ decltype(auto)
+ __variant_cast(_Tp&& __rhs)
{
if constexpr (is_lvalue_reference_v<_Tp>)
{
@@ -829,14 +831,21 @@ namespace __variant
{
static constexpr size_t __index =
sizeof...(_Variants) - sizeof...(__rest) - 1;
+
using _Variant = typename _Nth_type<__index, _Variants...>::type;
+
static constexpr int __do_cookie =
_Extra_visit_slot_needed<_Ret, _Variant>::value ? 1 : 0;
+
using _Tp = _Ret(*)(_Visitor, _Variants...);
+
template<typename... _Args>
constexpr const _Tp&
_M_access(size_t __first_index, _Args... __rest_indices) const
- { return _M_arr[__first_index + __do_cookie]._M_access(__rest_indices...); }
+ {
+ return _M_arr[__first_index + __do_cookie]
+ ._M_access(__rest_indices...);
+ }
_Multi_array<_Tp, __rest...> _M_arr[__first + __do_cookie];
};
@@ -880,6 +889,7 @@ namespace __variant
using _Next =
remove_reference_t<typename _Nth_type<sizeof...(__indices),
_Variants...>::type>;
+
using _Array_type =
_Multi_array<_Result_type (*)(_Visitor, _Variants...),
__dimensions...>;
@@ -939,7 +949,7 @@ namespace __variant
tuple<_Variants...>, std::index_sequence<__indices...>>
{
using _Array_type =
- _Multi_array<_Result_type (*)(_Visitor&&, _Variants...)>;
+ _Multi_array<_Result_type (*)(_Visitor, _Variants...)>;
template<size_t __index, typename _Variant>
static constexpr decltype(auto)
@@ -956,9 +966,9 @@ namespace __variant
{
if constexpr (is_same_v<_Result_type, __variant_idx_cookie>)
return std::__invoke(std::forward<_Visitor>(__visitor),
- __element_by_index_or_cookie<__indices>(
- std::forward<_Variants>(__vars))...,
- integral_constant<size_t, __indices>()...);
+ __element_by_index_or_cookie<__indices>(
+ std::forward<_Variants>(__vars))...,
+ integral_constant<size_t, __indices>()...);
else
return std::__invoke(std::forward<_Visitor>(__visitor),
__element_by_index_or_cookie<__indices>(
@@ -988,6 +998,104 @@ namespace __variant
static constexpr auto _S_vtable = _S_apply();
};
+#if __cplusplus > 201703L
+ // Similar to __gen_vtable_impl but for visit<R>
+ template<typename _Array_type, typename _Variant_tuple, typename _Index_seq>
+ struct __gen_vtable_r_impl;
+
+ template<typename _Result_type, typename _Visitor, size_t... __dimensions,
+ typename... _Variants, size_t... __indices>
+ struct __gen_vtable_r_impl<
+ _Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
+ tuple<_Variants...>, std::index_sequence<__indices...>>
+ {
+ using _Next =
+ remove_reference_t<typename _Nth_type<sizeof...(__indices),
+ _Variants...>::type>;
+
+ using _Array_type =
+ _Multi_array<_Result_type (*)(_Visitor, _Variants...),
+ __dimensions...>;
+
+ static constexpr _Array_type
+ _S_apply()
+ {
+ _Array_type __vtable{};
+ _S_apply_all_alts(
+ __vtable, make_index_sequence<variant_size_v<_Next>>());
+ return __vtable;
+ }
+
+ template<size_t... __var_indices>
+ static constexpr void
+ _S_apply_all_alts(_Array_type& __vtable,
+ std::index_sequence<__var_indices...>)
+ {
+ (_S_apply_single_alt<__var_indices>(
+ __vtable._M_arr[__var_indices]), ...);
+ }
+
+ template<size_t __index, typename _Tp>
+ static constexpr void
+ _S_apply_single_alt(_Tp& __element)
+ {
+ __element = __gen_vtable_r_impl<
+ remove_reference_t<decltype(__element)>, tuple<_Variants...>,
+ std::index_sequence<__indices..., __index>>::_S_apply();
+ }
+ };
+
+ template<typename _Result_type, typename _Visitor, typename... _Variants,
+ size_t... __indices>
+ struct __gen_vtable_r_impl<
+ _Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
+ tuple<_Variants...>, std::index_sequence<__indices...>>
+ {
+ using _Array_type =
+ _Multi_array<_Result_type (*)(_Visitor, _Variants...)>;
+
+ template<size_t __index, typename _Variant>
+ static constexpr decltype(auto)
+ __element_by_index_or_cookie(_Variant&& __var)
+ {
+ if constexpr (__index != variant_npos)
+ return __variant::__get<__index>(std::forward<_Variant>(__var));
+ else
+ return __variant_cookie{};
+ }
+
+ static constexpr _Result_type
+ __visit_r_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ if constexpr (is_void_v<_Result_type>)
+ return (void) std::__invoke(std::forward<_Visitor>(__visitor),
+ __element_by_index_or_cookie<__indices>(
+ std::forward<_Variants>(__vars))...);
+ else
+ return std::__invoke(std::forward<_Visitor>(__visitor),
+ __element_by_index_or_cookie<__indices>(
+ std::forward<_Variants>(__vars))...);
+ }
+
+ static constexpr _Array_type
+ _S_apply()
+ { return _Array_type{&__visit_r_invoke}; }
+ };
+
+
+ template<typename _Result_type, typename _Visitor, typename... _Variants>
+ struct __gen_vtable_r
+ {
+ using _Array_type =
+ _Multi_array<_Result_type (*)(_Visitor&&, _Variants...),
+ variant_size_v<remove_reference_t<_Variants>>...>;
+
+ static constexpr _Array_type _S_vtable
+ = __gen_vtable_r_impl<_Array_type, tuple<_Variants...>,
+ std::index_sequence<>>::_S_apply();
+ };
+#endif // C++20
+
template<size_t _Np, typename _Tp>
struct _Base_dedup : public _Tp { };
@@ -1599,6 +1707,7 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ /// std::visit
template<typename _Visitor, typename... _Variants>
constexpr decltype(auto)
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1611,6 +1720,7 @@ namespace __variant
}
#if __cplusplus > 201703L
+ /// std::visit<R>
template<typename _Res, typename _Visitor, typename... _Variants>
constexpr _Res
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1618,12 +1728,12 @@ namespace __variant
if ((__variants.valueless_by_exception() || ...))
__throw_bad_variant_access("Unexpected index");
- if constexpr (std::is_void_v<_Res>)
- (void) __do_visit(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
- else
- return __do_visit(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ constexpr auto& __vtable = __detail::__variant::__gen_vtable_r<
+ _Res, _Visitor&&, _Variants&&...>::_S_vtable;
+
+ auto __func_ptr = __vtable._M_access(__variants.index()...);
+ return (*__func_ptr)(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#endif
