* [PATCH] Implement std::visit<R> for C++2a (P0655R1)
@ 2019-04-05 18:06 Jonathan Wakely
2019-04-05 20:29 ` Jonathan Wakely
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2019-04-05 18:06 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 215 bytes --]
* doc/xml/manual/status_cxx2020.xml: Update status.
* include/std/variant (visit<R>): Define for C++2a (P0655R1).
* testsuite/20_util/variant/visit_r.cc: New test.
Tested powerpc64le-linux, committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3922 bytes --]
commit 60c01065cfdda4f93bd140b0ea37be16f537877c
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Fri Apr 5 18:36:25 2019 +0100
Implement std::visit<R> for C++2a (P0655R1)
* doc/xml/manual/status_cxx2020.xml: Update status.
* include/std/variant (visit<R>): Define for C++2a (P0655R1).
* testsuite/20_util/variant/visit_r.cc: New test.
diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2020.xml b/libstdc++-v3/doc/xml/manual/status_cxx2020.xml
index d40185c5db6..cedb3d03066 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2020.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2020.xml
@@ -712,14 +712,13 @@ Feature-testing recommendations for C++</link>.
</row>
<row>
- <?dbhtml bgcolor="#C8B0B0" ?>
<entry> <code>visit<R></code>: Explicit Return Type for <code>visit</code> </entry>
<entry>
<link xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0655r1.pdf">
P0655R1
</link>
</entry>
- <entry align="center"> </entry>
+ <entry align="center"> 9.1 </entry>
<entry />
</row>
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index e52aa403009..fdf04cf624a 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1610,6 +1610,23 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+#if __cplusplus > 201703L
+ template<typename _Res, typename _Visitor, typename... _Variants>
+ constexpr _Res
+ visit(_Visitor&& __visitor, _Variants&&... __variants)
+ {
+ 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)...);
+ }
+#endif
+
template<bool, typename... _Types>
struct __variant_hash_call_base_impl
{
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
new file mode 100644
index 00000000000..5eed0cf1a49
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
@@ -0,0 +1,49 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <variant>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+ struct Visitor
+ {
+ int operator()(int, void*) const { return 0; }
+ int operator()(char& c, void* p) const { return &c == p; }
+ int operator()(int i, const char* s) const { return s[i] == '\0'; }
+ int operator()(char c, const char* s) const { return c == *s; }
+ };
+
+ std::variant<int, char> v1{'c'};
+ std::variant<void*, const char*> v2{"chars"};
+
+ auto res = std::visit<bool>(Visitor{}, v1, v2);
+ static_assert(std::is_same_v<decltype(res), bool>);
+ VERIFY( res == true );
+
+ static_assert(std::is_void_v<decltype(std::visit<void>(Visitor{}, v1, v2))>);
+}
+
+int
+main()
+{
+ test01();
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-05 18:06 [PATCH] Implement std::visit<R> for C++2a (P0655R1) Jonathan Wakely
@ 2019-04-05 20:29 ` Jonathan Wakely
2019-04-05 22:55 ` Ville Voutilainen
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2019-04-05 20:29 UTC (permalink / raw)
To: libstdc++, gcc-patches
On 05/04/19 19:06 +0100, Jonathan Wakely wrote:
> * doc/xml/manual/status_cxx2020.xml: Update status.
> * include/std/variant (visit<R>): Define for C++2a (P0655R1).
> * testsuite/20_util/variant/visit_r.cc: New test.
This implementation is wrong, the conversions to R need to happen for
each possible invocation, which makes it more complicated (we need to
duplicate some code). I'll fix it next week.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-05 20:29 ` Jonathan Wakely
@ 2019-04-05 22:55 ` Ville Voutilainen
2019-04-05 23:19 ` Ville Voutilainen
0 siblings, 1 reply; 15+ messages in thread
From: Ville Voutilainen @ 2019-04-05 22:55 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]
On Fri, 5 Apr 2019 at 23:29, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On 05/04/19 19:06 +0100, Jonathan Wakely wrote:
> > * doc/xml/manual/status_cxx2020.xml: Update status.
> > * include/std/variant (visit<R>): Define for C++2a (P0655R1).
> > * testsuite/20_util/variant/visit_r.cc: New test.
>
> This implementation is wrong, the conversions to R need to happen for
> each possible invocation, which makes it more complicated (we need to
> duplicate some code). I'll fix it next week.
Deep sigh. I wanted to postpone this feature until next stage1, but
now that you have sent soldiers
to Mirkwood, brace yourself for The Elf. :) Tested on Linux-x64, OK for trunk?
2019-04-06 Ville Voutilainen <ville.voutilainen@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_r_neg.cc: New. Ensures that
visitors with different return types don't accidentally
compile with regular visitation.
[-- Attachment #2: variant-visit_r.diff --]
[-- Type: text/x-patch, Size: 8711 bytes --]
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf..2320d43d 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,7 +138,9 @@ 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,
+ bool __same_return_types = true,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
@@ -868,12 +870,15 @@ namespace __variant
// tuple<V1&&, V2&&>, std::index_sequence<1, 2>>
// The returned multi-dimensional vtable can be fast accessed by the visitor
// using index calculation.
- template<typename _Array_type, typename _Variant_tuple, typename _Index_seq>
+ template<bool __same_return_types,
+ typename _Array_type, typename _Variant_tuple, typename _Index_seq>
struct __gen_vtable_impl;
- template<typename _Result_type, typename _Visitor, size_t... __dimensions,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, size_t... __dimensions,
typename... _Variants, size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -915,10 +920,12 @@ namespace __variant
if constexpr (__do_cookie)
{
__element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
*__cookie_element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., variant_npos>>::_S_apply();
@@ -926,15 +933,18 @@ namespace __variant
else
{
__element = __gen_vtable_impl<
+ __same_return_types,
remove_reference_t<decltype(__element)>, tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
}
}
};
- template<typename _Result_type, typename _Visitor, typename... _Variants,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants,
size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -952,25 +962,55 @@ namespace __variant
}
static constexpr decltype(auto)
- __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ __visit_invoke_impl(_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 (is_same_v<_Result_type, void>)
+ 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>(
std::forward<_Variants>(__vars))...);
}
+ static constexpr decltype(auto)
+ __do_visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr _Result_type
+ __do_visit_invoke_r(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr decltype(auto)
+ __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ if constexpr (__same_return_types)
+ return __do_visit_invoke(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ else
+ return __do_visit_invoke_r(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
static constexpr auto
_S_apply()
{ return _Array_type{&__visit_invoke}; }
};
- template<typename _Result_type, typename _Visitor, typename... _Variants>
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants>
struct __gen_vtable
{
using _Func_ptr = _Result_type (*)(_Visitor&&, _Variants...);
@@ -981,7 +1021,8 @@ namespace __variant
static constexpr _Array_type
_S_apply()
{
- return __gen_vtable_impl<_Array_type, tuple<_Variants...>,
+ return __gen_vtable_impl<__same_return_types,
+ _Array_type, tuple<_Variants...>,
std::index_sequence<>>::_S_apply();
}
@@ -1582,7 +1623,9 @@ namespace __variant
std::get<0>(std::forward<_Variants>(__variants))...);
}
- template<bool __use_index, typename _Visitor, typename... _Variants>
+ template<bool __use_index,
+ bool __same_return_types,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants)
{
@@ -1592,6 +1635,7 @@ namespace __variant
std::forward<_Variants>(__variants)...));
constexpr auto& __vtable = __detail::__variant::__gen_vtable<
+ __same_return_types,
_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
auto __func_ptr = __vtable._M_access(__variants.index()...);
@@ -1618,12 +1662,9 @@ 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<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#endif
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
index 5eed0cf..28a6e79 100644
--- a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
@@ -42,8 +42,21 @@ test01()
static_assert(std::is_void_v<decltype(std::visit<void>(Visitor{}, v1, v2))>);
}
+void test02()
+{
+ struct Visitor
+ {
+ int operator()(double) {return 42;}
+ double operator()(int) {return 0.02;}
+ };
+ std::variant<int, double> v;
+ std::visit<int>(Visitor(), v);
+}
+
+
int
main()
{
test01();
+ test02();
}
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc
new file mode 100644
index 0000000..386e779
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <variant>
+#include <testsuite_hooks.h>
+
+// { dg-error "invalid conversion" "" { target *-*-* } 0 }
+// { dg-prune-output "in 'constexpr' expansion" }
+
+void
+test01()
+{
+ {
+ struct Visitor
+ {
+ int operator()(double) {return 42;}
+ double operator()(int) {return 0.02;}
+ };
+ std::variant<int, double> v;
+ std::visit(Visitor(), v);
+ }
+}
+
+int
+main()
+{
+ test01();
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-05 22:55 ` Ville Voutilainen
@ 2019-04-05 23:19 ` Ville Voutilainen
2019-04-05 23:48 ` Ville Voutilainen
0 siblings, 1 reply; 15+ messages in thread
From: Ville Voutilainen @ 2019-04-05 23:19 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]
On Sat, 6 Apr 2019 at 01:55, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
>
> On Fri, 5 Apr 2019 at 23:29, Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > On 05/04/19 19:06 +0100, Jonathan Wakely wrote:
> > > * doc/xml/manual/status_cxx2020.xml: Update status.
> > > * include/std/variant (visit<R>): Define for C++2a (P0655R1).
> > > * testsuite/20_util/variant/visit_r.cc: New test.
> >
> > This implementation is wrong, the conversions to R need to happen for
> > each possible invocation, which makes it more complicated (we need to
> > duplicate some code). I'll fix it next week.
>
> Deep sigh. I wanted to postpone this feature until next stage1, but
> now that you have sent soldiers
> to Mirkwood, brace yourself for The Elf. :) Tested on Linux-x64, OK for trunk?
>
> 2019-04-06 Ville Voutilainen <ville.voutilainen@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_r_neg.cc: New. Ensures that
> visitors with different return types don't accidentally
> compile with regular visitation.
And this patch fixes the existing visitation so that we don't
over-eagerly cast to void. The main gist of it is
- else if constexpr (is_same_v<_Result_type, void>)
+ else if constexpr (!__same_return_types &&
+ is_same_v<_Result_type, void>)
and the new ensure-test is adjusted to use void as the return type of
the first visitor.
[-- Attachment #2: variant-visit_r_2.diff --]
[-- Type: text/x-patch, Size: 8732 bytes --]
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf..e1960bc 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,7 +138,9 @@ 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,
+ bool __same_return_types = true,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
@@ -868,12 +870,15 @@ namespace __variant
// tuple<V1&&, V2&&>, std::index_sequence<1, 2>>
// The returned multi-dimensional vtable can be fast accessed by the visitor
// using index calculation.
- template<typename _Array_type, typename _Variant_tuple, typename _Index_seq>
+ template<bool __same_return_types,
+ typename _Array_type, typename _Variant_tuple, typename _Index_seq>
struct __gen_vtable_impl;
- template<typename _Result_type, typename _Visitor, size_t... __dimensions,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, size_t... __dimensions,
typename... _Variants, size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -915,10 +920,12 @@ namespace __variant
if constexpr (__do_cookie)
{
__element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
*__cookie_element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., variant_npos>>::_S_apply();
@@ -926,15 +933,18 @@ namespace __variant
else
{
__element = __gen_vtable_impl<
+ __same_return_types,
remove_reference_t<decltype(__element)>, tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
}
}
};
- template<typename _Result_type, typename _Visitor, typename... _Variants,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants,
size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -952,25 +962,56 @@ namespace __variant
}
static constexpr decltype(auto)
- __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ __visit_invoke_impl(_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 (!__same_return_types &&
+ is_same_v<_Result_type, void>)
+ 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>(
std::forward<_Variants>(__vars))...);
}
+ static constexpr decltype(auto)
+ __do_visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr _Result_type
+ __do_visit_invoke_r(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr decltype(auto)
+ __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ if constexpr (__same_return_types)
+ return __do_visit_invoke(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ else
+ return __do_visit_invoke_r(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
static constexpr auto
_S_apply()
{ return _Array_type{&__visit_invoke}; }
};
- template<typename _Result_type, typename _Visitor, typename... _Variants>
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants>
struct __gen_vtable
{
using _Func_ptr = _Result_type (*)(_Visitor&&, _Variants...);
@@ -981,7 +1022,8 @@ namespace __variant
static constexpr _Array_type
_S_apply()
{
- return __gen_vtable_impl<_Array_type, tuple<_Variants...>,
+ return __gen_vtable_impl<__same_return_types,
+ _Array_type, tuple<_Variants...>,
std::index_sequence<>>::_S_apply();
}
@@ -1582,7 +1624,9 @@ namespace __variant
std::get<0>(std::forward<_Variants>(__variants))...);
}
- template<bool __use_index, typename _Visitor, typename... _Variants>
+ template<bool __use_index,
+ bool __same_return_types,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants)
{
@@ -1592,6 +1636,7 @@ namespace __variant
std::forward<_Variants>(__variants)...));
constexpr auto& __vtable = __detail::__variant::__gen_vtable<
+ __same_return_types,
_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
auto __func_ptr = __vtable._M_access(__variants.index()...);
@@ -1618,12 +1663,9 @@ 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<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#endif
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
index 5eed0cf..28a6e79 100644
--- a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
@@ -42,8 +42,21 @@ test01()
static_assert(std::is_void_v<decltype(std::visit<void>(Visitor{}, v1, v2))>);
}
+void test02()
+{
+ struct Visitor
+ {
+ int operator()(double) {return 42;}
+ double operator()(int) {return 0.02;}
+ };
+ std::variant<int, double> v;
+ std::visit<int>(Visitor(), v);
+}
+
+
int
main()
{
test01();
+ test02();
}
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc
new file mode 100644
index 0000000..ae6fdd1
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <variant>
+#include <testsuite_hooks.h>
+
+// { dg-error "invalid conversion" "" { target *-*-* } 0 }
+// { dg-prune-output "in 'constexpr' expansion" }
+
+void
+test01()
+{
+ {
+ struct Visitor
+ {
+ double operator()(double) {return 0.02;}
+ void operator()(int) {}
+ };
+ std::variant<int, double> v;
+ std::visit(Visitor(), v);
+ }
+}
+
+int
+main()
+{
+ test01();
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-05 23:19 ` Ville Voutilainen
@ 2019-04-05 23:48 ` Ville Voutilainen
2019-04-05 23:56 ` Ville Voutilainen
0 siblings, 1 reply; 15+ messages in thread
From: Ville Voutilainen @ 2019-04-05 23:48 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]
On Sat, 6 Apr 2019 at 02:19, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
> And this patch fixes the existing visitation so that we don't
> over-eagerly cast to void. The main gist of it is
>
> - else if constexpr (is_same_v<_Result_type, void>)
> + else if constexpr (!__same_return_types &&
> + is_same_v<_Result_type, void>)
>
> and the new ensure-test is adjusted to use void as the return type of
> the first visitor.
..and this patch makes cv void work as the return type. The important
bits are (incrementally)
- is_same_v<_Result_type, void>)
+ is_void_v<_Result_type>)
and
- return __do_visit<false, false>(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ return (_Res)
+ __do_visit<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
and the visit_r test is amended to also test visitation of a visitor
with a const void result, when the visitor
itself does not return void.
[-- Attachment #2: variant-visit_r_3.diff --]
[-- Type: text/x-patch, Size: 8772 bytes --]
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf..34f3ec3 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,7 +138,9 @@ 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,
+ bool __same_return_types = true,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
@@ -868,12 +870,15 @@ namespace __variant
// tuple<V1&&, V2&&>, std::index_sequence<1, 2>>
// The returned multi-dimensional vtable can be fast accessed by the visitor
// using index calculation.
- template<typename _Array_type, typename _Variant_tuple, typename _Index_seq>
+ template<bool __same_return_types,
+ typename _Array_type, typename _Variant_tuple, typename _Index_seq>
struct __gen_vtable_impl;
- template<typename _Result_type, typename _Visitor, size_t... __dimensions,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, size_t... __dimensions,
typename... _Variants, size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -915,10 +920,12 @@ namespace __variant
if constexpr (__do_cookie)
{
__element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
*__cookie_element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., variant_npos>>::_S_apply();
@@ -926,15 +933,18 @@ namespace __variant
else
{
__element = __gen_vtable_impl<
+ __same_return_types,
remove_reference_t<decltype(__element)>, tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
}
}
};
- template<typename _Result_type, typename _Visitor, typename... _Variants,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants,
size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -952,25 +962,56 @@ namespace __variant
}
static constexpr decltype(auto)
- __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ __visit_invoke_impl(_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 (!__same_return_types &&
+ 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>(
std::forward<_Variants>(__vars))...);
}
+ static constexpr decltype(auto)
+ __do_visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr _Result_type
+ __do_visit_invoke_r(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr decltype(auto)
+ __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ if constexpr (__same_return_types)
+ return __do_visit_invoke(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ else
+ return __do_visit_invoke_r(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
static constexpr auto
_S_apply()
{ return _Array_type{&__visit_invoke}; }
};
- template<typename _Result_type, typename _Visitor, typename... _Variants>
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants>
struct __gen_vtable
{
using _Func_ptr = _Result_type (*)(_Visitor&&, _Variants...);
@@ -981,7 +1022,8 @@ namespace __variant
static constexpr _Array_type
_S_apply()
{
- return __gen_vtable_impl<_Array_type, tuple<_Variants...>,
+ return __gen_vtable_impl<__same_return_types,
+ _Array_type, tuple<_Variants...>,
std::index_sequence<>>::_S_apply();
}
@@ -1582,7 +1624,9 @@ namespace __variant
std::get<0>(std::forward<_Variants>(__variants))...);
}
- template<bool __use_index, typename _Visitor, typename... _Variants>
+ template<bool __use_index,
+ bool __same_return_types,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants)
{
@@ -1592,6 +1636,7 @@ namespace __variant
std::forward<_Variants>(__variants)...));
constexpr auto& __vtable = __detail::__variant::__gen_vtable<
+ __same_return_types,
_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
auto __func_ptr = __vtable._M_access(__variants.index()...);
@@ -1618,12 +1663,10 @@ 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 (_Res)
+ __do_visit<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#endif
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
index 5eed0cf..55f5f8b 100644
--- a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
@@ -42,8 +42,22 @@ test01()
static_assert(std::is_void_v<decltype(std::visit<void>(Visitor{}, v1, v2))>);
}
+void test02()
+{
+ struct Visitor
+ {
+ int operator()(double) {return 42;}
+ double operator()(int) {return 0.02;}
+ };
+ std::variant<int, double> v;
+ std::visit<int>(Visitor(), v);
+ std::visit<const void>(Visitor(), v);
+}
+
+
int
main()
{
test01();
+ test02();
}
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc
new file mode 100644
index 0000000..ae6fdd1
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <variant>
+#include <testsuite_hooks.h>
+
+// { dg-error "invalid conversion" "" { target *-*-* } 0 }
+// { dg-prune-output "in 'constexpr' expansion" }
+
+void
+test01()
+{
+ {
+ struct Visitor
+ {
+ double operator()(double) {return 0.02;}
+ void operator()(int) {}
+ };
+ std::variant<int, double> v;
+ std::visit(Visitor(), v);
+ }
+}
+
+int
+main()
+{
+ test01();
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-05 23:48 ` Ville Voutilainen
@ 2019-04-05 23:56 ` Ville Voutilainen
2019-04-06 0:07 ` Ville Voutilainen
0 siblings, 1 reply; 15+ messages in thread
From: Ville Voutilainen @ 2019-04-05 23:56 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]
On Sat, 6 Apr 2019 at 02:48, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
> and
>
> - return __do_visit<false, false>(std::forward<_Visitor>(__visitor),
> - std::forward<_Variants>(__variants)...);
> + return (_Res)
> + __do_visit<false, false>(std::forward<_Visitor>(__visitor),
> + std::forward<_Variants>(__variants)...);
>
> and the visit_r test is amended to also test visitation of a visitor
> with a const void result, when the visitor
> itself does not return void.
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.
[-- Attachment #2: variant-visit_r_4.diff --]
[-- Type: text/x-patch, Size: 8919 bytes --]
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf..e5d9da2 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,7 +138,9 @@ 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,
+ bool __same_return_types = true,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
@@ -868,12 +870,15 @@ namespace __variant
// tuple<V1&&, V2&&>, std::index_sequence<1, 2>>
// The returned multi-dimensional vtable can be fast accessed by the visitor
// using index calculation.
- template<typename _Array_type, typename _Variant_tuple, typename _Index_seq>
+ template<bool __same_return_types,
+ typename _Array_type, typename _Variant_tuple, typename _Index_seq>
struct __gen_vtable_impl;
- template<typename _Result_type, typename _Visitor, size_t... __dimensions,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, size_t... __dimensions,
typename... _Variants, size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -915,10 +920,12 @@ namespace __variant
if constexpr (__do_cookie)
{
__element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
*__cookie_element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., variant_npos>>::_S_apply();
@@ -926,15 +933,18 @@ namespace __variant
else
{
__element = __gen_vtable_impl<
+ __same_return_types,
remove_reference_t<decltype(__element)>, tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
}
}
};
- template<typename _Result_type, typename _Visitor, typename... _Variants,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants,
size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -952,25 +962,56 @@ namespace __variant
}
static constexpr decltype(auto)
- __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ __visit_invoke_impl(_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 (!__same_return_types &&
+ 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>(
std::forward<_Variants>(__vars))...);
}
+ static constexpr decltype(auto)
+ __do_visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr _Result_type
+ __do_visit_invoke_r(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr decltype(auto)
+ __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ if constexpr (__same_return_types)
+ return __do_visit_invoke(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ else
+ return __do_visit_invoke_r(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
static constexpr auto
_S_apply()
{ return _Array_type{&__visit_invoke}; }
};
- template<typename _Result_type, typename _Visitor, typename... _Variants>
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants>
struct __gen_vtable
{
using _Func_ptr = _Result_type (*)(_Visitor&&, _Variants...);
@@ -981,7 +1022,8 @@ namespace __variant
static constexpr _Array_type
_S_apply()
{
- return __gen_vtable_impl<_Array_type, tuple<_Variants...>,
+ return __gen_vtable_impl<__same_return_types,
+ _Array_type, tuple<_Variants...>,
std::index_sequence<>>::_S_apply();
}
@@ -1582,7 +1624,9 @@ namespace __variant
std::get<0>(std::forward<_Variants>(__variants))...);
}
- template<bool __use_index, typename _Visitor, typename... _Variants>
+ template<bool __use_index,
+ bool __same_return_types,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants)
{
@@ -1592,6 +1636,7 @@ namespace __variant
std::forward<_Variants>(__variants)...));
constexpr auto& __vtable = __detail::__variant::__gen_vtable<
+ __same_return_types,
_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
auto __func_ptr = __vtable._M_access(__variants.index()...);
@@ -1618,12 +1663,14 @@ 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)...);
+
+ if constexpr (!is_void_v<_Res>)
+ return
+ __do_visit<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
else
- return __do_visit(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ __do_visit<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#endif
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
index 5eed0cf..55f5f8b 100644
--- a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
@@ -42,8 +42,22 @@ test01()
static_assert(std::is_void_v<decltype(std::visit<void>(Visitor{}, v1, v2))>);
}
+void test02()
+{
+ struct Visitor
+ {
+ int operator()(double) {return 42;}
+ double operator()(int) {return 0.02;}
+ };
+ std::variant<int, double> v;
+ std::visit<int>(Visitor(), v);
+ std::visit<const void>(Visitor(), v);
+}
+
+
int
main()
{
test01();
+ test02();
}
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc
new file mode 100644
index 0000000..ae6fdd1
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r_neg.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <variant>
+#include <testsuite_hooks.h>
+
+// { dg-error "invalid conversion" "" { target *-*-* } 0 }
+// { dg-prune-output "in 'constexpr' expansion" }
+
+void
+test01()
+{
+ {
+ struct Visitor
+ {
+ double operator()(double) {return 0.02;}
+ void operator()(int) {}
+ };
+ std::variant<int, double> v;
+ std::visit(Visitor(), v);
+ }
+}
+
+int
+main()
+{
+ test01();
+}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-05 23:56 ` Ville Voutilainen
@ 2019-04-06 0:07 ` Ville Voutilainen
2019-04-08 16:02 ` Jonathan Wakely
0 siblings, 1 reply; 15+ messages in thread
From: Ville Voutilainen @ 2019-04-06 0:07 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]
On Sat, 6 Apr 2019 at 02:55, Ville Voutilainen
<ville.voutilainen@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.voutilainen@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.
[-- Attachment #2: variant-visit_r_5.diff --]
[-- Type: text/x-patch, Size: 8880 bytes --]
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf..7bab472 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,7 +138,9 @@ 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,
+ bool __same_return_types = true,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
@@ -868,12 +870,15 @@ namespace __variant
// tuple<V1&&, V2&&>, std::index_sequence<1, 2>>
// The returned multi-dimensional vtable can be fast accessed by the visitor
// using index calculation.
- template<typename _Array_type, typename _Variant_tuple, typename _Index_seq>
+ template<bool __same_return_types,
+ typename _Array_type, typename _Variant_tuple, typename _Index_seq>
struct __gen_vtable_impl;
- template<typename _Result_type, typename _Visitor, size_t... __dimensions,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, size_t... __dimensions,
typename... _Variants, size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -915,10 +920,12 @@ namespace __variant
if constexpr (__do_cookie)
{
__element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
*__cookie_element = __gen_vtable_impl<
+ __same_return_types,
_Tp,
tuple<_Variants...>,
std::index_sequence<__indices..., variant_npos>>::_S_apply();
@@ -926,15 +933,18 @@ namespace __variant
else
{
__element = __gen_vtable_impl<
+ __same_return_types,
remove_reference_t<decltype(__element)>, tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
}
}
};
- template<typename _Result_type, typename _Visitor, typename... _Variants,
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants,
size_t... __indices>
struct __gen_vtable_impl<
+ __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
tuple<_Variants...>, std::index_sequence<__indices...>>
{
@@ -952,25 +962,56 @@ namespace __variant
}
static constexpr decltype(auto)
- __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ __visit_invoke_impl(_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 (!__same_return_types &&
+ 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>(
std::forward<_Variants>(__vars))...);
}
+ static constexpr decltype(auto)
+ __do_visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr _Result_type
+ __do_visit_invoke_r(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
+ static constexpr decltype(auto)
+ __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
+ {
+ if constexpr (__same_return_types)
+ return __do_visit_invoke(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ else
+ return __do_visit_invoke_r(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__vars)...);
+ }
+
static constexpr auto
_S_apply()
{ return _Array_type{&__visit_invoke}; }
};
- template<typename _Result_type, typename _Visitor, typename... _Variants>
+ template<bool __same_return_types,
+ typename _Result_type, typename _Visitor, typename... _Variants>
struct __gen_vtable
{
using _Func_ptr = _Result_type (*)(_Visitor&&, _Variants...);
@@ -981,7 +1022,8 @@ namespace __variant
static constexpr _Array_type
_S_apply()
{
- return __gen_vtable_impl<_Array_type, tuple<_Variants...>,
+ return __gen_vtable_impl<__same_return_types,
+ _Array_type, tuple<_Variants...>,
std::index_sequence<>>::_S_apply();
}
@@ -1582,7 +1624,9 @@ namespace __variant
std::get<0>(std::forward<_Variants>(__variants))...);
}
- template<bool __use_index, typename _Visitor, typename... _Variants>
+ template<bool __use_index,
+ bool __same_return_types,
+ typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants)
{
@@ -1592,6 +1636,7 @@ namespace __variant
std::forward<_Variants>(__variants)...));
constexpr auto& __vtable = __detail::__variant::__gen_vtable<
+ __same_return_types,
_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
auto __func_ptr = __vtable._M_access(__variants.index()...);
@@ -1618,12 +1663,13 @@ 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)...);
+ (void) __do_visit<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
else
- return __do_visit(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ return __do_visit<false, false>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#endif
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc b/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc
new file mode 100644
index 0000000..ae6fdd1
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <variant>
+#include <testsuite_hooks.h>
+
+// { dg-error "invalid conversion" "" { target *-*-* } 0 }
+// { dg-prune-output "in 'constexpr' expansion" }
+
+void
+test01()
+{
+ {
+ struct Visitor
+ {
+ double operator()(double) {return 0.02;}
+ void operator()(int) {}
+ };
+ std::variant<int, double> v;
+ std::visit(Visitor(), v);
+ }
+}
+
+int
+main()
+{
+ test01();
+}
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
index 5eed0cf..55f5f8b 100644
--- a/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_r.cc
@@ -42,8 +42,22 @@ test01()
static_assert(std::is_void_v<decltype(std::visit<void>(Visitor{}, v1, v2))>);
}
+void test02()
+{
+ struct Visitor
+ {
+ int operator()(double) {return 42;}
+ double operator()(int) {return 0.02;}
+ };
+ std::variant<int, double> v;
+ std::visit<int>(Visitor(), v);
+ std::visit<const void>(Visitor(), v);
+}
+
+
int
main()
{
test01();
+ test02();
}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-06 0:07 ` Ville Voutilainen
@ 2019-04-08 16:02 ` Jonathan Wakely
2019-04-08 16:12 ` Ville Voutilainen
2019-04-08 16:23 ` Jonathan Wakely
0 siblings, 2 replies; 15+ messages in thread
From: Jonathan Wakely @ 2019-04-08 16:02 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]
On 06/04/19 03:07 +0300, Ville Voutilainen wrote:
>On Sat, 6 Apr 2019 at 02:55, Ville Voutilainen
><ville.voutilainen@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.voutilainen@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?
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 7385 bytes --]
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-08 16:02 ` Jonathan Wakely
@ 2019-04-08 16:12 ` Ville Voutilainen
2019-04-08 16:20 ` Ville Voutilainen
2019-04-08 16:23 ` Jonathan Wakely
1 sibling, 1 reply; 15+ messages in thread
From: Ville Voutilainen @ 2019-04-08 16:12 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely <jwakely@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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-08 16:12 ` Ville Voutilainen
@ 2019-04-08 16:20 ` Ville Voutilainen
2019-04-08 16:36 ` Jonathan Wakely
0 siblings, 1 reply; 15+ messages in thread
From: Ville Voutilainen @ 2019-04-08 16:20 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
>
> On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely <jwakely@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.
The latter is already a recursive template, with this patch we have
two of them. I knew that I could split the hierarchy
for <R> and non-<R>, but I instinctively went with a pass-through
template parameter that I then handled
in __visit_invoke, so as to specifically avoid splitting the recursive
template into two recursive templates.
That recursive template and how it recurses is not something we are
going to change often, or really reason about
often. But it's something that I need to rack my brain over every
time, whereas looking at an if-constexpr is simple as pie
to me. I suppose there are different levels of familiarity/comfort here.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-08 16:02 ` Jonathan Wakely
2019-04-08 16:12 ` Ville Voutilainen
@ 2019-04-08 16:23 ` Jonathan Wakely
1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2019-04-08 16:23 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: libstdc++, gcc-patches List
On 08/04/19 17:02 +0100, Jonathan Wakely wrote:
>+ 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{};
>+ }
I don't think we need __element_by_index_or_cookie for the visit<R>
case, so the function above can go, and ...
>+ 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>(
this just becomes __variant::__get<__indices>(
>+ std::forward<_Variants>(__vars))...);
>+ else
>+ return std::__invoke(std::forward<_Visitor>(__visitor),
>+ __element_by_index_or_cookie<__indices>(
and here too.
>+ std::forward<_Variants>(__vars))...);
>+ }
>+
>+ static constexpr _Array_type
>+ _S_apply()
>+ { return _Array_type{&__visit_r_invoke}; }
>+ };
With the above simplification the visit<R> code is a lot simpler than
the existing __gen_vtable code, let alone the version that adds
visit<R> support to it.
I'll spend some more time thinking about it.
>+ 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();
N.B. here I just declare a variable and its default initializer. I
don't see any benefit to how the original in __gen_vtable does it:
static constexpr _Array_type
_S_apply()
{
return __gen_vtable_impl<_Array_type, tuple<_Variants...>,
std::index_sequence<>>::_S_apply();
}
static constexpr auto _S_vtable = _S_apply();
We only ever use that function in the default initializer, so might as
well get rid of it.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-08 16:20 ` Ville Voutilainen
@ 2019-04-08 16:36 ` Jonathan Wakely
2019-04-08 18:54 ` Jonathan Wakely
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Wakely @ 2019-04-08 16:36 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: libstdc++, gcc-patches List
On 08/04/19 19:20 +0300, Ville Voutilainen wrote:
>On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen
><ville.voutilainen@gmail.com> wrote:
>>
>> On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely <jwakely@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.
>The latter is already a recursive template, with this patch we have
>two of them. I knew that I could split the hierarchy
>for <R> and non-<R>, but I instinctively went with a pass-through
>template parameter that I then handled
>in __visit_invoke, so as to specifically avoid splitting the recursive
>template into two recursive templates.
>
>That recursive template and how it recurses is not something we are
>going to change often, or really reason about
>often. But it's something that I need to rack my brain over every
>time, whereas looking at an if-constexpr is simple as pie
>to me. I suppose there are different levels of familiarity/comfort here.
The if-constexpr itself is easy to understand, but how we get from
__do_visit to a specific specialization of a recursive class template
and a specific if-constexpr condition is the hard part for me to
follow.
But now that I've wrapped my head around the existing code, I'll spend
some more time getting acquainted with your proposed changes and see
if they grow on me overnight.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-08 16:36 ` Jonathan Wakely
@ 2019-04-08 18:54 ` Jonathan Wakely
2019-04-08 18:56 ` Jonathan Wakely
2019-04-10 21:27 ` Jonathan Wakely
0 siblings, 2 replies; 15+ messages in thread
From: Jonathan Wakely @ 2019-04-08 18:54 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: libstdc++, gcc-patches List
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.voutilainen@gmail.com> wrote:
>>>
>>>On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely <jwakely@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
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-08 18:54 ` Jonathan Wakely
@ 2019-04-08 18:56 ` Jonathan Wakely
2019-04-10 21:27 ` Jonathan Wakely
1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2019-04-08 18:56 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]
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.voutilainen@gmail.com> wrote:
>>>>
>>>>On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely <jwakely@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.
>
>
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 7933 bytes --]
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)>;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
2019-04-08 18:54 ` Jonathan Wakely
2019-04-08 18:56 ` Jonathan Wakely
@ 2019-04-10 21:27 ` Jonathan Wakely
1 sibling, 0 replies; 15+ messages in thread
From: Jonathan Wakely @ 2019-04-10 21:27 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]
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.voutilainen@gmail.com> wrote:
>>>>
>>>>On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely <jwakely@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
>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.
Here's a version that doesn't have that problem.
This adds std::__invoke_r to implement INVOKE<R>, and then uses it in
std::bind, std::function, std::packaged_task and std::visit<R>. Having
a helper for INVOKE<R> simplifies a number of things, including the
visitation code in <variant>. It also means that we'll only need to
change one place to implement https://wg21.link/p0932r0 once a revised
version of that gets accepted into the draft.
I'll commit this (in a series of patches) in stage 1.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 30913 bytes --]
diff --git a/libstdc++-v3/include/bits/invoke.h b/libstdc++-v3/include/bits/invoke.h
index a5278a59f0c..59e22da84d4 100644
--- a/libstdc++-v3/include/bits/invoke.h
+++ b/libstdc++-v3/include/bits/invoke.h
@@ -96,6 +96,65 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
std::forward<_Args>(__args)...);
}
+#if __cplusplus >= 201703L
+ // INVOKE<R>: Invoke a callable object and convert the result to R.
+ template<typename _Res, typename _Callable, typename... _Args>
+ constexpr enable_if_t<is_invocable_r_v<_Res, _Callable, _Args...>, _Res>
+ __invoke_r(_Callable&& __fn, _Args&&... __args)
+ noexcept(is_nothrow_invocable_r_v<_Res, _Callable, _Args...>)
+ {
+ using __result = __invoke_result<_Callable, _Args...>;
+ using __type = typename __result::type;
+ using __tag = typename __result::__invoke_type;
+ if constexpr (is_void_v<_Res>)
+ std::__invoke_impl<__type>(__tag{}, std::forward<_Callable>(__fn),
+ std::forward<_Args>(__args)...);
+ else
+ return std::__invoke_impl<__type>(__tag{},
+ std::forward<_Callable>(__fn),
+ std::forward<_Args>(__args)...);
+ }
+#else // C++11
+ template<typename _Res, typename _Callable, typename... _Args>
+ using __can_invoke_as_void = __enable_if_t<
+ __and_<is_void<_Res>, __is_invocable<_Callable, _Args...>>::value,
+ _Res
+ >;
+
+ template<typename _Res, typename _Callable, typename... _Args>
+ using __can_invoke_as_nonvoid = __enable_if_t<
+ __and_<__not_<is_void<_Res>>,
+ is_convertible<typename __invoke_result<_Callable, _Args...>::type,
+ _Res>
+ >::value,
+ _Res
+ >;
+
+ // INVOKE<R>: Invoke a callable object and convert the result to R.
+ template<typename _Res, typename _Callable, typename... _Args>
+ constexpr __can_invoke_as_nonvoid<_Res, _Callable, _Args...>
+ __invoke_r(_Callable&& __fn, _Args&&... __args)
+ {
+ using __result = __invoke_result<_Callable, _Args...>;
+ using __type = typename __result::type;
+ using __tag = typename __result::__invoke_type;
+ return std::__invoke_impl<__type>(__tag{}, std::forward<_Callable>(__fn),
+ std::forward<_Args>(__args)...);
+ }
+
+ // INVOKE<R> when R is cv void
+ template<typename _Res, typename _Callable, typename... _Args>
+ constexpr __can_invoke_as_void<_Res, _Callable, _Args...>
+ __invoke_r(_Callable&& __fn, _Args&&... __args)
+ {
+ using __result = __invoke_result<_Callable, _Args...>;
+ using __type = typename __result::type;
+ using __tag = typename __result::__invoke_type;
+ std::__invoke_impl<__type>(__tag{}, std::forward<_Callable>(__fn),
+ std::forward<_Args>(__args)...);
+ }
+#endif // C++11
+
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace std
diff --git a/libstdc++-v3/include/bits/std_function.h b/libstdc++-v3/include/bits/std_function.h
index b70ed564d11..5733bf5f3f9 100644
--- a/libstdc++-v3/include/bits/std_function.h
+++ b/libstdc++-v3/include/bits/std_function.h
@@ -109,21 +109,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__destroy_functor
};
- // Simple type wrapper that helps avoid annoying const problems
- // when casting between void pointers and pointers-to-pointers.
- template<typename _Tp>
- struct _Simple_type_wrapper
- {
- _Simple_type_wrapper(_Tp __value) : __value(__value) { }
-
- _Tp __value;
- };
-
- template<typename _Tp>
- struct __is_location_invariant<_Simple_type_wrapper<_Tp> >
- : __is_location_invariant<_Tp>
- { };
-
template<typename _Signature>
class function;
@@ -278,56 +263,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
typedef _Function_base::_Base_manager<_Functor> _Base;
- public:
- static _Res
- _M_invoke(const _Any_data& __functor, _ArgTypes&&... __args)
- {
- return (*_Base::_M_get_pointer(__functor))(
- std::forward<_ArgTypes>(__args)...);
- }
- };
-
- template<typename _Functor, typename... _ArgTypes>
- class _Function_handler<void(_ArgTypes...), _Functor>
- : public _Function_base::_Base_manager<_Functor>
- {
- typedef _Function_base::_Base_manager<_Functor> _Base;
-
- public:
- static void
- _M_invoke(const _Any_data& __functor, _ArgTypes&&... __args)
- {
- (*_Base::_M_get_pointer(__functor))(
- std::forward<_ArgTypes>(__args)...);
- }
- };
-
- template<typename _Class, typename _Member, typename _Res,
- typename... _ArgTypes>
- class _Function_handler<_Res(_ArgTypes...), _Member _Class::*>
- : public _Function_handler<void(_ArgTypes...), _Member _Class::*>
- {
- typedef _Function_handler<void(_ArgTypes...), _Member _Class::*>
- _Base;
-
- public:
- static _Res
- _M_invoke(const _Any_data& __functor, _ArgTypes&&... __args)
- {
- return std::__invoke(_Base::_M_get_pointer(__functor)->__value,
- std::forward<_ArgTypes>(__args)...);
- }
- };
-
- template<typename _Class, typename _Member, typename... _ArgTypes>
- class _Function_handler<void(_ArgTypes...), _Member _Class::*>
- : public _Function_base::_Base_manager<
- _Simple_type_wrapper< _Member _Class::* > >
- {
- typedef _Member _Class::* _Functor;
- typedef _Simple_type_wrapper<_Functor> _Wrapper;
- typedef _Function_base::_Base_manager<_Wrapper> _Base;
-
public:
static bool
_M_manager(_Any_data& __dest, const _Any_data& __source,
@@ -341,8 +276,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
break;
#endif
case __get_functor_ptr:
- __dest._M_access<_Functor*>() =
- &_Base::_M_get_pointer(__source)->__value;
+ __dest._M_access<_Functor*>() = _Base::_M_get_pointer(__source);
break;
default:
@@ -351,11 +285,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return false;
}
- static void
+ static _Res
_M_invoke(const _Any_data& __functor, _ArgTypes&&... __args)
{
- std::__invoke(_Base::_M_get_pointer(__functor)->__value,
- std::forward<_ArgTypes>(__args)...);
+ return std::__invoke_r<_Res>(*_Base::_M_get_pointer(__functor),
+ std::forward<_ArgTypes>(__args)...);
}
};
diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index 8cf2c670648..1834ed8ca4d 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -550,78 +550,41 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Call unqualified
template<typename _Res, typename... _Args, std::size_t... _Indexes>
- __disable_if_void<_Res>
+ _Res
__call(tuple<_Args...>&& __args, _Index_tuple<_Indexes...>)
{
- return std::__invoke(_M_f, _Mu<_Bound_args>()
+ return std::__invoke_r<_Res>(_M_f, _Mu<_Bound_args>()
(std::get<_Indexes>(_M_bound_args), __args)...);
}
- // Call unqualified, return void
- template<typename _Res, typename... _Args, std::size_t... _Indexes>
- __enable_if_void<_Res>
- __call(tuple<_Args...>&& __args, _Index_tuple<_Indexes...>)
- {
- std::__invoke(_M_f, _Mu<_Bound_args>()
- (std::get<_Indexes>(_M_bound_args), __args)...);
- }
-
// Call as const
template<typename _Res, typename... _Args, std::size_t... _Indexes>
- __disable_if_void<_Res>
+ _Res
__call(tuple<_Args...>&& __args, _Index_tuple<_Indexes...>) const
{
- return std::__invoke(_M_f, _Mu<_Bound_args>()
+ return std::__invoke_r<_Res>(_M_f, _Mu<_Bound_args>()
(std::get<_Indexes>(_M_bound_args), __args)...);
}
- // Call as const, return void
- template<typename _Res, typename... _Args, std::size_t... _Indexes>
- __enable_if_void<_Res>
- __call(tuple<_Args...>&& __args, _Index_tuple<_Indexes...>) const
- {
- std::__invoke(_M_f, _Mu<_Bound_args>()
- (std::get<_Indexes>(_M_bound_args), __args)...);
- }
-
// Call as volatile
template<typename _Res, typename... _Args, std::size_t... _Indexes>
- __disable_if_void<_Res>
+ _Res
__call(tuple<_Args...>&& __args, _Index_tuple<_Indexes...>) volatile
{
- return std::__invoke(_M_f, _Mu<_Bound_args>()
+ return std::__invoke_r<_Res>(_M_f, _Mu<_Bound_args>()
(__volget<_Indexes>(_M_bound_args), __args)...);
}
- // Call as volatile, return void
- template<typename _Res, typename... _Args, std::size_t... _Indexes>
- __enable_if_void<_Res>
- __call(tuple<_Args...>&& __args, _Index_tuple<_Indexes...>) volatile
- {
- std::__invoke(_M_f, _Mu<_Bound_args>()
- (__volget<_Indexes>(_M_bound_args), __args)...);
- }
-
// Call as const volatile
template<typename _Res, typename... _Args, std::size_t... _Indexes>
- __disable_if_void<_Res>
+ _Res
__call(tuple<_Args...>&& __args,
_Index_tuple<_Indexes...>) const volatile
{
- return std::__invoke(_M_f, _Mu<_Bound_args>()
+ return std::__invoke_r<_Res>(_M_f, _Mu<_Bound_args>()
(__volget<_Indexes>(_M_bound_args), __args)...);
}
- // Call as const volatile, return void
- template<typename _Res, typename... _Args, std::size_t... _Indexes>
- __enable_if_void<_Res>
- __call(tuple<_Args...>&& __args,
- _Index_tuple<_Indexes...>) const volatile
- {
- std::__invoke(_M_f, _Mu<_Bound_args>()
- (__volget<_Indexes>(_M_bound_args), __args)...);
- }
-
public:
typedef _Result result_type;
diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index 10136e57a84..967110050b8 100644
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -1417,8 +1417,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
virtual void
_M_run(_Args&&... __args)
{
- auto __boundfn = [&] () -> typename result_of<_Fn&(_Args&&...)>::type {
- return std::__invoke(_M_impl._M_fn, std::forward<_Args>(__args)...);
+ auto __boundfn = [&] () -> _Res {
+ return std::__invoke_r<_Res>(_M_impl._M_fn,
+ std::forward<_Args>(__args)...);
};
this->_M_set_result(_S_task_setter(this->_M_result, __boundfn));
}
@@ -1426,8 +1427,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
virtual void
_M_run_delayed(_Args&&... __args, weak_ptr<_State_base> __self)
{
- auto __boundfn = [&] () -> typename result_of<_Fn&(_Args&&...)>::type {
- return std::__invoke(_M_impl._M_fn, std::forward<_Args>(__args)...);
+ auto __boundfn = [&] () -> _Res {
+ return std::__invoke_r<_Res>(_M_impl._M_fn,
+ std::forward<_Args>(__args)...);
};
this->_M_set_delayed_result(_S_task_setter(this->_M_result, __boundfn),
std::move(__self));
diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index c3cb67a457d..a1651650020 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -1378,7 +1378,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: public __is_convertible_helper<_From, _To>::type
{ };
-#if __cplusplus > 201703L
template<typename _From, typename _To,
bool = __or_<is_void<_From>, is_function<_To>,
is_array<_To>>::value>
@@ -1393,7 +1392,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static void __test_aux(_To1) noexcept;
template<typename _From1, typename _To1>
- static bool_constant<noexcept(__test_aux<_To1>(std::declval<_From1>()))>
+ static
+ __bool_constant<noexcept(__test_aux<_To1>(std::declval<_From1>()))>
__test(int);
template<typename, typename>
@@ -1404,6 +1404,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
using type = decltype(__test<_From, _To>(0));
};
+ // is_nothrow_convertible for C++11
+ template<typename _From, typename _To>
+ struct __is_nothrow_convertible
+ : public __is_nt_convertible_helper<_From, _To>::type
+ { };
+
+#if __cplusplus > 201703L
/// is_nothrow_convertible
template<typename _From, typename _To>
struct is_nothrow_convertible
@@ -2816,8 +2823,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
struct __is_nt_invocable_impl<_Result, _Ret,
__void_t<typename _Result::type>>
: __or_<is_void<_Ret>,
- __and_<is_convertible<typename _Result::type, _Ret>,
- is_nothrow_constructible<_Ret, typename _Result::type>>>
+ __is_nothrow_convertible<typename _Result::type, _Ret>>
{ };
/// std::is_nothrow_invocable_r
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 22b0c3d5c22..e38830d656d 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,9 +138,7 @@ namespace __variant
constexpr variant_alternative_t<_Np, variant<_Types...>> const&&
get(const variant<_Types...>&&);
- template<bool __use_index=false,
- bool __same_return_types = true,
- typename _Visitor, typename... _Variants>
+ template<typename _Result_type, typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
@@ -180,8 +178,26 @@ namespace __variant
struct __variant_cookie {};
// used for raw visitation with indices passed in
struct __variant_idx_cookie { using type = __variant_idx_cookie; };
- // a more explanatory name than 'true'
- inline constexpr auto __visit_with_index = bool_constant<true>{};
+ // Used to enable deduction (and same-type checking) for std::visit:
+ template<typename> struct __deduce_visit_result { };
+
+ // Visit variants that might be valueless.
+ template<typename _Visitor, typename... _Variants>
+ constexpr void
+ __raw_visit(_Visitor&& __visitor, _Variants&&... __variants)
+ {
+ std::__do_visit<__variant_cookie>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
+ }
+
+ // Visit variants that might be valueless, passing indices to the visitor.
+ template<typename _Visitor, typename... _Variants>
+ constexpr void
+ __raw_idx_visit(_Visitor&& __visitor, _Variants&&... __variants)
+ {
+ std::__do_visit<__variant_idx_cookie>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
+ }
// _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
@@ -381,13 +397,11 @@ namespace __variant
constexpr void _M_reset_impl()
{
- __do_visit([](auto&& __this_mem) mutable
- -> __detail::__variant::__variant_cookie
+ __variant::__raw_visit([](auto&& __this_mem) mutable
{
if constexpr (!is_same_v<remove_reference_t<decltype(__this_mem)>,
__variant_cookie>)
std::_Destroy(std::__addressof(__this_mem));
- return {};
}, __variant_cast<_Types...>(*this));
}
@@ -470,12 +484,10 @@ namespace __variant
void __variant_construct(_Tp&& __lhs, _Up&& __rhs)
{
__lhs._M_index = __rhs._M_index;
- __do_visit([&__lhs](auto&& __rhs_mem) mutable
- -> __detail::__variant::__variant_cookie
+ __variant::__raw_visit([&__lhs](auto&& __rhs_mem) mutable
{
__variant_construct_single(std::forward<_Tp>(__lhs),
std::forward<decltype(__rhs_mem)>( __rhs_mem));
- return {};
}, __variant_cast<_Types...>(std::forward<decltype(__rhs)>(__rhs)));
}
@@ -599,9 +611,8 @@ namespace __variant
operator=(const _Copy_assign_base& __rhs)
noexcept(_Traits<_Types...>::_S_nothrow_copy_assign)
{
- __do_visit<__visit_with_index>([this](auto&& __rhs_mem,
- auto __rhs_index) mutable
- -> __detail::__variant::__variant_idx_cookie
+ __variant::__raw_idx_visit(
+ [this](auto&& __rhs_mem, auto __rhs_index) mutable
{
if constexpr (__rhs_index != variant_npos)
{
@@ -635,7 +646,6 @@ namespace __variant
}
else
this->_M_reset();
- return {};
}, __variant_cast<_Types...>(__rhs));
return *this;
}
@@ -668,9 +678,8 @@ namespace __variant
operator=(_Move_assign_base&& __rhs)
noexcept(_Traits<_Types...>::_S_nothrow_move_assign)
{
- __do_visit<__visit_with_index>([this](auto&& __rhs_mem,
- auto __rhs_index) mutable
- -> __detail::__variant::__variant_idx_cookie
+ __variant::__raw_idx_visit(
+ [this](auto&& __rhs_mem, auto __rhs_index) mutable
{
if constexpr (__rhs_index != variant_npos)
{
@@ -692,7 +701,6 @@ namespace __variant
}
else
this->_M_reset();
- return {};
}, __variant_cast<_Types...>(__rhs));
return *this;
}
@@ -822,11 +830,38 @@ namespace __variant
template<typename _Tp>
struct _Multi_array<_Tp>
{
- constexpr const _Tp&
+ template<typename>
+ struct __untag_result
+ : false_type
+ { using element_type = _Tp; };
+
+ template <typename... _Args>
+ struct __untag_result<const void(*)(_Args...)>
+ : false_type
+ { using element_type = void(*)(_Args...); };
+
+ template <typename... _Args>
+ struct __untag_result<__variant_cookie(*)(_Args...)>
+ : false_type
+ { using element_type = void(*)(_Args...); };
+
+ template <typename... _Args>
+ struct __untag_result<__variant_idx_cookie(*)(_Args...)>
+ : false_type
+ { using element_type = void(*)(_Args...); };
+
+ template <typename _Res, typename... _Args>
+ struct __untag_result<__deduce_visit_result<_Res>(*)(_Args...)>
+ : true_type
+ { using element_type = _Res(*)(_Args...); };
+
+ using __result_is_deduced = __untag_result<_Tp>;
+
+ constexpr const typename __untag_result<_Tp>::element_type&
_M_access() const
{ return _M_data; }
- _Tp _M_data;
+ typename __untag_result<_Tp>::element_type _M_data;
};
// Partial specialization with rank >= 1.
@@ -847,7 +882,7 @@ namespace __variant
using _Tp = _Ret(*)(_Visitor, _Variants...);
template<typename... _Args>
- constexpr const _Tp&
+ constexpr decltype(auto)
_M_access(size_t __first_index, _Args... __rest_indices) const
{
return _M_arr[__first_index + __do_cookie]
@@ -859,37 +894,32 @@ namespace __variant
// Creates a multi-dimensional vtable recursively.
//
- // The __same_return_types non-type template parameter specifies whether
- // to enforce that all visitor invocations return the same type. This is
- // required by std::visit but not std::visit<R>.
- //
// For example,
// visit([](auto, auto){},
// variant<int, char>(), // typedef'ed as V1
// variant<float, double, long double>()) // typedef'ed as V2
// will trigger instantiations of:
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&), 2, 3>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&), 2, 3>,
// tuple<V1&&, V2&&>, std::index_sequence<>>
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&), 3>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&), 3>,
// tuple<V1&&, V2&&>, std::index_sequence<0>>
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&)>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&)>,
// tuple<V1&&, V2&&>, std::index_sequence<0, 0>>
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&)>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&)>,
// tuple<V1&&, V2&&>, std::index_sequence<0, 1>>
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&)>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&)>,
// tuple<V1&&, V2&&>, std::index_sequence<0, 2>>
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&), 3>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&), 3>,
// tuple<V1&&, V2&&>, std::index_sequence<1>>
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&)>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&)>,
// tuple<V1&&, V2&&>, std::index_sequence<1, 0>>
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&)>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&)>,
// tuple<V1&&, V2&&>, std::index_sequence<1, 1>>
- // __gen_vtable_impl<true, _Multi_array<void(*)(V1&&, V2&&)>,
+ // __gen_vtable_impl<_Multi_array<void(*)(V1&&, V2&&)>,
// tuple<V1&&, V2&&>, std::index_sequence<1, 2>>
// The returned multi-dimensional vtable can be fast accessed by the visitor
// using index calculation.
- template<bool __same_return_types,
- typename _Array_type, typename _Variant_tuple, typename _Index_seq>
+ template<typename _Array_type, typename _Index_seq>
struct __gen_vtable_impl;
// Defines the _S_apply() member that returns a _Multi_array populated
@@ -899,13 +929,11 @@ namespace __variant
// This partial specialization builds up the index sequences by recursively
// calling _S_apply() on the next specialization of __gen_vtable_impl.
// The base case of the recursion defines the actual function pointers.
- template<bool __same_return_types,
- typename _Result_type, typename _Visitor, size_t... __dimensions,
+ template<typename _Result_type, typename _Visitor, size_t... __dimensions,
typename... _Variants, size_t... __indices>
struct __gen_vtable_impl<
- __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
- tuple<_Variants...>, std::index_sequence<__indices...>>
+ std::index_sequence<__indices...>>
{
using _Next =
remove_reference_t<typename _Nth_type<sizeof...(__indices),
@@ -941,25 +969,19 @@ namespace __variant
static constexpr void
_S_apply_single_alt(_Tp& __element, _Tp* __cookie_element = nullptr)
{
- using _Alternative = variant_alternative_t<__index, _Next>;
if constexpr (__do_cookie)
{
__element = __gen_vtable_impl<
- __same_return_types,
_Tp,
- tuple<_Variants...>,
std::index_sequence<__indices..., __index>>::_S_apply();
*__cookie_element = __gen_vtable_impl<
- __same_return_types,
_Tp,
- tuple<_Variants...>,
std::index_sequence<__indices..., variant_npos>>::_S_apply();
}
else
{
__element = __gen_vtable_impl<
- __same_return_types,
- remove_reference_t<decltype(__element)>, tuple<_Variants...>,
+ remove_reference_t<decltype(__element)>,
std::index_sequence<__indices..., __index>>::_S_apply();
}
}
@@ -968,13 +990,11 @@ namespace __variant
// This partial specialization is the base case for the recursion.
// It populates a _Multi_array element with the address of a function
// that invokes the visitor with the alternatives specified by __indices.
- template<bool __same_return_types,
- typename _Result_type, typename _Visitor, typename... _Variants,
+ template<typename _Result_type, typename _Visitor, typename... _Variants,
size_t... __indices>
struct __gen_vtable_impl<
- __same_return_types,
_Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
- tuple<_Variants...>, std::index_sequence<__indices...>>
+ std::index_sequence<__indices...>>
{
using _Array_type =
_Multi_array<_Result_type (*)(_Visitor, _Variants...)>;
@@ -989,51 +1009,38 @@ namespace __variant
return __variant_cookie{};
}
+ // Perform the implicit conversion to _Result_type for std::visit<R>.
+ static constexpr _Result_type
+ __do_visit_invoke_r(_Visitor&& __visitor, _Variants... __vars)
+ {
+ return std::__invoke(std::forward<_Visitor>(__visitor),
+ __variant::__get<__indices>(std::forward<_Variants>(__vars))...);
+ }
+
static constexpr decltype(auto)
- __visit_invoke_impl(_Visitor&& __visitor, _Variants... __vars)
+ __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
{
- // For raw visitation using indices, pass the indices to the visitor:
if constexpr (is_same_v<_Result_type, __variant_idx_cookie>)
- return std::__invoke(std::forward<_Visitor>(__visitor),
+ // For raw visitation using indices, pass the indices to the visitor
+ // and discard the return value:
+ std::__invoke(std::forward<_Visitor>(__visitor),
__element_by_index_or_cookie<__indices>(
std::forward<_Variants>(__vars))...,
integral_constant<size_t, __indices>()...);
- // For std::visit<cv void>, cast the result to void:
- else if constexpr (!__same_return_types &&
- std::is_void_v<_Result_type>)
- return (void)std::__invoke(std::forward<_Visitor>(__visitor),
+ else if constexpr (is_same_v<_Result_type, __variant_cookie>)
+ // For raw visitation without indices, and discard the return value:
+ std::__invoke(std::forward<_Visitor>(__visitor),
__element_by_index_or_cookie<__indices>(
std::forward<_Variants>(__vars))...);
- else
+ else if constexpr (_Array_type::__result_is_deduced::value)
+ // For the usual std::visit case deduce the return value:
return std::__invoke(std::forward<_Visitor>(__visitor),
__element_by_index_or_cookie<__indices>(
std::forward<_Variants>(__vars))...);
- }
-
- static constexpr decltype(auto)
- __do_visit_invoke(_Visitor&& __visitor, _Variants... __vars)
- {
- return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__vars)...);
- }
-
- // Perform the implicit conversion to _Result_type for std::visit<R>.
- static constexpr _Result_type
- __do_visit_invoke_r(_Visitor&& __visitor, _Variants... __vars)
- {
- return __visit_invoke_impl(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__vars)...);
- }
-
- static constexpr decltype(auto)
- __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
- {
- if constexpr (__same_return_types)
- return __do_visit_invoke(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__vars)...);
- else
- return __do_visit_invoke_r(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__vars)...);
+ else // for std::visit<R> use INVOKE<R>
+ return std::__invoke_r<_Result_type>(
+ std::forward<_Visitor>(__visitor),
+ __variant::__get<__indices>(std::forward<_Variants>(__vars))...);
}
static constexpr auto
@@ -1041,8 +1048,7 @@ namespace __variant
{ return _Array_type{&__visit_invoke}; }
};
- template<bool __same_return_types,
- typename _Result_type, typename _Visitor, typename... _Variants>
+ template<typename _Result_type, typename _Visitor, typename... _Variants>
struct __gen_vtable
{
using _Array_type =
@@ -1050,9 +1056,7 @@ namespace __variant
variant_size_v<remove_reference_t<_Variants>>...>;
static constexpr _Array_type _S_vtable
- = __gen_vtable_impl<__same_return_types,
- _Array_type, tuple<_Variants...>,
- std::index_sequence<>>::_S_apply();
+ = __gen_vtable_impl<_Array_type, std::index_sequence<>>::_S_apply();
};
template<size_t _Np, typename _Tp>
@@ -1184,10 +1188,8 @@ namespace __variant
const variant<_Types...>& __rhs) \
{ \
bool __ret = true; \
- __do_visit<__detail::__variant::__visit_with_index>( \
- [&__ret, &__lhs] \
- (auto&& __rhs_mem, auto __rhs_index) mutable \
- -> __detail::__variant::__variant_idx_cookie \
+ __detail::__variant::__raw_idx_visit( \
+ [&__ret, &__lhs] (auto&& __rhs_mem, auto __rhs_index) mutable \
{ \
if constexpr (__rhs_index != variant_npos) \
{ \
@@ -1201,7 +1203,6 @@ namespace __variant
} \
else \
__ret = (__lhs.index() + 1) __OP (__rhs_index + 1); \
- return {}; \
}, __rhs); \
return __ret; \
} \
@@ -1521,10 +1522,8 @@ namespace __variant
noexcept((__is_nothrow_swappable<_Types>::value && ...)
&& is_nothrow_move_constructible_v<variant>)
{
- __do_visit<__detail::__variant::__visit_with_index>(
- [this, &__rhs](auto&& __rhs_mem,
- auto __rhs_index) mutable
- -> __detail::__variant::__variant_idx_cookie
+ __detail::__variant::__raw_idx_visit(
+ [this, &__rhs](auto&& __rhs_mem, auto __rhs_index) mutable
{
if constexpr (__rhs_index != variant_npos)
{
@@ -1560,7 +1559,6 @@ namespace __variant
this->_M_reset();
}
}
- return {};
}, __rhs);
}
@@ -1638,21 +1636,11 @@ namespace __variant
return __detail::__variant::__get<_Np>(std::move(__v));
}
- template<bool __use_index,
- bool __same_return_types,
- typename _Visitor, typename... _Variants>
+ template<typename _Result_type, typename _Visitor, typename... _Variants>
constexpr decltype(auto)
__do_visit(_Visitor&& __visitor, _Variants&&... __variants)
{
- using _Deduced_type = std::invoke_result<_Visitor,
- decltype(std::get<0>(std::declval<_Variants>()))...>;
-
- using _Result_type = typename std::conditional_t<__use_index,
- __detail::__variant::__variant_idx_cookie,
- _Deduced_type>::type;
-
constexpr auto& __vtable = __detail::__variant::__gen_vtable<
- __same_return_types,
_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
auto __func_ptr = __vtable._M_access(__variants.index()...);
@@ -1667,8 +1655,13 @@ 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 = std::invoke_result_t<_Visitor,
+ decltype(std::get<0>(std::declval<_Variants>()))...>;
+
+ using _Tag = __detail::__variant::__deduce_visit_result<_Result_type>;
+
+ return __do_visit<_Tag>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#if __cplusplus > 201703L
@@ -1679,12 +1672,8 @@ namespace __variant
if ((__variants.valueless_by_exception() || ...))
__throw_bad_variant_access("Unexpected index");
- if constexpr (std::is_void_v<_Res>)
- (void) __do_visit<false, false>(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
- else
- return __do_visit<false, false>(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ return __do_visit<_Res>(std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#endif
@@ -1696,8 +1685,8 @@ namespace __variant
noexcept((is_nothrow_invocable_v<hash<decay_t<_Types>>, _Types> && ...))
{
size_t __ret;
- __do_visit([&__t, &__ret](auto&& __t_mem) mutable
- -> __detail::__variant::__variant_cookie
+ __detail::__variant::__raw_visit(
+ [&__t, &__ret](auto&& __t_mem) mutable
{
using _Type = __remove_cvref_t<decltype(__t_mem)>;
if constexpr (!is_same_v<_Type,
@@ -1706,7 +1695,6 @@ namespace __variant
+ std::hash<_Type>{}(__t_mem);
else
__ret = std::hash<size_t>{}(__t.index());
- return {};
}, __t);
return __ret;
}
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-04-10 20:32 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-05 18:06 [PATCH] Implement std::visit<R> for C++2a (P0655R1) Jonathan Wakely
2019-04-05 20:29 ` Jonathan Wakely
2019-04-05 22:55 ` Ville Voutilainen
2019-04-05 23:19 ` Ville Voutilainen
2019-04-05 23:48 ` Ville Voutilainen
2019-04-05 23:56 ` Ville Voutilainen
2019-04-06 0:07 ` Ville Voutilainen
2019-04-08 16:02 ` Jonathan Wakely
2019-04-08 16:12 ` Ville Voutilainen
2019-04-08 16:20 ` Ville Voutilainen
2019-04-08 16:36 ` Jonathan Wakely
2019-04-08 18:54 ` Jonathan Wakely
2019-04-08 18:56 ` Jonathan Wakely
2019-04-10 21:27 ` Jonathan Wakely
2019-04-08 16:23 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).