On 08/04/19 19:54 +0100, Jonathan Wakely wrote:
On 08/04/19 17:36 +0100, Jonathan Wakely wrote:
On 08/04/19 19:20 +0300, Ville Voutilainen wrote:
On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen
<ville.voutilai...@gmail.com> wrote:
On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely <jwak...@redhat.com> wrote:
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?
I hate the duplication of __gen_vtable with a burning passion, because
*that* is the part that causes me heartburn,
not the compile-time ifs in the other bits. But if this is what you
want to ship, I can live with it.
A bit of elaboration: in all of this, the most dreadful parts to
understand were _Multi_array and __gen_vtable.
Completely agreed, that's why I found extending __gen_vtable_impl to
handle a new feature made it even harder to understand.
Whereas the attached patch *removes* code from __gen_vtable, but fixes
Patch actually attached this time.
the bug in my original std::visit<R> implementation, *and* fixes a bug
in __visitor_result_type (it doesn't use INVOKE).
The downside of this patch is that it fails to diagnose some
ill-formed visitors where not all invocations return the same type. So
it's not acceptable. But I still think there's a simpler design
struggling to get out.
Please commit your patch as-is. We can revisit (pun intended) this
later if necessary.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf624a..42c4bbcc54f 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,7 +138,7 @@ namespace __variant
constexpr variant_alternative_t<_Np, variant<_Types...>> const&&
get(const variant<_Types...>&&);
- template<bool __use_index=false, typename _Visitor, typename... _Variants>
+ template<typename _Result_type, typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
@@ -177,8 +177,6 @@ namespace __variant
struct __variant_cookie {};
// used for raw visitation with indices passed in
struct __variant_idx_cookie {};
- // a more explanatory name than 'true'
- inline constexpr auto __visit_with_index = bool_constant<true>{};
// _Uninitialized<T> is guaranteed to be a literal type, even if T is not.
// We have to do this, because [basic.types]p10.5.3 (n4606) is not implemented
@@ -377,7 +375,7 @@ namespace __variant
constexpr void _M_reset_impl()
{
- __do_visit([](auto&& __this_mem) mutable
+ __do_visit<__detail::__variant::__variant_cookie>([](auto&& __this_mem) mutable
-> __detail::__variant::__variant_cookie
{
if constexpr (!is_same_v<remove_reference_t<decltype(__this_mem)>,
@@ -466,7 +464,7 @@ namespace __variant
void __variant_construct(_Tp&& __lhs, _Up&& __rhs)
{
__lhs._M_index = __rhs._M_index;
- __do_visit([&__lhs](auto&& __rhs_mem) mutable
+ __do_visit<__detail::__variant::__variant_cookie>([&__lhs](auto&& __rhs_mem) mutable
-> __detail::__variant::__variant_cookie
{
__variant_construct_single(std::forward<_Tp>(__lhs),
@@ -594,9 +592,10 @@ namespace __variant
operator=(const _Copy_assign_base& __rhs)
noexcept(_Traits<_Types...>::_S_nothrow_copy_assign)
{
- __do_visit<__visit_with_index>([this](auto&& __rhs_mem,
+ using __detail::__variant::__variant_idx_cookie;
+ __do_visit<__variant_idx_cookie>([this](auto&& __rhs_mem,
auto __rhs_index) mutable
- -> __detail::__variant::__variant_idx_cookie
+ -> __variant_idx_cookie
{
if constexpr (__rhs_index != variant_npos)
{
@@ -663,9 +662,10 @@ namespace __variant
operator=(_Move_assign_base&& __rhs)
noexcept(_Traits<_Types...>::_S_nothrow_move_assign)
{
- __do_visit<__visit_with_index>([this](auto&& __rhs_mem,
+ using __detail::__variant::__variant_idx_cookie;
+ __do_visit<__variant_idx_cookie>([this](auto&& __rhs_mem,
auto __rhs_index) mutable
- -> __detail::__variant::__variant_idx_cookie
+ -> __variant_idx_cookie
{
if constexpr (__rhs_index != variant_npos)
{
@@ -951,14 +951,18 @@ namespace __variant
return __variant_cookie{};
}
- static constexpr decltype(auto)
+ static constexpr _Result_type
__visit_invoke(_Visitor&& __visitor, _Variants... __vars)
{
- if constexpr (is_same_v<_Result_type, __variant_idx_cookie>)
- return std::__invoke(std::forward<_Visitor>(__visitor),
+ 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>()...);
+ else if constexpr (std::is_void_v<_Result_type>)
+ return (void)std::__invoke(std::forward<_Visitor>(__visitor),
__element_by_index_or_cookie<__indices>(
- std::forward<_Variants>(__vars))...,
- integral_constant<size_t, __indices>()...);
+ std::forward<_Variants>(__vars))...);
else
return std::__invoke(std::forward<_Visitor>(__visitor),
__element_by_index_or_cookie<__indices>(
@@ -1117,10 +1121,11 @@ namespace __variant
const variant<_Types...>& __rhs) \
{ \
bool __ret = true; \
- __do_visit<__detail::__variant::__visit_with_index>( \
+ using __detail::__variant::__variant_idx_cookie; \
+ __do_visit<__variant_idx_cookie>( \
[&__ret, &__lhs, __rhs] \
(auto&& __rhs_mem, auto __rhs_index) mutable \
- -> __detail::__variant::__variant_idx_cookie \
+ -> __variant_idx_cookie \
{ \
if constexpr (__rhs_index != variant_npos) \
{ \
@@ -1454,10 +1459,11 @@ namespace __variant
noexcept((__is_nothrow_swappable<_Types>::value && ...)
&& is_nothrow_move_constructible_v<variant>)
{
- __do_visit<__detail::__variant::__visit_with_index>(
+ using __detail::__variant::__variant_idx_cookie;
+ __do_visit<__variant_idx_cookie>(
[this, &__rhs](auto&& __rhs_mem,
auto __rhs_index) mutable
- -> __detail::__variant::__variant_idx_cookie
+ -> __variant_idx_cookie
{
if constexpr (__rhs_index != variant_npos)
{
@@ -1571,26 +1577,11 @@ namespace __variant
return __detail::__variant::__get<_Np>(std::move(__v));
}
- template<bool __use_index, typename _Visitor, typename... _Variants>
- decltype(auto)
- __visitor_result_type(_Visitor&& __visitor, _Variants&&... __variants)
- {
- if constexpr(__use_index)
- return __detail::__variant::__variant_idx_cookie{};
- else
- return std::forward<_Visitor>(__visitor)(
- std::get<0>(std::forward<_Variants>(__variants))...);
- }
-
- template<bool __use_index, typename _Visitor, typename... _Variants>
+ template<typename _Result_type,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants)
{
- using _Result_type =
- decltype(__visitor_result_type<__use_index>(
- std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...));
-
constexpr auto& __vtable = __detail::__variant::__gen_vtable<
_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
@@ -1599,6 +1590,7 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ /// std::visit
template<typename _Visitor, typename... _Variants>
constexpr decltype(auto)
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1606,11 +1598,16 @@ namespace __variant
if ((__variants.valueless_by_exception() || ...))
__throw_bad_variant_access("Unexpected index");
- return __do_visit(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ using _Result_type = invoke_result_t<_Visitor,
+ decltype(__detail::__variant::__get<0>(std::forward<_Variants>(__variants)))...>;
+
+ return __do_visit<_Result_type>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#if __cplusplus > 201703L
+ /// std::visit<R>
template<typename _Res, typename _Visitor, typename... _Variants>
constexpr _Res
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1618,12 +1615,8 @@ 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)...);
+ return __do_visit<_Res>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#endif
@@ -1635,7 +1628,7 @@ namespace __variant
noexcept((is_nothrow_invocable_v<hash<decay_t<_Types>>, _Types> && ...))
{
size_t __ret;
- __do_visit([&__t, &__ret](auto&& __t_mem) mutable
+ __do_visit<__detail::__variant::__variant_cookie>([&__t, &__ret](auto&& __t_mem) mutable
-> __detail::__variant::__variant_cookie
{
using _Type = __remove_cvref_t<decltype(__t_mem)>;
