This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
- From: Ville Voutilainen <ville dot voutilainen at gmail dot com>
- To: Jonathan Wakely <jwakely at redhat dot com>
- Cc: "libstdc++" <libstdc++ at gcc dot gnu dot org>, gcc-patches List <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 6 Apr 2019 02:19:15 +0300
- Subject: Re: [PATCH] Implement std::visit<R> for C++2a (P0655R1)
- References: <20190405180616.GA15228@redhat.com> <20190405202927.GR943@redhat.com> <CAFk2RUZKezA1tyhUbsShhH8hba_6JiNUaHU=HRKeyRh9os4yvw@mail.gmail.com>
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.
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();
+}